Merge pull request #7094 from heyitsanthony/fix-duplicate-grant

auth: use quorum get for GetUser/GetRole for mutable operations

etcdserver/auth/auth.go

@@ -167,7 +167,7 @@ func (_ passwordStore) HashPassword(password string) (string, error) {
 }
 
 func (s *store) AllUsers() ([]string, error) {
-	resp, err := s.requestResource("/users/", false)
+	resp, err := s.requestResource("/users/", false, false)
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -185,33 +185,13 @@ func (s *store) AllUsers() ([]string, error) {
 	return nodes, nil
 }
 
-func (s *store) GetUser(name string) (User, error) {
-	resp, err := s.requestResource("/users/"+name, false)
-	if err != nil {
-		if e, ok := err.(*etcderr.Error); ok {
-			if e.ErrorCode == etcderr.EcodeKeyNotFound {
-				return User{}, authErr(http.StatusNotFound, "User %s does not exist.", name)
-			}
-		}
-		return User{}, err
-	}
-	var u User
-	err = json.Unmarshal([]byte(*resp.Event.Node.Value), &u)
-	if err != nil {
-		return u, err
-	}
-	// Attach root role to root user.
-	if u.User == "root" {
-		u = attachRootRole(u)
-	}
-	return u, nil
-}
+func (s *store) GetUser(name string) (User, error) { return s.getUser(name, false) }
 
 // CreateOrUpdateUser should be only used for creating the new user or when you are not
 // sure if it is a create or update. (When only password is passed in, we are not sure
 // if it is a update or create)
 func (s *store) CreateOrUpdateUser(user User) (out User, created bool, err error) {
-	_, err = s.GetUser(user.User)
+	_, err = s.getUser(user.User, true)
 	if err == nil {
 		out, err = s.UpdateUser(user)
 		return out, false, err
@@ -271,7 +251,7 @@ func (s *store) DeleteUser(name string) error {
 }
 
 func (s *store) UpdateUser(user User) (User, error) {
-	old, err := s.GetUser(user.User)
+	old, err := s.getUser(user.User, true)
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -297,7 +277,7 @@ func (s *store) UpdateUser(user User) (User, error) {
 
 func (s *store) AllRoles() ([]string, error) {
 	nodes := []string{RootRoleName}
-	resp, err := s.requestResource("/roles/", false)
+	resp, err := s.requestResource("/roles/", false, false)
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -314,23 +294,7 @@ func (s *store) AllRoles() ([]string, error) {
 	return nodes, nil
 }
 
-func (s *store) GetRole(name string) (Role, error) {
-	if name == RootRoleName {
-		return rootRole, nil
-	}
-	resp, err := s.requestResource("/roles/"+name, false)
-	if err != nil {
-		if e, ok := err.(*etcderr.Error); ok {
-			if e.ErrorCode == etcderr.EcodeKeyNotFound {
-				return Role{}, authErr(http.StatusNotFound, "Role %s does not exist.", name)
-			}
-		}
-		return Role{}, err
-	}
-	var r Role
-	err = json.Unmarshal([]byte(*resp.Event.Node.Value), &r)
-	return r, err
-}
+func (s *store) GetRole(name string) (Role, error) { return s.getRole(name, false) }
 
 func (s *store) CreateRole(role Role) error {
 	if role.Role == RootRoleName {
@@ -372,7 +336,7 @@ func (s *store) UpdateRole(role Role) (Role, error) {
 	if role.Role == RootRoleName {
 		return Role{}, authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", role.Role)
 	}
-	old, err := s.GetRole(role.Role)
+	old, err := s.getRole(role.Role, true)
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -404,10 +368,10 @@ func (s *store) EnableAuth() error {
 		return authErr(http.StatusConflict, "already enabled")
 	}
 
-	if _, err := s.GetUser("root"); err != nil {
+	if _, err := s.getUser("root", true); err != nil {
 		return authErr(http.StatusConflict, "No root user available, please create one")
 	}
-	if _, err := s.GetRole(GuestRoleName); err != nil {
+	if _, err := s.getRole(GuestRoleName, true); err != nil {
 		plog.Printf("no guest role access found, creating default")
 		if err := s.CreateRole(guestRole); err != nil {
 			plog.Errorf("error creating guest role. aborting auth enable.")
@@ -641,3 +605,43 @@ func attachRootRole(u User) User {
 	}
 	return u
 }
+
+func (s *store) getUser(name string, quorum bool) (User, error) {
+	resp, err := s.requestResource("/users/"+name, false, quorum)
+	if err != nil {
+		if e, ok := err.(*etcderr.Error); ok {
+			if e.ErrorCode == etcderr.EcodeKeyNotFound {
+				return User{}, authErr(http.StatusNotFound, "User %s does not exist.", name)
+			}
+		}
+		return User{}, err
+	}
+	var u User
+	err = json.Unmarshal([]byte(*resp.Event.Node.Value), &u)
+	if err != nil {
+		return u, err
+	}
+	// Attach root role to root user.
+	if u.User == "root" {
+		u = attachRootRole(u)
+	}
+	return u, nil
+}
+
+func (s *store) getRole(name string, quorum bool) (Role, error) {
+	if name == RootRoleName {
+		return rootRole, nil
+	}
+	resp, err := s.requestResource("/roles/"+name, false, quorum)
+	if err != nil {
+		if e, ok := err.(*etcderr.Error); ok {
+			if e.ErrorCode == etcderr.EcodeKeyNotFound {
+				return Role{}, authErr(http.StatusNotFound, "Role %s does not exist.", name)
+			}
+		}
+		return Role{}, err
+	}
+	var r Role
+	err = json.Unmarshal([]byte(*resp.Event.Node.Value), &r)
+	return r, err
+}

etcdserver/auth/auth_requests.go

@@ -85,7 +85,7 @@ func (s *store) detectAuth() bool {
 	if s.server == nil {
 		return false
 	}
-	value, err := s.requestResource("/enabled", false)
+	value, err := s.requestResource("/enabled", false, false)
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -105,12 +105,16 @@ func (s *store) detectAuth() bool {
 	return u
 }
 
-func (s *store) requestResource(res string, dir bool) (etcdserver.Response, error) {
+func (s *store) requestResource(res string, dir, quorum bool) (etcdserver.Response, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
 	defer cancel()
 	p := path.Join(StorePermsPrefix, res)
+	method := "GET"
+	if quorum {
+		method = "QGET"
+	}
 	rr := etcdserverpb.Request{
-		Method: "GET",
+		Method: method,
 		Path:   p,
 		Dir:    dir,
 	}

etcdserver/auth/auth_test.go

@@ -173,7 +173,7 @@ func (td *testDoer) Do(_ context.Context, req etcdserverpb.Request) (etcdserver.
 			},
 		}, nil
 	}
-	if req.Method == "GET" && td.get != nil {
+	if (req.Method == "GET" || req.Method == "QGET") && td.get != nil {
 		res := td.get[td.getindex]
 		if res.Event == nil {
 			td.getindex++