Explorar el Código

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

GetUser would not propagate to the minority node, causing TestCtlV2GetRoleUser to
run CreateUser instead of UpdateUser. Instead, use quorum get to fetch the
current state of auth.

Fixes #7069
Anthony Romano hace 9 años
padre
commit
f9f691ef1f
Se han modificado 3 ficheros con 57 adiciones y 49 borrados
  1. 49 45
      etcdserver/auth/auth.go
  2. 7 3
      etcdserver/auth/auth_requests.go
  3. 1 1
      etcdserver/auth/auth_test.go

+ 49 - 45
etcdserver/auth/auth.go

@@ -167,7 +167,7 @@ func (_ passwordStore) HashPassword(password string) (string, error) {
 }
 }
 
 
 func (s *store) AllUsers() ([]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 err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -185,33 +185,13 @@ func (s *store) AllUsers() ([]string, error) {
 	return nodes, nil
 	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
 // 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
 // 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)
 // if it is a update or create)
 func (s *store) CreateOrUpdateUser(user User) (out User, created bool, err error) {
 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 {
 	if err == nil {
 		out, err = s.UpdateUser(user)
 		out, err = s.UpdateUser(user)
 		return out, false, err
 		return out, false, err
@@ -271,7 +251,7 @@ func (s *store) DeleteUser(name string) error {
 }
 }
 
 
 func (s *store) UpdateUser(user User) (User, 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 err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -297,7 +277,7 @@ func (s *store) UpdateUser(user User) (User, error) {
 
 
 func (s *store) AllRoles() ([]string, error) {
 func (s *store) AllRoles() ([]string, error) {
 	nodes := []string{RootRoleName}
 	nodes := []string{RootRoleName}
-	resp, err := s.requestResource("/roles/", false)
+	resp, err := s.requestResource("/roles/", false, false)
 	if err != nil {
 	if err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -314,23 +294,7 @@ func (s *store) AllRoles() ([]string, error) {
 	return nodes, nil
 	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 {
 func (s *store) CreateRole(role Role) error {
 	if role.Role == RootRoleName {
 	if role.Role == RootRoleName {
@@ -372,7 +336,7 @@ func (s *store) UpdateRole(role Role) (Role, error) {
 	if role.Role == RootRoleName {
 	if role.Role == RootRoleName {
 		return Role{}, authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", role.Role)
 		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 err != nil {
 		if e, ok := err.(*etcderr.Error); ok {
 		if e, ok := err.(*etcderr.Error); ok {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
 			if e.ErrorCode == etcderr.EcodeKeyNotFound {
@@ -404,10 +368,10 @@ func (s *store) EnableAuth() error {
 		return authErr(http.StatusConflict, "already enabled")
 		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")
 		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")
 		plog.Printf("no guest role access found, creating default")
 		if err := s.CreateRole(guestRole); err != nil {
 		if err := s.CreateRole(guestRole); err != nil {
 			plog.Errorf("error creating guest role. aborting auth enable.")
 			plog.Errorf("error creating guest role. aborting auth enable.")
@@ -641,3 +605,43 @@ func attachRootRole(u User) User {
 	}
 	}
 	return u
 	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
+}

+ 7 - 3
etcdserver/auth/auth_requests.go

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

+ 1 - 1
etcdserver/auth/auth_test.go

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