From f9f691ef1f6a54533749c528cbd5792f99740573 Mon Sep 17 00:00:00 2001 From: Anthony Romano Date: Tue, 3 Jan 2017 16:34:36 -0800 Subject: [PATCH] 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 --- etcdserver/auth/auth.go | 94 +++++++++++++++++--------------- etcdserver/auth/auth_requests.go | 10 +++- etcdserver/auth/auth_test.go | 2 +- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/etcdserver/auth/auth.go b/etcdserver/auth/auth.go index 0922a7f8b..19e96d57c 100644 --- a/etcdserver/auth/auth.go +++ b/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 +} diff --git a/etcdserver/auth/auth_requests.go b/etcdserver/auth/auth_requests.go index 36bed20d0..eec700acc 100644 --- a/etcdserver/auth/auth_requests.go +++ b/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, } diff --git a/etcdserver/auth/auth_test.go b/etcdserver/auth/auth_test.go index fc11fdc13..2d81d9246 100644 --- a/etcdserver/auth/auth_test.go +++ b/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++