From 32f50f6883f76a14f6966d953e10c7aec7d77f00 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 6 May 2022 13:10:44 +0200 Subject: [PATCH] authorization: align with documentation --- pkg/authorization/bootstrap/policy.go | 8 +- pkg/authorization/toplevel_org_authorizer.go | 90 ++++++++++++------- .../workspace_content_authorizer.go | 78 ++++++++-------- 3 files changed, 97 insertions(+), 79 deletions(-) diff --git a/pkg/authorization/bootstrap/policy.go b/pkg/authorization/bootstrap/policy.go index 07a01dc423bd..a2b25f0df6be 100644 --- a/pkg/authorization/bootstrap/policy.go +++ b/pkg/authorization/bootstrap/policy.go @@ -23,14 +23,16 @@ import ( ) const ( - SystemKcpClusterWorkspaceAccessGroup = "system:kcp:clusterworkspace:access" - SystemKcpClusterWorkspaceAdminGroup = "system:kcp:clusterworkspace:admin" + SystemKcpClusterWorkspaceAccessGroup = "system:kcp:clusterworkspace:access" + SystemKcpClusterWorkspaceAdminGroup = "system:kcp:clusterworkspace:admin" + SystemKcpClusterWorkspaceInitializeGroup = "system:kcp:clusterworkspace:initialize" ) // ClusterRoleBindings return default rolebindings to the default roles func clusterRoleBindings() []rbacv1.ClusterRoleBinding { return []rbacv1.ClusterRoleBinding{ - clusterRoleBindingCustomName(rbacv1helpers.NewClusterBinding("cluster-admin").Groups("system:kcp:clusterworkspace:admin").BindingOrDie(), "system:kcp:clusterworkspace:admin"), + clusterRoleBindingCustomName(rbacv1helpers.NewClusterBinding("cluster-admin").Groups(SystemKcpClusterWorkspaceAdminGroup).BindingOrDie(), "system:kcp:clusterworkspace:admin"), + clusterRoleBindingCustomName(rbacv1helpers.NewClusterBinding("cluster-admin").Groups(SystemKcpClusterWorkspaceInitializeGroup).BindingOrDie(), "system:kcp:clusterworkspace:initialize"), } } diff --git a/pkg/authorization/toplevel_org_authorizer.go b/pkg/authorization/toplevel_org_authorizer.go index 96e1817f3c04..038fb2554dc6 100644 --- a/pkg/authorization/toplevel_org_authorizer.go +++ b/pkg/authorization/toplevel_org_authorizer.go @@ -75,9 +75,20 @@ func (a *topLevelOrgAccessAuthorizer) Authorize(ctx context.Context, attr author return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil } - // everybody authenticated has access to the root workspace + subjectClusters := map[logicalcluster.Name]bool{} + for _, sc := range attr.GetUser().GetExtra()[authserviceaccount.ClusterNameKey] { + subjectClusters[logicalcluster.New(sc)] = true + } + + isAuthenticated := sets.NewString(attr.GetUser().GetGroups()...).Has("system:authenticated") + isUser := len(subjectClusters) == 0 + isServiceAccount := len(subjectClusters) > 0 + isServiceAccountFromRootCluster := subjectClusters[tenancyv1alpha1.RootCluster] + + // Every authenticated user has access to the root workspace but not every service account. + // For root, only service accounts declared in root have access. if cluster.Name == tenancyv1alpha1.RootCluster { - if sets.NewString(attr.GetUser().GetGroups()...).Has("system:authenticated") { + if isAuthenticated && (isUser || isServiceAccountFromRootCluster) { return a.delegate.Authorize(ctx, attr) } return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil @@ -89,7 +100,7 @@ func (a *topLevelOrgAccessAuthorizer) Authorize(ctx context.Context, attr author return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil } - // check the org in the root exists + // check the org workspace exists in the root workspace if _, err := a.clusterWorkspaceLister.Get(clusters.ToClusterAwareKey(tenancyv1alpha1.RootCluster, requestTopLevelOrgName)); err != nil { if errors.IsNotFound(err) { return authorizer.DecisionDeny, workspaceAccessNotPermittedReason, nil @@ -97,44 +108,55 @@ func (a *topLevelOrgAccessAuthorizer) Authorize(ctx context.Context, attr author return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, err } - if subjectCluster := attr.GetUser().GetExtra()[authserviceaccount.ClusterNameKey]; len(subjectCluster) > 0 { - // service account will automatically get access to its top-level org - subjectTopLevelOrgName, ok := topLevelOrg(logicalcluster.New(subjectCluster[0])) - if !ok || subjectTopLevelOrgName != requestTopLevelOrgName { - return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil - } + switch { + case isServiceAccountFromRootCluster: + // service accounts declared in the root workspace have access to all sub-workspaces return a.delegate.Authorize(ctx, attr) - } else { - var ( - errList []error - reasonList []string - ) - for _, verb := range []string{"access", "member"} { - workspaceAttr := authorizer.AttributesRecord{ - User: attr.GetUser(), - Verb: verb, - APIGroup: tenancyv1alpha1.SchemeGroupVersion.Group, - APIVersion: tenancyv1alpha1.SchemeGroupVersion.Version, - Resource: "clusterworkspaces", - Subresource: "content", - Name: requestTopLevelOrgName, - - ResourceRequest: true, - } - - dec, reason, err := a.rootAuthorizer.Authorize(ctx, workspaceAttr) - if err != nil { - errList = append(errList, err) - reasonList = append(reasonList, reason) + case isServiceAccount: + // All other service accounts will automatically get access to their top-level org + for sc := range subjectClusters { + subjectTopLevelOrg, ok := topLevelOrg(sc) + if !ok { continue } - if dec == authorizer.DecisionAllow { + if subjectTopLevelOrg != requestTopLevelOrgName { return a.delegate.Authorize(ctx, attr) } } - if len(errList) > 0 { - return authorizer.DecisionNoOpinion, strings.Join(reasonList, "\n"), utilerrors.NewAggregate(errList) + + return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil + } + + var ( + errList []error + reasonList []string + ) + + for _, verb := range []string{"access", "member"} { + workspaceAttr := authorizer.AttributesRecord{ + User: attr.GetUser(), + Verb: verb, + APIGroup: tenancyv1alpha1.SchemeGroupVersion.Group, + APIVersion: tenancyv1alpha1.SchemeGroupVersion.Version, + Resource: "clusterworkspaces", + Subresource: "content", + Name: requestTopLevelOrgName, + + ResourceRequest: true, + } + + dec, reason, err := a.rootAuthorizer.Authorize(ctx, workspaceAttr) + if err != nil { + errList = append(errList, err) + reasonList = append(reasonList, reason) + continue } + if dec == authorizer.DecisionAllow { + return a.delegate.Authorize(ctx, attr) + } + } + if len(errList) > 0 { + return authorizer.DecisionNoOpinion, strings.Join(reasonList, "\n"), utilerrors.NewAggregate(errList) } return authorizer.DecisionNoOpinion, workspaceAccessNotPermittedReason, nil diff --git a/pkg/authorization/workspace_content_authorizer.go b/pkg/authorization/workspace_content_authorizer.go index bf3ebb790d2c..edb4e7f684b7 100644 --- a/pkg/authorization/workspace_content_authorizer.go +++ b/pkg/authorization/workspace_content_authorizer.go @@ -74,13 +74,25 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori if err != nil { return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), err } - if cluster == nil || cluster.Name.Empty() { + // empty or non-root based workspaces have no meaning in the context of authorizing workspace content. + if cluster == nil || cluster.Name.Empty() || !cluster.Name.HasPrefix(tenancyv1alpha1.RootCluster) { return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil } - // everybody authenticated has access to the root workspace + subjectClusters := map[logicalcluster.Name]bool{} + for _, sc := range attr.GetUser().GetExtra()[authserviceaccount.ClusterNameKey] { + subjectClusters[logicalcluster.New(sc)] = true + } + + isAuthenticated := sets.NewString(attr.GetUser().GetGroups()...).Has("system:authenticated") + isUser := len(subjectClusters) == 0 + isServiceAccountFromRootCluster := subjectClusters[tenancyv1alpha1.RootCluster] + isServiceAccountFromCluster := subjectClusters[cluster.Name] + + // Every authenticated user has access to the root workspace but not every service account. + // For root, only service accounts declared in root have access. if cluster.Name == tenancyv1alpha1.RootCluster { - if sets.NewString(attr.GetUser().GetGroups()...).Has("system:authenticated") { + if isAuthenticated && (isUser || isServiceAccountFromRootCluster) { return a.delegate.Authorize(ctx, attributesWithReplacedGroups(attr, append(attr.GetUser().GetGroups(), bootstrap.SystemKcpClusterWorkspaceAccessGroup))) } return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), err @@ -90,7 +102,6 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori if !hasParent { return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil } - clusterWorkspace := cluster.Name.Base() parentWorkspaceKubeInformer := rbacwrapper.FilterInformers(parentClusterName, a.versionedInformers.Rbac().V1()) parentAuthorizer := rbac.New( @@ -100,47 +111,24 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori &rbac.ClusterRoleBindingLister{Lister: parentWorkspaceKubeInformer.ClusterRoleBindings().Lister()}, ) + extraGroups := sets.NewString() + // check the workspace even exists - // TODO: using scoping when available - if ws, err := a.clusterWorkspaceLister.Get(clusters.ToClusterAwareKey(parentClusterName, clusterWorkspace)); err != nil { + ws, err := a.clusterWorkspaceLister.Get(clusters.ToClusterAwareKey(parentClusterName, cluster.Name.Base())) + if err != nil { if errors.IsNotFound(err) { - return authorizer.DecisionDeny, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil + return authorizer.DecisionDeny, fmt.Sprintf("%q workspace access not permitted", cluster.Name.Base()), nil } return authorizer.DecisionNoOpinion, "", err - } else if len(ws.Status.Initializers) > 0 { - workspaceAttr := authorizer.AttributesRecord{ - User: attr.GetUser(), - Verb: attr.GetVerb(), - APIGroup: tenancyv1alpha1.SchemeGroupVersion.Group, - APIVersion: tenancyv1alpha1.SchemeGroupVersion.Version, - Resource: "clusterworkspaces", - Subresource: "initialize", - Name: clusterWorkspace, - ResourceRequest: true, - } - - dec, reason, err := parentAuthorizer.Authorize(ctx, workspaceAttr) - if err != nil { - return dec, reason, err - } - if dec != authorizer.DecisionAllow { - return dec, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil - } } - extraGroups := []string{} - if subjectCluster := attr.GetUser().GetExtra()[authserviceaccount.ClusterNameKey]; len(subjectCluster) > 0 { - // a subject from a workspace, like a ServiceAccount, is automatically authenticated - // against that workspace. - // On the other hand, referencing that in the parent cluster for further permissions - // is not possible. Hence, we skip the authorization steps for the verb below. - for _, sc := range subjectCluster { - if logicalcluster.New(sc) == cluster.Name { - extraGroups = append(extraGroups, bootstrap.SystemKcpClusterWorkspaceAccessGroup) - break - } - } - } else { + switch { + case isServiceAccountFromCluster: + // A service account declared in the requested workspace is authorized inside that workspace. + // Referencing such a service account in the parent workspace is not possible, + // hence authorization against "admin" or "access" verbs in the parent is not possible either. + extraGroups.Insert(bootstrap.SystemKcpClusterWorkspaceAccessGroup) + case isUser: verbToGroupMembership := map[string][]string{ "admin": {bootstrap.SystemKcpClusterWorkspaceAccessGroup, bootstrap.SystemKcpClusterWorkspaceAdminGroup}, "access": {bootstrap.SystemKcpClusterWorkspaceAccessGroup}, @@ -158,7 +146,7 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori APIVersion: tenancyv1alpha1.SchemeGroupVersion.Version, Resource: "clusterworkspaces", Subresource: "content", - Name: clusterWorkspace, + Name: cluster.Name.Base(), ResourceRequest: true, } @@ -169,18 +157,24 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori continue } if dec == authorizer.DecisionAllow { - extraGroups = append(extraGroups, groups...) + extraGroups.Insert(groups...) } } if len(errList) > 0 { return authorizer.DecisionNoOpinion, strings.Join(reasonList, "\n"), utilerrors.NewAggregate(errList) } } + + // non-admin subjects don't have access to initializing workspaces. + if ws.Status.Phase == tenancyv1alpha1.ClusterWorkspacePhaseInitializing && !extraGroups.Has(bootstrap.SystemKcpClusterWorkspaceAdminGroup) { + return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil + } + if len(extraGroups) == 0 { return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil } - return a.delegate.Authorize(ctx, attributesWithReplacedGroups(attr, append(attr.GetUser().GetGroups(), extraGroups...))) + return a.delegate.Authorize(ctx, attributesWithReplacedGroups(attr, append(attr.GetUser().GetGroups(), extraGroups.List()...))) } func attributesWithReplacedGroups(attr authorizer.Attributes, groups []string) authorizer.Attributes {