From e0c33ef881cb9aa2c5a68fc7656a6521d130a295 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Fri, 23 Jun 2017 14:40:37 +0900 Subject: [PATCH 1/2] auth, etcdserver: allow users to know their roles and permissions Current UserGet() and RoleGet() RPCs require admin permission. It means that users cannot know which roles they belong to and what permissions the roles have. This commit change the semantics and now users can know their roles and permissions. --- auth/store.go | 23 +++++++++++++++++++++++ etcdserver/apply_auth.go | 26 ++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/auth/store.go b/auth/store.go index e4ca093a100..4efa5bdd1d7 100644 --- a/auth/store.go +++ b/auth/store.go @@ -165,6 +165,9 @@ type AuthStore interface { // WithRoot generates and installs a token that can be used as a root credential WithRoot(ctx context.Context) context.Context + + // HasRole checks that user has role + HasRole(user, role string) bool } type TokenProvider interface { @@ -1097,3 +1100,23 @@ func (as *authStore) WithRoot(ctx context.Context) context.Context { tokenMD := metadata.New(mdMap) return metadata.NewContext(ctx, tokenMD) } + +func (as *authStore) HasRole(user, role string) bool { + tx := as.be.BatchTx() + tx.Lock() + defer tx.Unlock() + + u := getUser(tx, user) + if u == nil { + plog.Warningf("tried to check user %s has role %s, but user %s doesn't exist", user, role, user) + return false + } + + for _, r := range u.Roles { + if role == r { + return true + } + } + + return false +} diff --git a/etcdserver/apply_auth.go b/etcdserver/apply_auth.go index d4d881ab045..ec9391435da 100644 --- a/etcdserver/apply_auth.go +++ b/etcdserver/apply_auth.go @@ -189,6 +189,28 @@ func (aa *authApplierV3) checkLeasePuts(leaseID lease.LeaseID) error { return nil } +func (aa *authApplierV3) UserGet(r *pb.AuthUserGetRequest) (*pb.AuthUserGetResponse, error) { + err := aa.as.IsAdminPermitted(&aa.authInfo) + if err != nil && r.Name != aa.authInfo.Username { + aa.authInfo.Username = "" + aa.authInfo.Revision = 0 + return &pb.AuthUserGetResponse{}, err + } + + return aa.applierV3.UserGet(r) +} + +func (aa *authApplierV3) RoleGet(r *pb.AuthRoleGetRequest) (*pb.AuthRoleGetResponse, error) { + err := aa.as.IsAdminPermitted(&aa.authInfo) + if err != nil && !aa.as.HasRole(aa.authInfo.Username, r.Role) { + aa.authInfo.Username = "" + aa.authInfo.Revision = 0 + return &pb.AuthRoleGetResponse{}, err + } + + return aa.applierV3.RoleGet(r) +} + func needAdminPermission(r *pb.InternalRaftRequest) bool { switch { case r.AuthEnable != nil: @@ -203,16 +225,12 @@ func needAdminPermission(r *pb.InternalRaftRequest) bool { return true case r.AuthUserGrantRole != nil: return true - case r.AuthUserGet != nil: - return true case r.AuthUserRevokeRole != nil: return true case r.AuthRoleAdd != nil: return true case r.AuthRoleGrantPermission != nil: return true - case r.AuthRoleGet != nil: - return true case r.AuthRoleRevokePermission != nil: return true case r.AuthRoleDelete != nil: From db595887cf3f7df2be0d19baba579b925e8daac1 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Mon, 26 Jun 2017 22:20:35 -0700 Subject: [PATCH 2/2] e2e: add test cases for getting user and role information of user itself --- e2e/ctl_v3_auth_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/e2e/ctl_v3_auth_test.go b/e2e/ctl_v3_auth_test.go index d9f0123e794..ff823a0dc3e 100644 --- a/e2e/ctl_v3_auth_test.go +++ b/e2e/ctl_v3_auth_test.go @@ -41,6 +41,7 @@ func TestCtlV3AuthFromKeyPerm(t *testing.T) { testCtl(t, authTestFromKeyPer func TestCtlV3AuthAndWatch(t *testing.T) { testCtl(t, authTestWatch) } func TestCtlV3AuthRoleGet(t *testing.T) { testCtl(t, authTestRoleGet) } +func TestCtlV3AuthUserGet(t *testing.T) { testCtl(t, authTestUserGet) } func TestCtlV3AuthRoleList(t *testing.T) { testCtl(t, authTestRoleList) } func authEnableTest(cx ctlCtx) { @@ -758,6 +759,51 @@ func authTestRoleGet(cx ctlCtx) { if err := spawnWithExpects(append(cx.PrefixArgs(), "role", "get", "test-role"), expected...); err != nil { cx.t.Fatal(err) } + + // test-user can get the information of test-role because it belongs to the role + cx.user, cx.pass = "test-user", "pass" + if err := spawnWithExpects(append(cx.PrefixArgs(), "role", "get", "test-role"), expected...); err != nil { + cx.t.Fatal(err) + } + + // test-user cannot get the information of root because it doesn't belong to the role + expected = []string{ + "Error: etcdserver: permission denied", + } + if err := spawnWithExpects(append(cx.PrefixArgs(), "role", "get", "root"), expected...); err != nil { + cx.t.Fatal(err) + } +} + +func authTestUserGet(cx ctlCtx) { + if err := authEnable(cx); err != nil { + cx.t.Fatal(err) + } + cx.user, cx.pass = "root", "root" + authSetupTestUser(cx) + + expected := []string{ + "User: test-user", + "Roles: test-role", + } + + if err := spawnWithExpects(append(cx.PrefixArgs(), "user", "get", "test-user"), expected...); err != nil { + cx.t.Fatal(err) + } + + // test-user can get the information of test-user itself + cx.user, cx.pass = "test-user", "pass" + if err := spawnWithExpects(append(cx.PrefixArgs(), "user", "get", "test-user"), expected...); err != nil { + cx.t.Fatal(err) + } + + // test-user cannot get the information of root + expected = []string{ + "Error: etcdserver: permission denied", + } + if err := spawnWithExpects(append(cx.PrefixArgs(), "user", "get", "root"), expected...); err != nil { + cx.t.Fatal(err) + } } func authTestRoleList(cx ctlCtx) {