From dd1a8fe330fd981a4de5284c0d394a728ebb55b8 Mon Sep 17 00:00:00 2001 From: Barak Michener Date: Wed, 1 Jul 2015 15:10:16 -0400 Subject: [PATCH] etcdhttp: Improve test coverage surrounding auth --- etcdserver/auth/auth.go | 83 +++-- etcdserver/auth/auth_requests.go | 18 +- etcdserver/auth/auth_test.go | 188 +++++----- etcdserver/etcdhttp/client.go | 4 +- etcdserver/etcdhttp/client_auth.go | 10 +- etcdserver/etcdhttp/client_auth_test.go | 436 ++++++++++++++++++++++++ 6 files changed, 596 insertions(+), 143 deletions(-) create mode 100644 etcdserver/etcdhttp/client_auth_test.go diff --git a/etcdserver/auth/auth.go b/etcdserver/auth/auth.go index 09d907c18..14636bc63 100644 --- a/etcdserver/auth/auth.go +++ b/etcdserver/auth/auth.go @@ -51,7 +51,7 @@ var ( var rootRole = Role{ Role: RootRoleName, Permissions: Permissions{ - KV: rwPermission{ + KV: RWPermission{ Read: []string{"*"}, Write: []string{"*"}, }, @@ -61,7 +61,7 @@ var rootRole = Role{ var guestRole = Role{ Role: GuestRoleName, Permissions: Permissions{ - KV: rwPermission{ + KV: RWPermission{ Read: []string{"*"}, Write: []string{"*"}, }, @@ -72,7 +72,24 @@ type doer interface { Do(context.Context, etcdserverpb.Request) (etcdserver.Response, error) } -type Store struct { +type Store interface { + AllUsers() ([]string, error) + GetUser(name string) (User, error) + CreateOrUpdateUser(user User) (out User, created bool, err error) + CreateUser(user User) (User, error) + DeleteUser(name string) error + UpdateUser(user User) (User, error) + AllRoles() ([]string, error) + GetRole(name string) (Role, error) + CreateRole(role Role) error + DeleteRole(name string) error + UpdateRole(role Role) (Role, error) + AuthEnabled() bool + EnableAuth() error + DisableAuth() error +} + +type store struct { server doer timeout time.Duration ensuredOnce bool @@ -94,39 +111,39 @@ type Role struct { } type Permissions struct { - KV rwPermission `json:"kv"` + KV RWPermission `json:"kv"` } func (p *Permissions) IsEmpty() bool { return p == nil || (len(p.KV.Read) == 0 && len(p.KV.Write) == 0) } -type rwPermission struct { +type RWPermission struct { Read []string `json:"read"` Write []string `json:"write"` } type Error struct { - httpStatus int - errmsg string + Status int + Errmsg string } -func (ae Error) Error() string { return ae.errmsg } -func (ae Error) HTTPStatus() int { return ae.httpStatus } +func (ae Error) Error() string { return ae.Errmsg } +func (ae Error) HTTPStatus() int { return ae.Status } func authErr(hs int, s string, v ...interface{}) Error { - return Error{httpStatus: hs, errmsg: fmt.Sprintf("auth: "+s, v...)} + return Error{Status: hs, Errmsg: fmt.Sprintf("auth: "+s, v...)} } -func NewStore(server doer, timeout time.Duration) *Store { - s := &Store{ +func NewStore(server doer, timeout time.Duration) Store { + s := &store{ server: server, timeout: timeout, } return s } -func (s *Store) AllUsers() ([]string, error) { +func (s *store) AllUsers() ([]string, error) { resp, err := s.requestResource("/users/", false) if err != nil { if e, ok := err.(*etcderr.Error); ok { @@ -145,7 +162,7 @@ func (s *Store) AllUsers() ([]string, error) { return nodes, nil } -func (s *Store) GetUser(name string) (User, error) { +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 { @@ -170,7 +187,7 @@ func (s *Store) GetUser(name string) (User, error) { // 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) { +func (s *store) CreateOrUpdateUser(user User) (out User, created bool, err error) { _, err = s.GetUser(user.User) if err == nil { out, err := s.UpdateUser(user) @@ -180,7 +197,7 @@ func (s *Store) CreateOrUpdateUser(user User) (out User, created bool, err error return u, true, err } -func (s *Store) CreateUser(user User) (User, error) { +func (s *store) CreateUser(user User) (User, error) { // Attach root role to root user. if user.User == "root" { user = attachRootRole(user) @@ -192,7 +209,7 @@ func (s *Store) CreateUser(user User) (User, error) { return u, err } -func (s *Store) createUserInternal(user User) (User, error) { +func (s *store) createUserInternal(user User) (User, error) { if user.Password == "" { return user, authErr(http.StatusBadRequest, "Cannot create user %s with an empty password", user.User) } @@ -213,7 +230,7 @@ func (s *Store) createUserInternal(user User) (User, error) { return user, err } -func (s *Store) DeleteUser(name string) error { +func (s *store) DeleteUser(name string) error { if s.AuthEnabled() && name == "root" { return authErr(http.StatusForbidden, "Cannot delete root user while auth is enabled.") } @@ -230,7 +247,7 @@ func (s *Store) DeleteUser(name string) error { return nil } -func (s *Store) UpdateUser(user User) (User, error) { +func (s *store) UpdateUser(user User) (User, error) { old, err := s.GetUser(user.User) if err != nil { if e, ok := err.(*etcderr.Error); ok { @@ -254,7 +271,7 @@ func (s *Store) UpdateUser(user User) (User, error) { return newUser, err } -func (s *Store) AllRoles() ([]string, error) { +func (s *store) AllRoles() ([]string, error) { nodes := []string{RootRoleName} resp, err := s.requestResource("/roles/", false) if err != nil { @@ -273,7 +290,7 @@ func (s *Store) AllRoles() ([]string, error) { return nodes, nil } -func (s *Store) GetRole(name string) (Role, error) { +func (s *store) GetRole(name string) (Role, error) { if name == RootRoleName { return rootRole, nil } @@ -295,7 +312,7 @@ func (s *Store) GetRole(name string) (Role, error) { return r, nil } -func (s *Store) CreateRole(role Role) error { +func (s *store) CreateRole(role Role) error { if role.Role == RootRoleName { return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", role.Role) } @@ -313,7 +330,7 @@ func (s *Store) CreateRole(role Role) error { return err } -func (s *Store) DeleteRole(name string) error { +func (s *store) DeleteRole(name string) error { if name == RootRoleName { return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", name) } @@ -331,7 +348,7 @@ func (s *Store) DeleteRole(name string) error { return err } -func (s *Store) UpdateRole(role Role) (Role, error) { +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) } @@ -358,11 +375,11 @@ func (s *Store) UpdateRole(role Role) (Role, error) { return newRole, err } -func (s *Store) AuthEnabled() bool { +func (s *store) AuthEnabled() bool { return s.detectAuth() } -func (s *Store) EnableAuth() error { +func (s *store) EnableAuth() error { if s.AuthEnabled() { return authErr(http.StatusConflict, "already enabled") } @@ -388,7 +405,7 @@ func (s *Store) EnableAuth() error { return err } -func (s *Store) DisableAuth() error { +func (s *store) DisableAuth() error { if !s.AuthEnabled() { return authErr(http.StatusConflict, "already disabled") } @@ -505,8 +522,8 @@ func (p Permissions) Revoke(n *Permissions) (Permissions, error) { // Grant adds a set of permissions to the permission object on which it is called, // returning a new permission object. -func (rw rwPermission) Grant(n rwPermission) (rwPermission, error) { - var out rwPermission +func (rw RWPermission) Grant(n RWPermission) (RWPermission, error) { + var out RWPermission currentRead := types.NewUnsafeSet(rw.Read...) for _, r := range n.Read { if currentRead.Contains(r) { @@ -530,8 +547,8 @@ func (rw rwPermission) Grant(n rwPermission) (rwPermission, error) { // Revoke removes a set of permissions to the permission object on which it is called, // returning a new permission object. -func (rw rwPermission) Revoke(n rwPermission) (rwPermission, error) { - var out rwPermission +func (rw RWPermission) Revoke(n RWPermission) (RWPermission, error) { + var out RWPermission currentRead := types.NewUnsafeSet(rw.Read...) for _, r := range n.Read { if !currentRead.Contains(r) { @@ -555,7 +572,7 @@ func (rw rwPermission) Revoke(n rwPermission) (rwPermission, error) { return out, nil } -func (rw rwPermission) HasAccess(key string, write bool) bool { +func (rw RWPermission) HasAccess(key string, write bool) bool { var list []string if write { list = rw.Write @@ -571,7 +588,7 @@ func (rw rwPermission) HasAccess(key string, write bool) bool { return false } -func (rw rwPermission) HasRecursiveAccess(key string, write bool) bool { +func (rw RWPermission) HasRecursiveAccess(key string, write bool) bool { list := rw.Read if write { list = rw.Write diff --git a/etcdserver/auth/auth_requests.go b/etcdserver/auth/auth_requests.go index d51c57db8..7103578c4 100644 --- a/etcdserver/auth/auth_requests.go +++ b/etcdserver/auth/auth_requests.go @@ -24,7 +24,7 @@ import ( "github.com/coreos/etcd/etcdserver/etcdserverpb" ) -func (s *Store) ensureAuthDirectories() error { +func (s *store) ensureAuthDirectories() error { if s.ensuredOnce { return nil } @@ -72,16 +72,16 @@ func (s *Store) ensureAuthDirectories() error { return nil } -func (s *Store) enableAuth() error { +func (s *store) enableAuth() error { _, err := s.updateResource("/enabled", true) return err } -func (s *Store) disableAuth() error { +func (s *store) disableAuth() error { _, err := s.updateResource("/enabled", false) return err } -func (s *Store) detectAuth() bool { +func (s *store) detectAuth() bool { if s.server == nil { return false } @@ -105,7 +105,7 @@ 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 bool) (etcdserver.Response, error) { ctx, cancel := context.WithTimeout(context.Background(), s.timeout) defer cancel() p := path.Join(StorePermsPrefix, res) @@ -117,13 +117,13 @@ func (s *Store) requestResource(res string, dir bool) (etcdserver.Response, erro return s.server.Do(ctx, rr) } -func (s *Store) updateResource(res string, value interface{}) (etcdserver.Response, error) { +func (s *store) updateResource(res string, value interface{}) (etcdserver.Response, error) { return s.setResource(res, value, true) } -func (s *Store) createResource(res string, value interface{}) (etcdserver.Response, error) { +func (s *store) createResource(res string, value interface{}) (etcdserver.Response, error) { return s.setResource(res, value, false) } -func (s *Store) setResource(res string, value interface{}, prevexist bool) (etcdserver.Response, error) { +func (s *store) setResource(res string, value interface{}, prevexist bool) (etcdserver.Response, error) { err := s.ensureAuthDirectories() if err != nil { return etcdserver.Response{}, err @@ -144,7 +144,7 @@ func (s *Store) setResource(res string, value interface{}, prevexist bool) (etcd return s.server.Do(ctx, rr) } -func (s *Store) deleteResource(res string) (etcdserver.Response, error) { +func (s *store) deleteResource(res string) (etcdserver.Response, error) { err := s.ensureAuthDirectories() if err != nil { return etcdserver.Response{}, err diff --git a/etcdserver/auth/auth_test.go b/etcdserver/auth/auth_test.go index ea55642ad..704e56ddf 100644 --- a/etcdserver/auth/auth_test.go +++ b/etcdserver/auth/auth_test.go @@ -24,7 +24,7 @@ import ( etcderr "github.com/coreos/etcd/error" "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdserverpb" - "github.com/coreos/etcd/store" + etcdstore "github.com/coreos/etcd/store" ) const testTimeout = time.Millisecond @@ -112,19 +112,19 @@ func TestMergeRole(t *testing.T) { }, { Role{Role: "foo"}, - Role{Role: "foo", Grant: &Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, - Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Grant: &Permissions{KV: RWPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Permissions: Permissions{KV: RWPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, false, }, { - Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, - Role{Role: "foo", Revoke: &Permissions{KV: rwPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, - Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{}, Write: []string{}}}}, + Role{Role: "foo", Permissions: Permissions{KV: RWPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Revoke: &Permissions{KV: RWPermission{Read: []string{"/foodir"}, Write: []string{"/foodir"}}}}, + Role{Role: "foo", Permissions: Permissions{KV: RWPermission{Read: []string{}, Write: []string{}}}}, false, }, { - Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/bardir"}}}}, - Role{Role: "foo", Revoke: &Permissions{KV: rwPermission{Read: []string{"/foodir"}}}}, + Role{Role: "foo", Permissions: Permissions{KV: RWPermission{Read: []string{"/bardir"}}}}, + Role{Role: "foo", Revoke: &Permissions{KV: RWPermission{Read: []string{"/foodir"}}}}, Role{}, true, }, @@ -154,9 +154,9 @@ func (td *testDoer) Do(_ context.Context, req etcdserverpb.Request) (etcdserver. if td.explicitlyEnabled && (req.Path == StorePermsPrefix+"/enabled") { t := "true" return etcdserver.Response{ - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &t, }, @@ -192,14 +192,14 @@ func TestAllUsers(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Nodes: store.NodeExterns([]*store.NodeExtern{ - &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ + Nodes: etcdstore.NodeExterns([]*etcdstore.NodeExtern{ + &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", }, - &store.NodeExtern{ + &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/dog", }, }), @@ -210,7 +210,7 @@ func TestAllUsers(t *testing.T) { } expected := []string{"cat", "dog"} - s := Store{d, testTimeout, false} + s := store{d, testTimeout, false} users, err := s.AllUsers() if err != nil { t.Error("Unexpected error", err) @@ -225,9 +225,9 @@ func TestGetAndDeleteUser(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &data, }, @@ -238,7 +238,7 @@ func TestGetAndDeleteUser(t *testing.T) { } expected := User{User: "cat", Roles: []string{"animal"}} - s := Store{d, testTimeout, false} + s := store{d, testTimeout, false} out, err := s.GetUser("cat") if err != nil { t.Error("Unexpected error", err) @@ -256,14 +256,14 @@ func TestAllRoles(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ - Nodes: store.NodeExterns([]*store.NodeExtern{ - &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ + Nodes: etcdstore.NodeExterns([]*etcdstore.NodeExtern{ + &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/animal", }, - &store.NodeExtern{ + &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/human", }, }), @@ -275,7 +275,7 @@ func TestAllRoles(t *testing.T) { } expected := []string{"animal", "human", "root"} - s := Store{d, testTimeout, false} + s := store{d, testTimeout, false} out, err := s.AllRoles() if err != nil { t.Error("Unexpected error", err) @@ -290,9 +290,9 @@ func TestGetAndDeleteRole(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/animal", Value: &data, }, @@ -303,7 +303,7 @@ func TestGetAndDeleteRole(t *testing.T) { } expected := Role{Role: "animal"} - s := Store{d, testTimeout, false} + s := store{d, testTimeout, false} out, err := s.GetRole("animal") if err != nil { t.Error("Unexpected error", err) @@ -321,27 +321,27 @@ func TestEnsure(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Set, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Set, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix, Dir: true, }, }, }, { - Event: &store.Event{ - Action: store.Set, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Set, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/", Dir: true, }, }, }, { - Event: &store.Event{ - Action: store.Set, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Set, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/", Dir: true, }, @@ -350,7 +350,7 @@ func TestEnsure(t *testing.T) { }, } - s := Store{d, testTimeout, false} + s := store{d, testTimeout, false} err := s.ensureAuthDirectories() if err != nil { t.Error("Unexpected error", err) @@ -366,18 +366,18 @@ func TestCreateAndUpdateUser(t *testing.T) { Event: nil, }, { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &olduser, }, }, }, { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &olduser, }, @@ -386,18 +386,18 @@ func TestCreateAndUpdateUser(t *testing.T) { }, put: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Update, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Update, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &olduser, }, }, }, { - Event: &store.Event{ - Action: store.Update, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Update, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/users/cat", Value: &newuser, }, @@ -410,7 +410,7 @@ func TestCreateAndUpdateUser(t *testing.T) { update := User{User: "cat", Grant: []string{"pet"}} expected := User{User: "cat", Roles: []string{"animal", "pet"}} - s := Store{d, testTimeout, true} + s := store{d, testTimeout, true} out, created, err := s.CreateOrUpdateUser(user) if created == false { t.Error("Should have created user, instead updated?") @@ -440,9 +440,9 @@ func TestUpdateRole(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/animal", Value: &oldrole, }, @@ -451,9 +451,9 @@ func TestUpdateRole(t *testing.T) { }, put: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Update, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Update, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/animal", Value: &newrole, }, @@ -462,10 +462,10 @@ func TestUpdateRole(t *testing.T) { }, explicitlyEnabled: true, } - update := Role{Role: "animal", Grant: &Permissions{KV: rwPermission{Read: []string{}, Write: []string{"/animal"}}}} - expected := Role{Role: "animal", Permissions: Permissions{KV: rwPermission{Read: []string{"/animal"}, Write: []string{"/animal"}}}} + update := Role{Role: "animal", Grant: &Permissions{KV: RWPermission{Read: []string{}, Write: []string{"/animal"}}}} + expected := Role{Role: "animal", Permissions: Permissions{KV: RWPermission{Read: []string{"/animal"}, Write: []string{"/animal"}}}} - s := Store{d, testTimeout, true} + s := store{d, testTimeout, true} out, err := s.UpdateRole(update) if err != nil { t.Error("Unexpected error", err) @@ -480,9 +480,9 @@ func TestCreateRole(t *testing.T) { d := &testDoer{ put: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Create, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Create, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/animal", Value: &role, }, @@ -494,9 +494,9 @@ func TestCreateRole(t *testing.T) { }, explicitlyEnabled: true, } - r := Role{Role: "animal", Permissions: Permissions{KV: rwPermission{Read: []string{"/animal"}, Write: []string{}}}} + r := Role{Role: "animal", Permissions: Permissions{KV: RWPermission{Read: []string{"/animal"}, Write: []string{}}}} - s := Store{d, testTimeout, true} + s := store{d, testTimeout, true} err := s.CreateRole(Role{Role: "root"}) if err == nil { t.Error("Should error creating root role") @@ -519,18 +519,18 @@ func TestEnableAuth(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/enabled", Value: &falseval, }, }, }, { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/user/root", Value: &rootUser, }, @@ -542,18 +542,18 @@ func TestEnableAuth(t *testing.T) { }, put: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Create, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Create, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/roles/guest", Value: &guestRole, }, }, }, { - Event: &store.Event{ - Action: store.Update, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Update, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/enabled", Value: &trueval, }, @@ -562,7 +562,7 @@ func TestEnableAuth(t *testing.T) { }, explicitlyEnabled: false, } - s := Store{d, testTimeout, true} + s := store{d, testTimeout, true} err := s.EnableAuth() if err != nil { t.Error("Unexpected error", err) @@ -574,18 +574,18 @@ func TestDisableAuth(t *testing.T) { d := &testDoer{ get: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/enabled", Value: &falseval, }, }, }, { - Event: &store.Event{ - Action: store.Get, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Get, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/enabled", Value: &trueval, }, @@ -594,9 +594,9 @@ func TestDisableAuth(t *testing.T) { }, put: []etcdserver.Response{ { - Event: &store.Event{ - Action: store.Update, - Node: &store.NodeExtern{ + Event: &etcdstore.Event{ + Action: etcdstore.Update, + Node: &etcdstore.NodeExtern{ Key: StorePermsPrefix + "/enabled", Value: &falseval, }, @@ -605,7 +605,7 @@ func TestDisableAuth(t *testing.T) { }, explicitlyEnabled: false, } - s := Store{d, testTimeout, true} + s := store{d, testTimeout, true} err := s.DisableAuth() if err == nil { t.Error("Expected error; already disabled") @@ -617,7 +617,7 @@ func TestDisableAuth(t *testing.T) { } func TestSimpleMatch(t *testing.T) { - role := Role{Role: "foo", Permissions: Permissions{KV: rwPermission{Read: []string{"/foodir/*", "/fookey"}, Write: []string{"/bardir/*", "/barkey"}}}} + role := Role{Role: "foo", Permissions: Permissions{KV: RWPermission{Read: []string{"/foodir/*", "/fookey"}, Write: []string{"/bardir/*", "/barkey"}}}} if !role.HasKeyAccess("/foodir/foo/bar", false) { t.Fatal("role lacks expected access") } diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 41843d9ce..c59c1a5c5 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -111,7 +111,7 @@ func NewClientHandler(server *etcdserver.EtcdServer) http.Handler { } type keysHandler struct { - sec *auth.Store + sec auth.Store server etcdserver.Server cluster etcdserver.Cluster timer etcdserver.RaftTimer @@ -173,7 +173,7 @@ func (h *deprecatedMachinesHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } type membersHandler struct { - sec *auth.Store + sec auth.Store server etcdserver.Server cluster etcdserver.Cluster clock clockwork.Clock diff --git a/etcdserver/etcdhttp/client_auth.go b/etcdserver/etcdhttp/client_auth.go index d74f29e46..c1a34aebb 100644 --- a/etcdserver/etcdhttp/client_auth.go +++ b/etcdserver/etcdhttp/client_auth.go @@ -27,18 +27,18 @@ import ( ) type authHandler struct { - sec *auth.Store + sec auth.Store cluster etcdserver.Cluster } -func hasWriteRootAccess(sec *auth.Store, r *http.Request) bool { +func hasWriteRootAccess(sec auth.Store, r *http.Request) bool { if r.Method == "GET" || r.Method == "HEAD" { return true } return hasRootAccess(sec, r) } -func hasRootAccess(sec *auth.Store, r *http.Request) bool { +func hasRootAccess(sec auth.Store, r *http.Request) bool { if sec == nil { // No store means no auth available, eg, tests. return true @@ -68,7 +68,7 @@ func hasRootAccess(sec *auth.Store, r *http.Request) bool { return false } -func hasKeyPrefixAccess(sec *auth.Store, r *http.Request, key string, recursive bool) bool { +func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive bool) bool { if sec == nil { // No store means no auth available, eg, tests. return true @@ -105,7 +105,7 @@ func hasKeyPrefixAccess(sec *auth.Store, r *http.Request, key string, recursive return false } -func hasGuestAccess(sec *auth.Store, r *http.Request, key string) bool { +func hasGuestAccess(sec auth.Store, r *http.Request, key string) bool { writeAccess := r.Method != "GET" && r.Method != "HEAD" role, err := sec.GetRole(auth.GuestRoleName) if err != nil { diff --git a/etcdserver/etcdhttp/client_auth_test.go b/etcdserver/etcdhttp/client_auth_test.go new file mode 100644 index 000000000..d7cf4accf --- /dev/null +++ b/etcdserver/etcdhttp/client_auth_test.go @@ -0,0 +1,436 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package etcdhttp + +import ( + "errors" + "net/http" + "net/http/httptest" + "path" + "strings" + "testing" + + "github.com/coreos/etcd/etcdserver/auth" +) + +const goodPassword = "$2a$10$VYdJecHfm6WNodzv8XhmYeIG4n2SsQefdo5V2t6xIq/aWDHNqSUQW" + +func mustJSONRequest(t *testing.T, method string, p string, body string) *http.Request { + req, err := http.NewRequest(method, path.Join(authPrefix, p), strings.NewReader(body)) + if err != nil { + t.Fatalf("Error making JSON request: %s %s %s\n", method, p, body) + } + req.Header.Set("Content-Type", "application/json") + return req +} + +type mockAuthStore struct { + user *auth.User + role *auth.Role + err error + enabled bool +} + +func (s *mockAuthStore) AllUsers() ([]string, error) { return []string{"alice", "bob", "root"}, s.err } +func (s *mockAuthStore) GetUser(name string) (auth.User, error) { return *s.user, s.err } +func (s *mockAuthStore) CreateOrUpdateUser(user auth.User) (out auth.User, created bool, err error) { + if s.user == nil { + u, err := s.CreateUser(user) + return u, true, err + } + u, err := s.UpdateUser(user) + return u, false, err +} +func (s *mockAuthStore) CreateUser(user auth.User) (auth.User, error) { return user, s.err } +func (s *mockAuthStore) DeleteUser(name string) error { return s.err } +func (s *mockAuthStore) UpdateUser(user auth.User) (auth.User, error) { return *s.user, s.err } +func (s *mockAuthStore) AllRoles() ([]string, error) { + return []string{"awesome", "guest", "root"}, s.err +} +func (s *mockAuthStore) GetRole(name string) (auth.Role, error) { return *s.role, s.err } +func (s *mockAuthStore) CreateRole(role auth.Role) error { return s.err } +func (s *mockAuthStore) DeleteRole(name string) error { return s.err } +func (s *mockAuthStore) UpdateRole(role auth.Role) (auth.Role, error) { return *s.role, s.err } +func (s *mockAuthStore) AuthEnabled() bool { return s.enabled } +func (s *mockAuthStore) EnableAuth() error { return s.err } +func (s *mockAuthStore) DisableAuth() error { return s.err } + +func TestAuthFlow(t *testing.T) { + enableMapMu.Lock() + enabledMap = make(map[capability]bool) + enabledMap[authCapability] = true + enableMapMu.Unlock() + var testCases = []struct { + req *http.Request + store mockAuthStore + + wcode int + wbody string + }{ + { + req: mustJSONRequest(t, "PUT", "users/alice", `{{{{{{{`), + store: mockAuthStore{}, + wcode: http.StatusBadRequest, + wbody: `{"message":"Invalid JSON in request body."}`, + }, + { + req: mustJSONRequest(t, "PUT", "users/alice", `{"user": "alice", "password": "goodpassword"}`), + store: mockAuthStore{enabled: true}, + wcode: http.StatusUnauthorized, + wbody: `{"message":"Insufficient credentials"}`, + }, + // Users + { + req: mustJSONRequest(t, "GET", "users", ""), + store: mockAuthStore{}, + wcode: http.StatusOK, + wbody: `{"users":["alice","bob","root"]}`, + }, + { + req: mustJSONRequest(t, "GET", "users/alice", ""), + store: mockAuthStore{ + user: &auth.User{ + User: "alice", + Roles: []string{"alicerole", "guest"}, + Password: "wheeee", + }, + }, + wcode: http.StatusOK, + wbody: `{"user":"alice","roles":["alicerole","guest"]}`, + }, + { + req: mustJSONRequest(t, "PUT", "users/alice", `{"user": "alice", "password": "goodpassword"}`), + store: mockAuthStore{}, + wcode: http.StatusCreated, + wbody: `{"user":"alice","roles":null}`, + }, + { + req: mustJSONRequest(t, "DELETE", "users/alice", ``), + store: mockAuthStore{}, + wcode: http.StatusOK, + wbody: ``, + }, + { + req: mustJSONRequest(t, "PUT", "users/alice", `{"user": "alice", "password": "goodpassword"}`), + store: mockAuthStore{ + user: &auth.User{ + User: "alice", + Roles: []string{"alicerole", "guest"}, + Password: "wheeee", + }, + }, + wcode: http.StatusOK, + wbody: `{"user":"alice","roles":["alicerole","guest"]}`, + }, + { + req: mustJSONRequest(t, "PUT", "users/alice", `{"user": "alice", "grant": ["alicerole"]}`), + store: mockAuthStore{ + user: &auth.User{ + User: "alice", + Roles: []string{"alicerole", "guest"}, + Password: "wheeee", + }, + }, + wcode: http.StatusOK, + wbody: `{"user":"alice","roles":["alicerole","guest"]}`, + }, + { + req: mustJSONRequest(t, "GET", "users/alice", ``), + store: mockAuthStore{ + user: &auth.User{}, + err: auth.Error{Status: http.StatusNotFound, Errmsg: "auth: User alice doesn't exist."}, + }, + wcode: http.StatusNotFound, + wbody: `{"message":"auth: User alice doesn't exist."}`, + }, + // Roles + { + req: mustJSONRequest(t, "GET", "roles/manager", ""), + store: mockAuthStore{ + role: &auth.Role{ + Role: "manager", + }, + }, + wcode: http.StatusOK, + wbody: `{"role":"manager","permissions":{"kv":{"read":null,"write":null}}}`, + }, + { + req: mustJSONRequest(t, "DELETE", "roles/manager", ``), + store: mockAuthStore{}, + wcode: http.StatusOK, + wbody: ``, + }, + { + req: mustJSONRequest(t, "PUT", "roles/manager", `{"role":"manager","permissions":{"kv":{"read":[],"write":[]}}}`), + store: mockAuthStore{}, + wcode: http.StatusCreated, + wbody: `{"role":"manager","permissions":{"kv":{"read":[],"write":[]}}}`, + }, + { + req: mustJSONRequest(t, "PUT", "roles/manager", `{"role":"manager","revoke":{"kv":{"read":["foo"],"write":[]}}}`), + store: mockAuthStore{ + role: &auth.Role{ + Role: "manager", + }, + }, + wcode: http.StatusOK, + wbody: `{"role":"manager","permissions":{"kv":{"read":null,"write":null}}}`, + }, + { + req: mustJSONRequest(t, "GET", "roles", ""), + store: mockAuthStore{}, + wcode: http.StatusOK, + wbody: `{"roles":["awesome","guest","root"]}`, + }, + { + req: mustJSONRequest(t, "GET", "enable", ""), + store: mockAuthStore{ + enabled: true, + }, + wcode: http.StatusOK, + wbody: `{"enabled":true}`, + }, + { + req: mustJSONRequest(t, "PUT", "enable", ""), + store: mockAuthStore{ + enabled: false, + }, + wcode: http.StatusOK, + wbody: ``, + }, + { + req: (func() *http.Request { + req := mustJSONRequest(t, "DELETE", "enable", "") + req.SetBasicAuth("root", "good") + return req + })(), + store: mockAuthStore{ + enabled: true, + user: &auth.User{ + User: "root", + Password: goodPassword, + Roles: []string{"root"}, + }, + role: &auth.Role{ + Role: "root", + }, + }, + wcode: http.StatusOK, + wbody: ``, + }, + } + + for i, tt := range testCases { + mux := http.NewServeMux() + h := &authHandler{ + sec: &tt.store, + cluster: &fakeCluster{id: 1}, + } + handleAuth(mux, h) + rw := httptest.NewRecorder() + mux.ServeHTTP(rw, tt.req) + if rw.Code != tt.wcode { + t.Errorf("#%d: got code=%d, want %d", i, rw.Code, tt.wcode) + } + g := rw.Body.String() + g = strings.TrimSpace(g) + if g != tt.wbody { + t.Errorf("#%d: got body=%s, want %s", i, g, tt.wbody) + } + } +} + +func mustAuthRequest(method, username, password string) *http.Request { + req, err := http.NewRequest(method, "path", strings.NewReader("")) + if err != nil { + panic("Cannot make auth request: " + err.Error()) + } + req.SetBasicAuth(username, password) + return req +} + +func TestPrefixAccess(t *testing.T) { + var table = []struct { + key string + req *http.Request + store *mockAuthStore + hasRoot bool + hasKeyPrefixAccess bool + hasRecursiveAccess bool + }{ + { + key: "/foo", + req: mustAuthRequest("GET", "root", "good"), + store: &mockAuthStore{ + user: &auth.User{ + User: "root", + Password: goodPassword, + Roles: []string{"root"}, + }, + role: &auth.Role{ + Role: "root", + }, + enabled: true, + }, + hasRoot: true, + hasKeyPrefixAccess: true, + hasRecursiveAccess: true, + }, + { + key: "/foo", + req: mustAuthRequest("GET", "user", "good"), + store: &mockAuthStore{ + user: &auth.User{ + User: "user", + Password: goodPassword, + Roles: []string{"foorole"}, + }, + role: &auth.Role{ + Role: "foorole", + Permissions: auth.Permissions{ + KV: auth.RWPermission{ + Read: []string{"/foo"}, + Write: []string{"/foo"}, + }, + }, + }, + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: true, + hasRecursiveAccess: false, + }, + { + key: "/foo", + req: mustAuthRequest("GET", "user", "good"), + store: &mockAuthStore{ + user: &auth.User{ + User: "user", + Password: goodPassword, + Roles: []string{"foorole"}, + }, + role: &auth.Role{ + Role: "foorole", + Permissions: auth.Permissions{ + KV: auth.RWPermission{ + Read: []string{"/foo*"}, + Write: []string{"/foo*"}, + }, + }, + }, + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: true, + hasRecursiveAccess: true, + }, + { + key: "/foo", + req: mustAuthRequest("GET", "user", "bad"), + store: &mockAuthStore{ + user: &auth.User{ + User: "user", + Password: goodPassword, + Roles: []string{"foorole"}, + }, + role: &auth.Role{ + Role: "foorole", + Permissions: auth.Permissions{ + KV: auth.RWPermission{ + Read: []string{"/foo*"}, + Write: []string{"/foo*"}, + }, + }, + }, + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: false, + hasRecursiveAccess: false, + }, + { + key: "/foo", + req: mustAuthRequest("GET", "user", "good"), + store: &mockAuthStore{ + user: &auth.User{}, + err: errors.New("Not the user"), + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: false, + hasRecursiveAccess: false, + }, + { + key: "/foo", + req: mustJSONRequest(t, "GET", "somepath", ""), + store: &mockAuthStore{ + user: &auth.User{ + User: "user", + Password: goodPassword, + Roles: []string{"foorole"}, + }, + role: &auth.Role{ + Role: "guest", + Permissions: auth.Permissions{ + KV: auth.RWPermission{ + Read: []string{"/foo*"}, + Write: []string{"/foo*"}, + }, + }, + }, + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: true, + hasRecursiveAccess: true, + }, + { + key: "/bar", + req: mustJSONRequest(t, "GET", "somepath", ""), + store: &mockAuthStore{ + user: &auth.User{ + User: "user", + Password: goodPassword, + Roles: []string{"foorole"}, + }, + role: &auth.Role{ + Role: "guest", + Permissions: auth.Permissions{ + KV: auth.RWPermission{ + Read: []string{"/foo*"}, + Write: []string{"/foo*"}, + }, + }, + }, + enabled: true, + }, + hasRoot: false, + hasKeyPrefixAccess: false, + hasRecursiveAccess: false, + }, + } + + for i, tt := range table { + if tt.hasRoot != hasRootAccess(tt.store, tt.req) { + t.Errorf("#%d: hasRoot doesn't match (expected %v)", i, tt.hasRoot) + } + if tt.hasKeyPrefixAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, false) { + t.Errorf("#%d: hasKeyPrefixAccess doesn't match (expected %v)", i, tt.hasRoot) + } + if tt.hasRecursiveAccess != hasKeyPrefixAccess(tt.store, tt.req, tt.key, true) { + t.Errorf("#%d: hasRecursiveAccess doesn't match (expected %v)", i, tt.hasRoot) + } + } +}