Skip to content

Commit 8b594fe

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3266 from sttts/sttts-authz-simplify
🌱 authorizers: misc prefactorings
2 parents 2325423 + 11f12e6 commit 8b594fe

File tree

6 files changed

+101
-89
lines changed

6 files changed

+101
-89
lines changed

‎pkg/authorization/maximal_permission_policy_authorizer.go

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ const (
4444

4545
// NewMaximalPermissionPolicyAuthorizer returns an authorizer that first checks if the request is for a
4646
// bound resource or not. If the resource is bound it checks the maximal permission policy of the underlying API export.
47-
func NewMaximalPermissionPolicyAuthorizer(kubeInformers, globalKubeInformers kcpkubernetesinformers.SharedInformerFactory, kcpInformers, globalKcpInformers kcpinformers.SharedInformerFactory, delegate authorizer.Authorizer) authorizer.Authorizer {
47+
func NewMaximalPermissionPolicyAuthorizer(
48+
kubeInformers, globalKubeInformers kcpkubernetesinformers.SharedInformerFactory,
49+
kcpInformers, globalKcpInformers kcpinformers.SharedInformerFactory,
50+
) func(delegate authorizer.Authorizer) authorizer.Authorizer {
4851
// Make sure informer knows what to watch
4952
kubeInformers.Rbac().V1().Roles().Lister()
5053
kubeInformers.Rbac().V1().RoleBindings().Lister()
@@ -64,37 +67,39 @@ func NewMaximalPermissionPolicyAuthorizer(kubeInformers, globalKubeInformers kcp
6467
indexers.ByLogicalClusterPathAndName: indexers.IndexByLogicalClusterPathAndName,
6568
})
6669

67-
return &MaximalPermissionPolicyAuthorizer{
68-
getAPIBindings: func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) {
69-
return kcpInformers.Apis().V1alpha1().APIBindings().Lister().Cluster(clusterName).List(labels.Everything())
70-
},
71-
getAPIExport: func(path logicalcluster.Path, name string) (*apisv1alpha1.APIExport, error) {
72-
return indexers.ByPathAndNameWithFallback[*apisv1alpha1.APIExport](apisv1alpha1.Resource("apiexports"), kcpInformers.Apis().V1alpha1().APIExports().Informer().GetIndexer(), globalKcpInformers.Apis().V1alpha1().APIExports().Informer().GetIndexer(), path, name)
73-
},
74-
newAuthorizer: func(clusterName logicalcluster.Name) authorizer.Authorizer {
75-
return rbac.New(
76-
&rbac.RoleGetter{Lister: rbacwrapper.NewMergedRoleLister(
77-
kubeInformers.Rbac().V1().Roles().Lister().Cluster(clusterName),
78-
globalKubeInformers.Rbac().V1().Roles().Lister().Cluster(clusterName),
79-
kubeInformers.Rbac().V1().Roles().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
80-
)},
81-
&rbac.RoleBindingLister{Lister: rbacwrapper.NewMergedRoleBindingLister(
82-
kubeInformers.Rbac().V1().RoleBindings().Lister().Cluster(clusterName),
83-
globalKubeInformers.Rbac().V1().RoleBindings().Lister().Cluster(clusterName),
84-
)},
85-
&rbac.ClusterRoleGetter{Lister: rbacwrapper.NewMergedClusterRoleLister(
86-
kubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(clusterName),
87-
globalKubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(clusterName),
88-
kubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
89-
)},
90-
&rbac.ClusterRoleBindingLister{Lister: rbacwrapper.NewMergedClusterRoleBindingLister(
91-
kubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(clusterName),
92-
globalKubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(clusterName),
93-
kubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
94-
)},
95-
)
96-
},
97-
delegate: delegate,
70+
return func(delegate authorizer.Authorizer) authorizer.Authorizer {
71+
return &MaximalPermissionPolicyAuthorizer{
72+
getAPIBindings: func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) {
73+
return kcpInformers.Apis().V1alpha1().APIBindings().Lister().Cluster(clusterName).List(labels.Everything())
74+
},
75+
getAPIExport: func(path logicalcluster.Path, name string) (*apisv1alpha1.APIExport, error) {
76+
return indexers.ByPathAndNameWithFallback[*apisv1alpha1.APIExport](apisv1alpha1.Resource("apiexports"), kcpInformers.Apis().V1alpha1().APIExports().Informer().GetIndexer(), globalKcpInformers.Apis().V1alpha1().APIExports().Informer().GetIndexer(), path, name)
77+
},
78+
newAuthorizer: func(clusterName logicalcluster.Name) authorizer.Authorizer {
79+
return rbac.New(
80+
&rbac.RoleGetter{Lister: rbacwrapper.NewMergedRoleLister(
81+
kubeInformers.Rbac().V1().Roles().Lister().Cluster(clusterName),
82+
globalKubeInformers.Rbac().V1().Roles().Lister().Cluster(clusterName),
83+
kubeInformers.Rbac().V1().Roles().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
84+
)},
85+
&rbac.RoleBindingLister{Lister: rbacwrapper.NewMergedRoleBindingLister(
86+
kubeInformers.Rbac().V1().RoleBindings().Lister().Cluster(clusterName),
87+
globalKubeInformers.Rbac().V1().RoleBindings().Lister().Cluster(clusterName),
88+
)},
89+
&rbac.ClusterRoleGetter{Lister: rbacwrapper.NewMergedClusterRoleLister(
90+
kubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(clusterName),
91+
globalKubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(clusterName),
92+
kubeInformers.Rbac().V1().ClusterRoles().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
93+
)},
94+
&rbac.ClusterRoleBindingLister{Lister: rbacwrapper.NewMergedClusterRoleBindingLister(
95+
kubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(clusterName),
96+
globalKubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(clusterName),
97+
kubeInformers.Rbac().V1().ClusterRoleBindings().Lister().Cluster(controlplaneapiserver.LocalAdminCluster),
98+
)},
99+
)
100+
},
101+
delegate: delegate,
102+
}
98103
}
99104
}
100105

‎pkg/authorization/requiredgroups_authorizer.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,20 @@ const (
4242

4343
// NewRequiredGroupsAuthorizer returns an authorizer that a set of groups stored
4444
// on the LogicalCluster object. Service account by-pass this.
45-
func NewRequiredGroupsAuthorizer(local, global corev1alpha1listers.LogicalClusterClusterLister, delegate authorizer.Authorizer) authorizer.Authorizer {
46-
return &requiredGroupsAuthorizer{
47-
getLogicalCluster: func(logicalCluster logicalcluster.Name) (*corev1alpha1.LogicalCluster, error) {
48-
obj, err := local.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
49-
if err != nil && !errors.IsNotFound(err) {
50-
return nil, err
51-
} else if errors.IsNotFound(err) {
52-
return global.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
53-
}
54-
return obj, nil
55-
},
56-
delegate: delegate,
45+
func NewRequiredGroupsAuthorizer(local, global corev1alpha1listers.LogicalClusterClusterLister) func(delegate authorizer.Authorizer) authorizer.Authorizer {
46+
return func(delegate authorizer.Authorizer) authorizer.Authorizer {
47+
return &requiredGroupsAuthorizer{
48+
getLogicalCluster: func(logicalCluster logicalcluster.Name) (*corev1alpha1.LogicalCluster, error) {
49+
obj, err := local.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
50+
if err != nil && !errors.IsNotFound(err) {
51+
return nil, err
52+
} else if errors.IsNotFound(err) {
53+
return global.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
54+
}
55+
return obj, nil
56+
},
57+
delegate: delegate,
58+
}
5759
}
5860
}
5961

‎pkg/authorization/workspace_content_authorizer.go

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,27 @@ const (
4444
WorkspaceAccessNotPermittedReason = "workspace access not permitted"
4545
)
4646

47-
func NewWorkspaceContentAuthorizer(localInformers, globalInformers kcpkubernetesinformers.SharedInformerFactory, localLogicalClusterLister, globalLogicalClusterLister corev1alpha1listers.LogicalClusterClusterLister, delegate authorizer.Authorizer) authorizer.Authorizer {
48-
return &workspaceContentAuthorizer{
49-
localClusterRoleLister: localInformers.Rbac().V1().ClusterRoles().Lister(),
50-
localClusterRoleBindingLister: localInformers.Rbac().V1().ClusterRoleBindings().Lister(),
51-
52-
globalClusterRoleLister: globalInformers.Rbac().V1().ClusterRoles().Lister(),
53-
globalClusterRoleBindingLister: globalInformers.Rbac().V1().ClusterRoleBindings().Lister(),
54-
55-
getLogicalCluster: func(logicalCluster logicalcluster.Name) (*corev1alpha1.LogicalCluster, error) {
56-
obj, err := localLogicalClusterLister.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
57-
if err != nil && !errors.IsNotFound(err) {
58-
return nil, err
59-
} else if errors.IsNotFound(err) {
60-
return globalLogicalClusterLister.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
61-
}
62-
return obj, nil
63-
},
64-
65-
delegate: delegate,
47+
func NewWorkspaceContentAuthorizer(localInformers, globalInformers kcpkubernetesinformers.SharedInformerFactory, localLogicalClusterLister, globalLogicalClusterLister corev1alpha1listers.LogicalClusterClusterLister) func(delegate authorizer.Authorizer) authorizer.Authorizer {
48+
return func(delegate authorizer.Authorizer) authorizer.Authorizer {
49+
return &workspaceContentAuthorizer{
50+
localClusterRoleLister: localInformers.Rbac().V1().ClusterRoles().Lister(),
51+
localClusterRoleBindingLister: localInformers.Rbac().V1().ClusterRoleBindings().Lister(),
52+
53+
globalClusterRoleLister: globalInformers.Rbac().V1().ClusterRoles().Lister(),
54+
globalClusterRoleBindingLister: globalInformers.Rbac().V1().ClusterRoleBindings().Lister(),
55+
56+
getLogicalCluster: func(logicalCluster logicalcluster.Name) (*corev1alpha1.LogicalCluster, error) {
57+
obj, err := localLogicalClusterLister.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
58+
if err != nil && !errors.IsNotFound(err) {
59+
return nil, err
60+
} else if errors.IsNotFound(err) {
61+
return globalLogicalClusterLister.Cluster(logicalCluster).Get(corev1alpha1.LogicalClusterName)
62+
}
63+
return obj, nil
64+
},
65+
66+
delegate: delegate,
67+
}
6668
}
6769
}
6870

‎pkg/authorization/workspace_content_authorizer_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
125125
wantReason: "delegating due to user logical cluster access",
126126
},
127127
{
128-
testName: "user with scope to another cluster is denied",
128+
testName: "user with scope to another cluster is not allowed",
129129

130130
requestedWorkspace: "root:ready",
131131
requestingUser: &user.DefaultInfo{Name: "user-access", Extra: map[string][]string{
@@ -143,26 +143,26 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
143143
wantReason: "delegating due to local service account access",
144144
},
145145
{
146-
testName: "user is granted access on root",
146+
testName: "a authenticated user is granted access on root:authenticated",
147147

148-
requestedWorkspace: "root",
148+
requestedWorkspace: "root:authenticated",
149149
requestingUser: &user.DefaultInfo{Name: "somebody", Groups: []string{"system:authenticated"}},
150150
wantDecision: authorizer.DecisionAllow,
151151
wantReason: "delegating due to user logical cluster access",
152152
},
153153
{
154-
testName: "service account from other cluster is denied on root",
154+
testName: "service account from other cluster is denied on root:authenticated",
155155

156-
requestedWorkspace: "root",
156+
requestedWorkspace: "root:authenticated",
157157
requestingUser: newServiceAccountWithCluster("somebody", "someworkspace", "system:authenticated"),
158158
wantDecision: authorizer.DecisionDeny,
159159
wantReason: "foreign service account",
160160
},
161161
{
162-
testName: "service account from root cluster is granted access on root",
162+
testName: "service account from root:authenticated cluster is granted access on root:authenticated",
163163

164-
requestedWorkspace: "root",
165-
requestingUser: newServiceAccountWithCluster("somebody", "root", "system:authenticated"),
164+
requestedWorkspace: "root:authenticated",
165+
requestingUser: newServiceAccountWithCluster("somebody", "root:authenticated", "system:authenticated"),
166166
wantDecision: authorizer.DecisionAllow,
167167
wantReason: "delegating due to local service account access",
168168
},
@@ -248,9 +248,9 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
248248
&v1.ClusterRoleBinding{
249249
ObjectMeta: metav1.ObjectMeta{
250250
Annotations: map[string]string{
251-
logicalcluster.AnnotationKey: "root",
251+
logicalcluster.AnnotationKey: "root:authenticated",
252252
},
253-
Name: "system:authenticated:access",
253+
Name: "system:authenticated:root:authenticated:access",
254254
},
255255
Subjects: []v1.Subject{
256256
{
@@ -270,7 +270,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
270270
Annotations: map[string]string{
271271
logicalcluster.AnnotationKey: "root:ready",
272272
},
273-
Name: "user-access-ready-access",
273+
Name: "user-access:root:ready:access",
274274
},
275275
Subjects: []v1.Subject{
276276
{
@@ -290,7 +290,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
290290
Annotations: map[string]string{
291291
logicalcluster.AnnotationKey: "root:initializing",
292292
},
293-
Name: "user-access-initializing-access",
293+
Name: "user-access:root:initializing:access",
294294
},
295295
Subjects: []v1.Subject{
296296
{
@@ -310,7 +310,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
310310
Annotations: map[string]string{
311311
logicalcluster.AnnotationKey: "rootwithoutparent",
312312
},
313-
Name: "system:authenticated:access",
313+
Name: "user-access:rootwithoutparent:access",
314314
},
315315
Subjects: []v1.Subject{
316316
{
@@ -343,7 +343,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
343343

344344
localIndexer := cache.NewIndexer(kcpcache.MetaClusterNamespaceKeyFunc, cache.Indexers{})
345345
require.NoError(t, localIndexer.Add(&corev1alpha1.LogicalCluster{
346-
ObjectMeta: metav1.ObjectMeta{Name: corev1alpha1.LogicalClusterName, Annotations: map[string]string{logicalcluster.AnnotationKey: "root"}},
346+
ObjectMeta: metav1.ObjectMeta{Name: corev1alpha1.LogicalClusterName, Annotations: map[string]string{logicalcluster.AnnotationKey: "root:authenticated"}},
347347
Status: corev1alpha1.LogicalClusterStatus{Phase: corev1alpha1.LogicalClusterPhaseReady},
348348
}))
349349
require.NoError(t, localIndexer.Add(&corev1alpha1.LogicalCluster{
@@ -369,7 +369,7 @@ func TestWorkspaceContentAuthorizer(t *testing.T) {
369369
globalLogicalClusters := corev1alpha1listers.NewLogicalClusterClusterLister(globalIndexer)
370370

371371
recordingAuthorizer := &recordingAuthorizer{decision: authorizer.DecisionAllow, reason: "allowed"}
372-
w := NewWorkspaceContentAuthorizer(local, global, localLogicalClusters, globalLogicalClusters, recordingAuthorizer)
372+
w := NewWorkspaceContentAuthorizer(local, global, localLogicalClusters, globalLogicalClusters)(recordingAuthorizer)
373373

374374
requestedCluster := request.Cluster{
375375
Name: logicalcluster.Name(tt.requestedWorkspace),

‎pkg/server/options/authorization.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ func (s *Authorization) ApplyTo(ctx context.Context, config *genericapiserver.Co
136136

137137
// group authorizer
138138
if len(s.AlwaysAllowGroups) > 0 {
139-
authorizers = append(authorizers, authorizerfactory.NewPrivilegedGroups(s.AlwaysAllowGroups...))
139+
privGroups := authorizerfactory.NewPrivilegedGroups(s.AlwaysAllowGroups...)
140+
authorizers = append(authorizers, privGroups)
140141
}
141142

142143
// path authorizer
@@ -190,31 +191,33 @@ func (s *Authorization) ApplyTo(ctx context.Context, config *genericapiserver.Co
190191
globalAuth, _ := authz.NewGlobalAuthorizer(kubeInformers, globalKubeInformers)
191192
globalAuth = authz.NewDecorator("05-global", globalAuth).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
192193

194+
chain := union.New(bootstrapAuth, localAuth, globalAuth)
195+
193196
// everything below - skipped for Deep SAR
194197

195198
// enforce maximal permission policy
196-
maxPermissionPolicyAuth := authz.NewMaximalPermissionPolicyAuthorizer(kubeInformers, globalKubeInformers, kcpInformers, globalKcpInformers, union.New(bootstrapAuth, localAuth, globalAuth))
197-
maxPermissionPolicyAuth = authz.NewDecorator("04-maxpermissionpolicy", maxPermissionPolicyAuth).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
199+
chain = authz.NewMaximalPermissionPolicyAuthorizer(kubeInformers, globalKubeInformers, kcpInformers, globalKcpInformers)(chain)
200+
chain = authz.NewDecorator("04-maxpermissionpolicy", chain).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
198201

199202
// protect status updates to apiexport and apibinding
200-
systemCRDAuth := authz.NewSystemCRDAuthorizer(maxPermissionPolicyAuth)
201-
systemCRDAuth = authz.NewDecorator("03-systemcrd", systemCRDAuth).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
203+
chain = authz.NewSystemCRDAuthorizer(chain)
204+
chain = authz.NewDecorator("03-systemcrd", chain).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
202205

203206
// content auth deteremines if users have access to the workspace itself - by default, in Kube there is a set
204207
// of default permissions given even to system:authenticated (like access to discovery) - this authorizer allows
205208
// kcp to make workspaces entirely invisible to users that have not been given access, by making system:authenticated
206209
// mean nothing unless they also have `verb=access` on `/`
207-
contentAuth := authz.NewWorkspaceContentAuthorizer(kubeInformers, globalKubeInformers, localLogicalClusterLister, globalLogicalClusterLister, systemCRDAuth)
208-
contentAuth = authz.NewDecorator("02-content", contentAuth).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
210+
chain = authz.NewWorkspaceContentAuthorizer(kubeInformers, globalKubeInformers, localLogicalClusterLister, globalLogicalClusterLister)(chain)
211+
chain = authz.NewDecorator("02-content", chain).AddAuditLogging().AddAnonymization().AddReasonAnnotation()
209212

210213
// workspaces are annotated to list the groups required on users wishing to access the workspace -
211214
// this is mostly useful when adding a core set of groups to an org workspace and having them inherited
212215
// by child workspaces; this gives administrators of an org control over which users can be given access
213216
// to content in sub-workspaces
214-
requiredGroupsAuth := authz.NewRequiredGroupsAuthorizer(localLogicalClusterLister, globalLogicalClusterLister, contentAuth)
215-
requiredGroupsAuth = authz.NewDecorator("01-requiredgroups", requiredGroupsAuth).AddAuditLogging().AddAnonymization()
217+
chain = authz.NewRequiredGroupsAuthorizer(localLogicalClusterLister, globalLogicalClusterLister)(chain)
218+
chain = authz.NewDecorator("01-requiredgroups", chain).AddAuditLogging().AddAnonymization()
216219

217-
authorizers = append(authorizers, requiredGroupsAuth)
220+
authorizers = append(authorizers, chain)
218221

219222
config.RuleResolver = union.NewRuleResolvers(bootstrapRules, localResolver)
220223
config.Authorization.Authorizer = union.New(authorizers...)

‎test/e2e/authorizer/serviceaccounts_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func TestServiceAccounts(t *testing.T) {
253253
}, metav1.CreateOptions{})
254254
require.NoError(t, err, "failed to create role")
255255

256-
t.Log("Verifying if service account is allowed to escalate")
256+
t.Log("Verifying if service account is allowed to delegate")
257257
framework.Eventually(t, func() (bool, string) { // authz makes this eventually succeed
258258
_, err = saKubeClusterClient.Cluster(wsPath).RbacV1().ClusterRoles().Create(ctx, &rbacv1.ClusterRole{
259259
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)