Browse code

Refactor build strategy permissions into distinct roles

Jordan Liggitt authored on 2016/04/16 04:50:13
Showing 7 changed files
... ...
@@ -75,16 +75,17 @@ func (a *buildByStrategy) Validate() error {
75 75
 	return nil
76 76
 }
77 77
 
78
-func resourceForStrategyType(strategy buildapi.BuildStrategy) unversioned.GroupResource {
78
+func resourceForStrategyType(strategy buildapi.BuildStrategy) (unversioned.GroupResource, error) {
79 79
 	switch {
80 80
 	case strategy.DockerStrategy != nil:
81
-		return buildapi.Resource(authorizationapi.DockerBuildResource)
81
+		return buildapi.Resource(authorizationapi.DockerBuildResource), nil
82 82
 	case strategy.CustomStrategy != nil:
83
-		return buildapi.Resource(authorizationapi.CustomBuildResource)
83
+		return buildapi.Resource(authorizationapi.CustomBuildResource), nil
84 84
 	case strategy.SourceStrategy != nil:
85
-		return buildapi.Resource(authorizationapi.SourceBuildResource)
85
+		return buildapi.Resource(authorizationapi.SourceBuildResource), nil
86
+	default:
87
+		return unversioned.GroupResource{}, fmt.Errorf("unrecognized build strategy: %#v", strategy)
86 88
 	}
87
-	return unversioned.GroupResource{}
88 89
 }
89 90
 
90 91
 func resourceName(objectMeta kapi.ObjectMeta) string {
... ...
@@ -96,11 +97,15 @@ func resourceName(objectMeta kapi.ObjectMeta) string {
96 96
 
97 97
 func (a *buildByStrategy) checkBuildAuthorization(build *buildapi.Build, attr admission.Attributes) error {
98 98
 	strategy := build.Spec.Strategy
99
+	resource, err := resourceForStrategyType(strategy)
100
+	if err != nil {
101
+		return err
102
+	}
99 103
 	subjectAccessReview := &authorizationapi.LocalSubjectAccessReview{
100 104
 		Action: authorizationapi.AuthorizationAttributes{
101 105
 			Verb:         "create",
102
-			Group:        resourceForStrategyType(strategy).Group,
103
-			Resource:     resourceForStrategyType(strategy).Resource,
106
+			Group:        resource.Group,
107
+			Resource:     resource.Resource,
104 108
 			Content:      build,
105 109
 			ResourceName: resourceName(build.ObjectMeta),
106 110
 		},
... ...
@@ -112,11 +117,15 @@ func (a *buildByStrategy) checkBuildAuthorization(build *buildapi.Build, attr ad
112 112
 
113 113
 func (a *buildByStrategy) checkBuildConfigAuthorization(buildConfig *buildapi.BuildConfig, attr admission.Attributes) error {
114 114
 	strategy := buildConfig.Spec.Strategy
115
+	resource, err := resourceForStrategyType(strategy)
116
+	if err != nil {
117
+		return err
118
+	}
115 119
 	subjectAccessReview := &authorizationapi.LocalSubjectAccessReview{
116 120
 		Action: authorizationapi.AuthorizationAttributes{
117 121
 			Verb:         "create",
118
-			Group:        resourceForStrategyType(strategy).Group,
119
-			Resource:     resourceForStrategyType(strategy).Resource,
122
+			Group:        resource.Group,
123
+			Resource:     resource.Resource,
120 124
 			Content:      buildConfig,
121 125
 			ResourceName: resourceName(buildConfig.ObjectMeta),
122 126
 		},
... ...
@@ -63,6 +63,10 @@ const (
63 63
 	RegistryViewerRoleName = "registry-viewer"
64 64
 	RegistryEditorRoleName = "registry-editor"
65 65
 
66
+	BuildStrategyDockerRoleName = "system:build-strategy-docker"
67
+	BuildStrategyCustomRoleName = "system:build-strategy-custom"
68
+	BuildStrategySourceRoleName = "system:build-strategy-source"
69
+
66 70
 	ImageAuditorRoleName      = "system:image-auditor"
67 71
 	ImagePullerRoleName       = "system:image-puller"
68 72
 	ImagePusherRoleName       = "system:image-pusher"
... ...
@@ -114,5 +118,9 @@ const (
114 114
 	RegistryViewerRoleBindingName    = RegistryViewerRoleName + "s"
115 115
 	RegistryEditorRoleBindingName    = RegistryEditorRoleName + "s"
116 116
 
117
+	BuildStrategyDockerRoleBindingName = BuildStrategyDockerRoleName + "-binding"
118
+	BuildStrategyCustomRoleBindingName = BuildStrategyCustomRoleName + "-binding"
119
+	BuildStrategySourceRoleBindingName = BuildStrategySourceRoleName + "-binding"
120
+
117 121
 	OpenshiftSharedResourceViewRoleBindingName = OpenshiftSharedResourceViewRoleName + "s"
118 122
 )
... ...
@@ -111,6 +111,43 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
111 111
 		},
112 112
 		{
113 113
 			ObjectMeta: kapi.ObjectMeta{
114
+				Name: BuildStrategyDockerRoleName,
115
+			},
116
+			Rules: []authorizationapi.PolicyRule{
117
+				{
118
+					APIGroups: []string{api.GroupName},
119
+					Verbs:     sets.NewString("create"),
120
+					Resources: sets.NewString(authorizationapi.DockerBuildResource),
121
+				},
122
+			},
123
+		},
124
+		{
125
+			ObjectMeta: kapi.ObjectMeta{
126
+				Name: BuildStrategyCustomRoleName,
127
+			},
128
+			Rules: []authorizationapi.PolicyRule{
129
+				{
130
+					APIGroups: []string{api.GroupName},
131
+					Verbs:     sets.NewString("create"),
132
+					Resources: sets.NewString(authorizationapi.CustomBuildResource),
133
+				},
134
+			},
135
+		},
136
+		{
137
+			ObjectMeta: kapi.ObjectMeta{
138
+				Name: BuildStrategySourceRoleName,
139
+			},
140
+			Rules: []authorizationapi.PolicyRule{
141
+				{
142
+					APIGroups: []string{api.GroupName},
143
+					Verbs:     sets.NewString("create"),
144
+					Resources: sets.NewString(authorizationapi.SourceBuildResource),
145
+				},
146
+			},
147
+		},
148
+
149
+		{
150
+			ObjectMeta: kapi.ObjectMeta{
114 151
 				Name: AdminRoleName,
115 152
 			},
116 153
 			Rules: []authorizationapi.PolicyRule{
... ...
@@ -132,9 +169,6 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
132 132
 						authorizationapi.OpenshiftExposedGroupName,
133 133
 						authorizationapi.PermissionGrantingGroupName,
134 134
 						"projects",
135
-						authorizationapi.DockerBuildResource,
136
-						authorizationapi.SourceBuildResource,
137
-						authorizationapi.CustomBuildResource,
138 135
 						"deploymentconfigs/scale",
139 136
 						"imagestreams/secrets",
140 137
 					),
... ...
@@ -196,9 +230,6 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
196 196
 					Verbs:     sets.NewString("get", "list", "watch", "create", "update", "patch", "delete", "deletecollection"),
197 197
 					Resources: sets.NewString(
198 198
 						authorizationapi.OpenshiftExposedGroupName,
199
-						authorizationapi.DockerBuildResource,
200
-						authorizationapi.SourceBuildResource,
201
-						authorizationapi.CustomBuildResource,
202 199
 						"deploymentconfigs/scale",
203 200
 						"imagestreams/secrets",
204 201
 					),
... ...
@@ -942,5 +973,24 @@ func GetBootstrapClusterRoleBindings() []authorizationapi.ClusterRoleBinding {
942 942
 				{Kind: authorizationapi.SystemGroupKind, Name: UnauthenticatedGroup},
943 943
 			},
944 944
 		},
945
+
946
+		// Allow all build strategies by default.
947
+		// Cluster admins can remove these role bindings, and the reconcile-cluster-role-bindings command
948
+		// run during an upgrade won't re-add the "system:authenticated" group
949
+		{
950
+			ObjectMeta: kapi.ObjectMeta{Name: BuildStrategyDockerRoleBindingName},
951
+			RoleRef:    kapi.ObjectReference{Name: BuildStrategyDockerRoleName},
952
+			Subjects:   []kapi.ObjectReference{{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup}},
953
+		},
954
+		{
955
+			ObjectMeta: kapi.ObjectMeta{Name: BuildStrategyCustomRoleBindingName},
956
+			RoleRef:    kapi.ObjectReference{Name: BuildStrategyCustomRoleName},
957
+			Subjects:   []kapi.ObjectReference{{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup}},
958
+		},
959
+		{
960
+			ObjectMeta: kapi.ObjectMeta{Name: BuildStrategySourceRoleBindingName},
961
+			RoleRef:    kapi.ObjectReference{Name: BuildStrategySourceRoleName},
962
+			Subjects:   []kapi.ObjectReference{{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup}},
963
+		},
945 964
 	}
946 965
 }
... ...
@@ -35,6 +35,28 @@ os::cmd::expect_success_and_not_text 'oc get rolebinding/cluster-admin --no-head
35 35
 os::cmd::expect_success 'oc policy remove-group system:unauthenticated'
36 36
 os::cmd::expect_success 'oc policy remove-user system:no-user'
37 37
 
38
+
39
+os::cmd::expect_success 'oc policy add-role-to-user admin namespaced-user'
40
+# Ensure the user has create permissions on builds, but that build strategy permissions are granted through the authenticated users group
41
+os::cmd::try_until_text              'oadm policy who-can create builds' 'namespaced-user'
42
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/docker' 'namespaced-user'
43
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/custom' 'namespaced-user'
44
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/source' 'namespaced-user'
45
+os::cmd::expect_success_and_text     'oadm policy who-can create builds/docker' 'system:authenticated'
46
+os::cmd::expect_success_and_text     'oadm policy who-can create builds/custom' 'system:authenticated'
47
+os::cmd::expect_success_and_text     'oadm policy who-can create builds/source' 'system:authenticated'
48
+# if this method for removing access to docker/custom/source builds changes, docs need to be updated as well
49
+os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-custom system:authenticated'
50
+os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-docker system:authenticated'
51
+os::cmd::expect_success 'oadm policy remove-cluster-role-from-group system:build-strategy-source system:authenticated'
52
+# ensure build strategy permissions no longer exist
53
+os::cmd::try_until_failure           'oadm policy who-can create builds/source | grep system:authenticated'
54
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/docker' 'system:authenticated'
55
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/custom' 'system:authenticated'
56
+os::cmd::expect_success_and_not_text 'oadm policy who-can create builds/source' 'system:authenticated'
57
+os::cmd::expect_success 'oadm policy reconcile-cluster-role-bindings --confirm'
58
+
59
+
38 60
 # adjust the cluster-admin role to check defaulting and coverage checks
39 61
 # this is done here instead of an integration test because we need to make sure the actual yaml serializations work
40 62
 workingdir=$(mktemp -d)
... ...
@@ -210,5 +210,44 @@ items:
210 210
   - kind: SystemGroup
211 211
     name: system:unauthenticated
212 212
   userNames: null
213
+- apiVersion: v1
214
+  groupNames:
215
+  - system:authenticated
216
+  kind: ClusterRoleBinding
217
+  metadata:
218
+    creationTimestamp: null
219
+    name: system:build-strategy-docker-binding
220
+  roleRef:
221
+    name: system:build-strategy-docker
222
+  subjects:
223
+  - kind: SystemGroup
224
+    name: system:authenticated
225
+  userNames: null
226
+- apiVersion: v1
227
+  groupNames:
228
+  - system:authenticated
229
+  kind: ClusterRoleBinding
230
+  metadata:
231
+    creationTimestamp: null
232
+    name: system:build-strategy-custom-binding
233
+  roleRef:
234
+    name: system:build-strategy-custom
235
+  subjects:
236
+  - kind: SystemGroup
237
+    name: system:authenticated
238
+  userNames: null
239
+- apiVersion: v1
240
+  groupNames:
241
+  - system:authenticated
242
+  kind: ClusterRoleBinding
243
+  metadata:
244
+    creationTimestamp: null
245
+    name: system:build-strategy-source-binding
246
+  roleRef:
247
+    name: system:build-strategy-source
248
+  subjects:
249
+  - kind: SystemGroup
250
+    name: system:authenticated
251
+  userNames: null
213 252
 kind: List
214 253
 metadata: {}
... ...
@@ -162,6 +162,45 @@ items:
162 162
   kind: ClusterRole
163 163
   metadata:
164 164
     creationTimestamp: null
165
+    name: system:build-strategy-docker
166
+  rules:
167
+  - apiGroups:
168
+    - ""
169
+    attributeRestrictions: null
170
+    resources:
171
+    - builds/docker
172
+    verbs:
173
+    - create
174
+- apiVersion: v1
175
+  kind: ClusterRole
176
+  metadata:
177
+    creationTimestamp: null
178
+    name: system:build-strategy-custom
179
+  rules:
180
+  - apiGroups:
181
+    - ""
182
+    attributeRestrictions: null
183
+    resources:
184
+    - builds/custom
185
+    verbs:
186
+    - create
187
+- apiVersion: v1
188
+  kind: ClusterRole
189
+  metadata:
190
+    creationTimestamp: null
191
+    name: system:build-strategy-source
192
+  rules:
193
+  - apiGroups:
194
+    - ""
195
+    attributeRestrictions: null
196
+    resources:
197
+    - builds/source
198
+    verbs:
199
+    - create
200
+- apiVersion: v1
201
+  kind: ClusterRole
202
+  metadata:
203
+    creationTimestamp: null
165 204
     name: admin
166 205
   rules:
167 206
   - apiGroups:
... ...
@@ -203,10 +242,7 @@ items:
203 203
     - buildlogs
204 204
     - builds
205 205
     - builds/clone
206
-    - builds/custom
207
-    - builds/docker
208 206
     - builds/log
209
-    - builds/source
210 207
     - deploymentconfigrollbacks
211 208
     - deploymentconfigs
212 209
     - deploymentconfigs/log
... ...
@@ -383,10 +419,7 @@ items:
383 383
     - buildlogs
384 384
     - builds
385 385
     - builds/clone
386
-    - builds/custom
387
-    - builds/docker
388 386
     - builds/log
389
-    - builds/source
390 387
     - deploymentconfigrollbacks
391 388
     - deploymentconfigs
392 389
     - deploymentconfigs/log
... ...
@@ -202,33 +202,30 @@ func setupBuildStrategyTest(t *testing.T, includeControllers bool) (clusterAdmin
202 202
 
203 203
 func removeBuildStrategyRoleResources(t *testing.T, clusterAdminClient, projectAdminClient, projectEditorClient *client.Client) {
204 204
 	// remove resources from role so that certain build strategies are forbidden
205
-	removeBuildStrategyPrivileges(t, clusterAdminClient.ClusterRoles(), bootstrappolicy.EditRoleName)
206
-	if err := testutil.WaitForPolicyUpdate(projectEditorClient, testutil.Namespace(), "create", buildapi.Resource(authorizationapi.DockerBuildResource), false); err != nil {
207
-		t.Error(err)
205
+	for _, role := range []string{bootstrappolicy.BuildStrategyCustomRoleName, bootstrappolicy.BuildStrategyDockerRoleName, bootstrappolicy.BuildStrategySourceRoleName} {
206
+		remove := &policy.RoleModificationOptions{
207
+			RoleNamespace:       "",
208
+			RoleName:            role,
209
+			RoleBindingAccessor: policy.NewClusterRoleBindingAccessor(clusterAdminClient),
210
+			Groups:              []string{"system:authenticated"},
211
+		}
212
+		if err := remove.RemoveRole(); err != nil {
213
+			t.Fatalf("unexpected error: %v", err)
214
+		}
208 215
 	}
209 216
 
210
-	removeBuildStrategyPrivileges(t, clusterAdminClient.ClusterRoles(), bootstrappolicy.AdminRoleName)
211
-	if err := testutil.WaitForPolicyUpdate(projectAdminClient, testutil.Namespace(), "create", buildapi.Resource(authorizationapi.DockerBuildResource), false); err != nil {
217
+	if err := testutil.WaitForPolicyUpdate(projectEditorClient, testutil.Namespace(), "create", buildapi.Resource(authorizationapi.DockerBuildResource), false); err != nil {
212 218
 		t.Error(err)
213 219
 	}
214
-}
215
-
216
-func removeBuildStrategyPrivileges(t *testing.T, clusterRoleInterface client.ClusterRoleInterface, roleName string) {
217
-	role, err := clusterRoleInterface.Get(roleName)
218
-	if err != nil {
219
-		t.Errorf("unexpected error: %v", err)
220
-	}
221
-
222
-	for i := range role.Rules {
223
-		role.Rules[i].Resources.Delete(authorizationapi.DockerBuildResource, authorizationapi.SourceBuildResource, authorizationapi.CustomBuildResource)
220
+	if err := testutil.WaitForPolicyUpdate(projectEditorClient, testutil.Namespace(), "create", buildapi.Resource(authorizationapi.SourceBuildResource), false); err != nil {
221
+		t.Error(err)
224 222
 	}
225
-	if _, err := clusterRoleInterface.Update(role); err != nil {
226
-		t.Errorf("unexpected error: %v", err)
223
+	if err := testutil.WaitForPolicyUpdate(projectEditorClient, testutil.Namespace(), "create", buildapi.Resource(authorizationapi.CustomBuildResource), false); err != nil {
224
+		t.Error(err)
227 225
 	}
228
-
229 226
 }
230 227
 
231
-func strategyForType(strategy string) buildapi.BuildStrategy {
228
+func strategyForType(t *testing.T, strategy string) buildapi.BuildStrategy {
232 229
 	buildStrategy := buildapi.BuildStrategy{}
233 230
 	switch strategy {
234 231
 	case "docker":
... ...
@@ -239,6 +236,8 @@ func strategyForType(strategy string) buildapi.BuildStrategy {
239 239
 	case "source":
240 240
 		buildStrategy.SourceStrategy = &buildapi.SourceBuildStrategy{}
241 241
 		buildStrategy.SourceStrategy.From.Name = "builderimage:latest"
242
+	default:
243
+		t.Fatalf("unknown strategy: %#v", strategy)
242 244
 	}
243 245
 	return buildStrategy
244 246
 }
... ...
@@ -246,7 +245,7 @@ func strategyForType(strategy string) buildapi.BuildStrategy {
246 246
 func createBuild(t *testing.T, buildInterface client.BuildInterface, strategy string) (*buildapi.Build, error) {
247 247
 	build := &buildapi.Build{}
248 248
 	build.GenerateName = strings.ToLower(string(strategy)) + "-build-"
249
-	build.Spec.Strategy = strategyForType(strategy)
249
+	build.Spec.Strategy = strategyForType(t, strategy)
250 250
 	build.Spec.Source.Git = &buildapi.GitBuildSource{URI: "example.org"}
251 251
 
252 252
 	return buildInterface.Create(build)
... ...
@@ -260,7 +259,7 @@ func updateBuild(t *testing.T, buildInterface client.BuildInterface, build *buil
260 260
 func createBuildConfig(t *testing.T, buildConfigInterface client.BuildConfigInterface, strategy string) (*buildapi.BuildConfig, error) {
261 261
 	buildConfig := &buildapi.BuildConfig{}
262 262
 	buildConfig.GenerateName = strings.ToLower(string(strategy)) + "-buildconfig-"
263
-	buildConfig.Spec.Strategy = strategyForType(strategy)
263
+	buildConfig.Spec.Strategy = strategyForType(t, strategy)
264 264
 	buildConfig.Spec.Source.Git = &buildapi.GitBuildSource{URI: "example.org"}
265 265
 
266 266
 	return buildConfigInterface.Create(buildConfig)