etcdhttp: fix access check for multiple roles in auth

Check access for multiple roles should go through all roles.

etcdserver/etcdhttp/client_auth.go

@@ -97,9 +97,12 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b
 			continue
 		}
 		if recursive {
-			return role.HasRecursiveAccess(key, writeAccess)
+			if role.HasRecursiveAccess(key, writeAccess) {
+				return true
+			}
+		} else if role.HasKeyAccess(key, writeAccess) {
+			return true
 		}
-		return role.HasKeyAccess(key, writeAccess)
 	}
 	plog.Warningf("auth: invalid access for user %s on key %s.", username, key)
 	return false

etcdserver/etcdhttp/client_auth_test.go

@@ -38,7 +38,7 @@ func mustJSONRequest(t *testing.T, method string, p string, body string) *http.R
 
 type mockAuthStore struct {
 	user    *auth.User
-	role    *auth.Role
+	roles   map[string]*auth.Role
 	err     error
 	enabled bool
 }
@@ -59,10 +59,12 @@ func (s *mockAuthStore) UpdateUser(user auth.User) (auth.User, error) { return *
 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) GetRole(name string) (auth.Role, error) { return *s.roles[name], 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) UpdateRole(role auth.Role) (auth.Role, error) {
+	return *s.roles[role.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 }
@@ -159,10 +161,12 @@ func TestAuthFlow(t *testing.T) {
 		{
 			req: mustJSONRequest(t, "GET", "roles/manager", ""),
 			store: mockAuthStore{
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"manager": {
 						Role: "manager",
 					},
 				},
+			},
 			wcode: http.StatusOK,
 			wbody: `{"role":"manager","permissions":{"kv":{"read":null,"write":null}}}`,
 		},
@@ -181,10 +185,12 @@ func TestAuthFlow(t *testing.T) {
 		{
 			req: mustJSONRequest(t, "PUT", "roles/manager", `{"role":"manager","revoke":{"kv":{"read":["foo"],"write":[]}}}`),
 			store: mockAuthStore{
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"manager": {
 						Role: "manager",
 					},
 				},
+			},
 			wcode: http.StatusOK,
 			wbody: `{"role":"manager","permissions":{"kv":{"read":null,"write":null}}}`,
 		},
@@ -223,10 +229,12 @@ func TestAuthFlow(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"root"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"root": {
 						Role: "root",
 					},
 				},
+			},
 			wcode: http.StatusOK,
 			wbody: ``,
 		},
@@ -279,9 +287,11 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"root"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"root": {
 						Role: "root",
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            true,
@@ -297,7 +307,8 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"foorole"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"foorole": {
 						Role: "foorole",
 						Permissions: auth.Permissions{
 							KV: auth.RWPermission{
@@ -306,6 +317,7 @@ func TestPrefixAccess(t *testing.T) {
 							},
 						},
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            false,
@@ -321,7 +333,8 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"foorole"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"foorole": {
 						Role: "foorole",
 						Permissions: auth.Permissions{
 							KV: auth.RWPermission{
@@ -330,6 +343,7 @@ func TestPrefixAccess(t *testing.T) {
 							},
 						},
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            false,
@@ -345,7 +359,8 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"foorole"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"foorole": {
 						Role: "foorole",
 						Permissions: auth.Permissions{
 							KV: auth.RWPermission{
@@ -354,6 +369,7 @@ func TestPrefixAccess(t *testing.T) {
 							},
 						},
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            false,
@@ -381,7 +397,8 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"foorole"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"guest": {
 						Role: "guest",
 						Permissions: auth.Permissions{
 							KV: auth.RWPermission{
@@ -390,6 +407,7 @@ func TestPrefixAccess(t *testing.T) {
 							},
 						},
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            false,
@@ -405,7 +423,8 @@ func TestPrefixAccess(t *testing.T) {
 					Password: goodPassword,
 					Roles:    []string{"foorole"},
 				},
-				role: &auth.Role{
+				roles: map[string]*auth.Role{
+					"guest": {
 						Role: "guest",
 						Permissions: auth.Permissions{
 							KV: auth.RWPermission{
@@ -414,12 +433,43 @@ func TestPrefixAccess(t *testing.T) {
 							},
 						},
 					},
+				},
 				enabled: true,
 			},
 			hasRoot:            false,
 			hasKeyPrefixAccess: false,
 			hasRecursiveAccess: false,
 		},
+		// check access for multiple roles
+		{
+			key: "/foo",
+			req: mustAuthRequest("GET", "user", "good"),
+			store: &mockAuthStore{
+				user: &auth.User{
+					User:     "user",
+					Password: goodPassword,
+					Roles:    []string{"role1", "role2"},
+				},
+				roles: map[string]*auth.Role{
+					"role1": {
+						Role: "role1",
+					},
+					"role2": {
+						Role: "role2",
+						Permissions: auth.Permissions{
+							KV: auth.RWPermission{
+								Read:  []string{"/foo"},
+								Write: []string{"/foo"},
+							},
+						},
+					},
+				},
+				enabled: true,
+			},
+			hasRoot:            false,
+			hasKeyPrefixAccess: true,
+			hasRecursiveAccess: false,
+		},
 	}
 
 	for i, tt := range table {