rbac authorizer: support non-resource urls with stars ("/apis/*")

pkg/apis/rbac/validation/policy_comparator.go

@@ -16,7 +16,11 @@ limitations under the License.
 
 package validation
 
-import "k8s.io/kubernetes/pkg/apis/rbac"
+import (
+	"strings"
+
+	"k8s.io/kubernetes/pkg/apis/rbac"
+)
 
 // Covers determines whether or not the ownerRules cover the servantRules in terms of allowed actions.
 // It returns whether or not the ownerRules cover and a list of the rules that the ownerRules do not cover.
@@ -101,6 +105,29 @@ func hasAll(set, contains []string) bool {
 	return true
 }
 
+func nonResourceURLsCoversAll(set, covers []string) bool {
+	for _, path := range covers {
+		covered := false
+		for _, owner := range set {
+			if nonResourceURLCovers(owner, path) {
+				covered = true
+				break
+			}
+		}
+		if !covered {
+			return false
+		}
+	}
+	return true
+}
+
+func nonResourceURLCovers(ownerPath, subPath string) bool {
+	if ownerPath == subPath {
+		return true
+	}
+	return strings.HasSuffix(ownerPath, "*") && strings.HasPrefix(subPath, strings.TrimRight(ownerPath, "*"))
+}
+
 // ruleCovers determines whether the ownerRule (which may have multiple verbs, resources, and resourceNames) covers
 // the subrule (which may only contain at most one verb, resource, and resourceName)
 func ruleCovers(ownerRule, subRule rbac.PolicyRule) bool {
@@ -108,7 +135,7 @@ func ruleCovers(ownerRule, subRule rbac.PolicyRule) bool {
 	verbMatches := has(ownerRule.Verbs, rbac.VerbAll) || hasAll(ownerRule.Verbs, subRule.Verbs)
 	groupMatches := has(ownerRule.APIGroups, rbac.APIGroupAll) || hasAll(ownerRule.APIGroups, subRule.APIGroups)
 	resourceMatches := has(ownerRule.Resources, rbac.ResourceAll) || hasAll(ownerRule.Resources, subRule.Resources)
-	nonResourceURLMatches := has(ownerRule.NonResourceURLs, rbac.NonResourceAll) || hasAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs)
+	nonResourceURLMatches := nonResourceURLsCoversAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs)
 
 	resourceNameMatches := false
 

pkg/apis/rbac/validation/policy_comparator_test.go

@@ -284,10 +284,10 @@ func TestCoversEnumerationNotCoveringResourceNameEmpty(t *testing.T) {
 func TestCoversNonResourceURLs(t *testing.T) {
 	escalationTest{
 		ownerRules: []rbac.PolicyRule{
-			{NonResourceURLs: []string{"/apis"}},
+			{NonResourceURLs: []string{"/apis"}, Verbs: []string{"*"}},
 		},
 		servantRules: []rbac.PolicyRule{
-			{NonResourceURLs: []string{"/apis"}},
+			{NonResourceURLs: []string{"/apis"}, Verbs: []string{"*"}},
 		},
 
 		expectedCovered:        true,
@@ -298,10 +298,40 @@ func TestCoversNonResourceURLs(t *testing.T) {
 func TestCoversNonResourceURLsStar(t *testing.T) {
 	escalationTest{
 		ownerRules: []rbac.PolicyRule{
-			{NonResourceURLs: []string{"*"}},
+			{NonResourceURLs: []string{"*"}, Verbs: []string{"*"}},
 		},
 		servantRules: []rbac.PolicyRule{
-			{NonResourceURLs: []string{"/apis", "/apis/v1", "/"}},
+			{NonResourceURLs: []string{"/apis", "/apis/v1", "/"}, Verbs: []string{"*"}},
+		},
+
+		expectedCovered:        true,
+		expectedUncoveredRules: []rbac.PolicyRule{},
+	}.test(t)
+}
+
+func TestCoversNonResourceURLsStarAfterPrefixDoesntCover(t *testing.T) {
+	escalationTest{
+		ownerRules: []rbac.PolicyRule{
+			{NonResourceURLs: []string{"/apis/*"}, Verbs: []string{"*"}},
+		},
+		servantRules: []rbac.PolicyRule{
+			{NonResourceURLs: []string{"/apis", "/apis/v1"}, Verbs: []string{"get"}},
+		},
+
+		expectedCovered: false,
+		expectedUncoveredRules: []rbac.PolicyRule{
+			{NonResourceURLs: []string{"/apis"}, Verbs: []string{"get"}},
+		},
+	}.test(t)
+}
+
+func TestCoversNonResourceURLsStarAfterPrefix(t *testing.T) {
+	escalationTest{
+		ownerRules: []rbac.PolicyRule{
+			{NonResourceURLs: []string{"/apis/*"}, Verbs: []string{"*"}},
+		},
+		servantRules: []rbac.PolicyRule{
+			{NonResourceURLs: []string{"/apis/v1/foo", "/apis/v1"}, Verbs: []string{"get"}},
 		},
 
 		expectedCovered:        true,

pkg/apis/rbac/validation/validation_test.go

@@ -219,3 +219,28 @@ func TestValidateRole(t *testing.T) {
 		}
 	}
 }
+
+func TestNonResourceURLCovers(t *testing.T) {
+	tests := []struct {
+		owner     string
+		requested string
+		want      bool
+	}{
+		{"*", "/api", true},
+		{"/api", "/api", true},
+		{"/apis", "/api", false},
+		{"/api/v1", "/api", false},
+		{"/api/v1", "/api/v1", true},
+		{"/api/*", "/api/v1", true},
+		{"/api/*", "/api", false},
+		{"/api/*/*", "/api/v1", false},
+		{"/*/v1/*", "/api/v1", false},
+	}
+
+	for _, tc := range tests {
+		got := nonResourceURLCovers(tc.owner, tc.requested)
+		if got != tc.want {
+			t.Errorf("nonResourceURLCovers(%q, %q): want=(%t), got=(%t)", tc.owner, tc.requested, tc.want, got)
+		}
+	}
+}

plugin/pkg/auth/authorizer/rbac/rbac_test.go

@@ -150,10 +150,12 @@ func TestAuthorizer(t *testing.T) {
 			clusterRoles: []rbac.ClusterRole{
 				newClusterRole("non-resource-url-getter", newRule("get", "", "", "/apis")),
 				newClusterRole("non-resource-url", newRule("*", "", "", "/apis")),
+				newClusterRole("non-resource-url-prefix", newRule("get", "", "", "/apis/*")),
 			},
 			clusterRoleBindings: []rbac.ClusterRoleBinding{
 				newClusterRoleBinding("non-resource-url-getter", "User:foo", "Group:bar"),
 				newClusterRoleBinding("non-resource-url", "User:admin", "Group:admin"),
+				newClusterRoleBinding("non-resource-url-prefix", "User:prefixed", "Group:prefixed"),
 			},
 			shouldPass: []authorizer.Attributes{
 				authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "foo"}, Verb: "get", Path: "/apis"},
@@ -162,6 +164,11 @@ func TestAuthorizer(t *testing.T) {
 				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"},
+
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "prefixed"}, Verb: "get", Path: "/apis/v1"},
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"prefixed"}}, Verb: "get", Path: "/apis/v1"},
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "prefixed"}, Verb: "get", Path: "/apis/v1/foobar"},
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"prefixed"}}, Verb: "get", Path: "/apis/v1/foorbar"},
 			},
 			shouldFail: []authorizer.Attributes{
 				// wrong verb
@@ -173,6 +180,10 @@ func TestAuthorizer(t *testing.T) {
 				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"},
+
+				// not covered by prefix
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "prefixed"}, Verb: "get", Path: "/api/v1"},
+				authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"prefixed"}}, Verb: "get", Path: "/api/v1"},
 			},
 		},
 	}