1. origin
  2. Commit 1d15a871e55fb3bf1e40fa986a38bf9fbcfcd372
Browse code

only resolve roles for bindings that matter

deads2k authored on 2015/03/05 22:28:58
Showing 2 changed files
pkg/authorization/rulevalidation/find_rules.go
History View file @ 1d15a87
... ... 
@@ -42,7 +42,6 @@ type BindingLister interface {
42 42 
 	ListPolicyBindings(ctx kapi.Context, labels, fields klabels.Selector) (*authorizationapi.PolicyBindingList, error)
43 43 
 }
44 44
45 
-// getPolicy provides a point for easy caching
46 45 
 func (a *DefaultRuleResolver) getPolicy(ctx kapi.Context) (*authorizationapi.Policy, error) {
47 46 
 	policy, err := a.policyGetter.GetPolicy(ctx, authorizationapi.PolicyName)
48 47 
 	if err != nil && !strings.Contains(err.Error(), "not found") {
... ... 
@@ -52,7 +51,6 @@ func (a *DefaultRuleResolver) getPolicy(ctx kapi.Context) (*authorizationapi.Pol
52 52 
 	return policy, nil
53 53 
 }
54 54
55 
-// getPolicyBindings provides a point for easy caching
56 55 
 func (a *DefaultRuleResolver) getPolicyBindings(ctx kapi.Context) ([]authorizationapi.PolicyBinding, error) {
57 56 
 	policyBindingList, err := a.bindingLister.ListPolicyBindings(ctx, klabels.Everything(), klabels.Everything())
58 57 
 	if err != nil {
... ... 
@@ -62,7 +60,6 @@ func (a *DefaultRuleResolver) getPolicyBindings(ctx kapi.Context) ([]authorizati
62 62 
 	return policyBindingList.Items, nil
63 63 
 }
64 64
65 
-// getRoleBindings provides a point for easy caching
66 65 
 func (a *DefaultRuleResolver) GetRoleBindings(ctx kapi.Context) ([]authorizationapi.RoleBinding, error) {
67 66 
 	policyBindings, err := a.getPolicyBindings(ctx)
68 67 
 	if err != nil {
... ... 
@@ -105,10 +102,18 @@ func (a *DefaultRuleResolver) GetEffectivePolicyRules(ctx kapi.Context) ([]autho
105 105 
 	if err != nil {
106 106 
 		return nil, err
107 107 
 	}
108 
+	user, exists := kapi.UserFrom(ctx)
109 
+	if !exists {
110 
+		return nil, errors.New("user missing from context")
111 
+	}
108 112
109 113 
 	errs := []error{}
110 114 
 	rules := make([]authorizationapi.PolicyRule, 0, len(roleBindings))
111 115 
 	for _, roleBinding := range roleBindings {
116 
+		if !appliesToUser(roleBinding.Users, roleBinding.Groups, user) {
117 
+			continue
118 
+		}
119 
+
112 120 
 		role, err := a.GetRole(roleBinding)
113 121 
 		if err != nil {
114 122 
 			errs = append(errs, err)
... ... 
@@ -116,21 +121,13 @@ func (a *DefaultRuleResolver) GetEffectivePolicyRules(ctx kapi.Context) ([]autho
116 116 
 		}
117 117
118 118 
 		for _, curr := range role.Rules {
119 
-			user, exists := kapi.UserFrom(ctx)
120 
-			if !exists {
121 
-				errs = append(errs, errors.New("user missing from context"))
122 
-			}
123 
-
124 
-			if doesApplyToUser(roleBinding.Users, roleBinding.Groups, user) {
125 
-				rules = append(rules, curr)
126 
-			}
119 
+			rules = append(rules, curr)
127 120 
 		}
128 121 
 	}
129 122
130 123 
 	return rules, kerrors.NewAggregate(errs)
131 124 
 }
132 
-
133 
-func doesApplyToUser(ruleUsers, ruleGroups util.StringSet, user user.Info) bool {
125 
+func appliesToUser(ruleUsers, ruleGroups util.StringSet, user user.Info) bool {
134 126 
 	if ruleUsers.Has(user.GetName()) {
135 127 
 		return true
136 128 
 	}
test/integration/authorization_test.go
History View file @ 1d15a87
... ... 
@@ -78,6 +78,50 @@ func TestRestrictedAccessForProjectAdmins(t *testing.T) {
78 78 
 	// }
79 79 
 }
80 80
81 
+func TestOnlyResolveRolesForBindingsThatMatter(t *testing.T) {
82 
+	startConfig, err := StartTestMaster()
83 
+	if err != nil {
84 
+		t.Fatalf("unexpected error: %v", err)
85 
+	}
86 
+
87 
+	openshiftClient, _, err := startConfig.GetOpenshiftClient()
88 
+	if err != nil {
89 
+		t.Errorf("unexpected error: %v", err)
90 
+	}
91 
+
92 
+	addValerie := &policy.AddUserOptions{
93 
+		RoleNamespace:    "master",
94 
+		RoleName:         "view",
95 
+		BindingNamespace: "master",
96 
+		Client:           openshiftClient,
97 
+		Users:            []string{"anypassword:valerie"},
98 
+	}
99 
+	if err := addValerie.Run(); err != nil {
100 
+		t.Errorf("unexpected error: %v", err)
101 
+	}
102 
+
103 
+	if err = openshiftClient.Roles("master").Delete("view"); err != nil {
104 
+		t.Errorf("unexpected error: %v", err)
105 
+	}
106 
+
107 
+	addEdgar := &policy.AddUserOptions{
108 
+		RoleNamespace:    "master",
109 
+		RoleName:         "edit",
110 
+		BindingNamespace: "master",
111 
+		Client:           openshiftClient,
112 
+		Users:            []string{"anypassword:edgar"},
113 
+	}
114 
+	if err := addEdgar.Run(); err != nil {
115 
+		t.Errorf("unexpected error: %v", err)
116 
+	}
117 
+
118 
+	// try to add Valerie to a non-existent role
119 
+	if err := addValerie.Run(); err == nil || err.Error() != "role view not found" {
120 
+		t.Errorf("expected error %v, got %v", "role view not found", err)
121 
+	}
122 
+
123 
+}
124 
+
81 125 
 // TODO this list should start collapsing as we continue to tighten access on generated system ids
82 126 
 var globalClusterAdminUsers = util.NewStringSet("system:kube-client", "system:openshift-client", "system:openshift-deployer")
83 127 
 var globalClusterAdminGroups = util.NewStringSet("system:cluster-admins")