From 411922f66cdbd95f4c233c31ea48d567db0b048b Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 30 Jun 2016 11:53:15 -0700 Subject: [PATCH] rbac authorizer: include verb in non-resource url requests --- pkg/apis/rbac/validation/policy_comparator.go | 6 +- .../rbac/validation/policy_comparator_test.go | 2 +- plugin/pkg/auth/authorizer/rbac/rbac.go | 1 + plugin/pkg/auth/authorizer/rbac/rbac_test.go | 59 +++++++++++++++++-- 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/pkg/apis/rbac/validation/policy_comparator.go b/pkg/apis/rbac/validation/policy_comparator.go index be9318d4d5a..756407a3795 100644 --- a/pkg/apis/rbac/validation/policy_comparator.go +++ b/pkg/apis/rbac/validation/policy_comparator.go @@ -69,9 +69,11 @@ func breakdownRule(rule rbac.PolicyRule) []rbac.PolicyRule { } } - // Non-resource URLs are unique because they don't combine with other policy rule fields. + // Non-resource URLs are unique because they only combine with verbs. for _, nonResourceURL := range rule.NonResourceURLs { - subrules = append(subrules, rbac.PolicyRule{NonResourceURLs: []string{nonResourceURL}}) + for _, verb := range rule.Verbs { + subrules = append(subrules, rbac.PolicyRule{NonResourceURLs: []string{nonResourceURL}, Verbs: []string{verb}}) + } } return subrules diff --git a/pkg/apis/rbac/validation/policy_comparator_test.go b/pkg/apis/rbac/validation/policy_comparator_test.go index 4de7fb869a8..f029d77d59c 100644 --- a/pkg/apis/rbac/validation/policy_comparator_test.go +++ b/pkg/apis/rbac/validation/policy_comparator_test.go @@ -333,7 +333,7 @@ func TestCoversNonResourceURLsWithOtherFieldsFailure(t *testing.T) { }, expectedCovered: false, - expectedUncoveredRules: []rbac.PolicyRule{{NonResourceURLs: []string{"/apis"}}}, + expectedUncoveredRules: []rbac.PolicyRule{{NonResourceURLs: []string{"/apis"}, Verbs: []string{"get"}}}, }.test(t) } diff --git a/plugin/pkg/auth/authorizer/rbac/rbac.go b/plugin/pkg/auth/authorizer/rbac/rbac.go index d46e178d27d..aee5cbd90a9 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac.go @@ -58,6 +58,7 @@ func (r *RBACAuthorizer) Authorize(attr authorizer.Attributes) error { } } else { requestedRule = rbac.PolicyRule{ + Verbs: []string{attr.GetVerb()}, NonResourceURLs: []string{attr.GetPath()}, } } diff --git a/plugin/pkg/auth/authorizer/rbac/rbac_test.go b/plugin/pkg/auth/authorizer/rbac/rbac_test.go index 025e2c856a8..e4111f1668a 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac_test.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac_test.go @@ -25,13 +25,15 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/apis/rbac/validation" "k8s.io/kubernetes/pkg/auth/authorizer" + "k8s.io/kubernetes/pkg/auth/user" ) -func newRule(verbs, apiGroups, resources string) rbac.PolicyRule { +func newRule(verbs, apiGroups, resources, nonResourceURLs string) rbac.PolicyRule { return rbac.PolicyRule{ - Verbs: strings.Split(verbs, ","), - APIGroups: strings.Split(apiGroups, ","), - Resources: strings.Split(resources, ","), + Verbs: strings.Split(verbs, ","), + APIGroups: strings.Split(apiGroups, ","), + Resources: strings.Split(resources, ","), + NonResourceURLs: strings.Split(nonResourceURLs, ","), } } @@ -48,6 +50,23 @@ const ( bindToClusterRole uint16 = 0x1 ) +func newClusterRoleBinding(roleName string, subjects ...string) rbac.ClusterRoleBinding { + r := rbac.ClusterRoleBinding{ + ObjectMeta: api.ObjectMeta{}, + RoleRef: api.ObjectReference{ + Kind: "ClusterRole", // ClusterRoleBindings can only refer to ClusterRole + Name: roleName, + }, + } + + r.Subjects = make([]rbac.Subject, len(subjects)) + for i, subject := range subjects { + split := strings.SplitN(subject, ":", 2) + r.Subjects[i].Kind, r.Subjects[i].Name = split[0], split[1] + } + return r +} + func newRoleBinding(namespace, roleName string, bindType uint16, subjects ...string) rbac.RoleBinding { r := rbac.RoleBinding{ObjectMeta: api.ObjectMeta{Namespace: namespace}} @@ -107,7 +126,7 @@ func TestAuthorizer(t *testing.T) { }{ { clusterRoles: []rbac.ClusterRole{ - newClusterRole("admin", newRule("*", "*", "*")), + newClusterRole("admin", newRule("*", "*", "*", "*")), }, roleBindings: []rbac.RoleBinding{ newRoleBinding("ns1", "admin", bindToClusterRole, "User:admin", "Group:admins"), @@ -126,6 +145,36 @@ func TestAuthorizer(t *testing.T) { &defaultAttributes{"admin", "admins", "GET", "Nodes", "", ""}, }, }, + { + // Non-resource-url tests + clusterRoles: []rbac.ClusterRole{ + newClusterRole("non-resource-url-getter", newRule("get", "", "", "/apis")), + newClusterRole("non-resource-url", newRule("*", "", "", "/apis")), + }, + clusterRoleBindings: []rbac.ClusterRoleBinding{ + newClusterRoleBinding("non-resource-url-getter", "User:foo", "Group:bar"), + newClusterRoleBinding("non-resource-url", "User:admin", "Group:admin"), + }, + shouldPass: []authorizer.Attributes{ + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "foo"}, Verb: "get", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"bar"}}, Verb: "get", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "admin"}, Verb: "get", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"admin"}}, Verb: "get", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "admin"}, Verb: "watch", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"admin"}}, Verb: "watch", Path: "/apis"}, + }, + shouldFail: []authorizer.Attributes{ + // wrong verb + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "foo"}, Verb: "watch", Path: "/apis"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"bar"}}, Verb: "watch", Path: "/apis"}, + + // wrong path + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "foo"}, Verb: "get", Path: "/api/v1"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"bar"}}, Verb: "get", Path: "/api/v1"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "admin"}, Verb: "get", Path: "/api/v1"}, + authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"admin"}}, Verb: "get", Path: "/api/v1"}, + }, + }, } for i, tt := range tests { ruleResolver := validation.NewTestRuleResolver(tt.roles, tt.roleBindings, tt.clusterRoles, tt.clusterRoleBindings)