[release-3.3] Check cluster permission for create/update workspacetemplate (#5310)

* add cluster authorization for create/update workspacetemplate

Signed-off-by: Wenhao Zhou <wenhaozhou@yunify.com>

add cluster authorization for create/update workspacetemplate

Signed-off-by: Wenhao Zhou <wenhaozhou@yunify.com>

* add handle forbidden err

* add forbidden error log

* allow to use clusters of public visibility

Signed-off-by: Wenhao Zhou <wenhaozhou@yunify.com>
Co-authored-by: Wenhao Zhou <wenhaozhou@yunify.com>

pkg/apiserver/authentication/identityprovider/ldap/ldap.go

@@ -45,7 +45,7 @@ func init() {
 type ldapProvider struct {
 	// Host and optional port of the LDAP server in the form "host:port".
 	// If the port is not supplied, 389 for insecure or StartTLS connections, 636
-	Host string `json:"host,omitempty" yaml:"managerDN"`
+	Host string `json:"host,omitempty" yaml:"host"`
 	// Timeout duration when reading data from remote server. Default to 15s.
 	ReadTimeout int `json:"readTimeout" yaml:"readTimeout"`
 	// If specified, connections will use the ldaps:// protocol

pkg/apiserver/request/requestinfo.go

@@ -246,8 +246,6 @@ func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, er
 	// parsing successful, so we now know the proper value for .Parts
 	requestInfo.Parts = currentParts
 
-	requestInfo.ResourceScope = r.resolveResourceScope(requestInfo)
-
 	// parts look like: resource/resourceName/subresource/other/stuff/we/don't/interpret
 	switch {
 	case len(requestInfo.Parts) >= 3 && !specialVerbsNoSubresources.Has(requestInfo.Verb):
@@ -260,6 +258,8 @@ func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, er
 		requestInfo.Resource = requestInfo.Parts[0]
 	}
 
+	requestInfo.ResourceScope = r.resolveResourceScope(requestInfo)
+
 	// if there's no name on the request and we thought it was a get before, then the actual verb is a list or a watch
 	if len(requestInfo.Name) == 0 && requestInfo.Verb == "get" {
 		opts := metainternalversion.ListOptions{}

pkg/kapis/tenant/v1alpha2/handler.go

@@ -202,30 +202,40 @@ func (h *tenantHandler) CreateNamespace(request *restful.Request, response *rest
 	response.WriteEntity(created)
 }
 
-func (h *tenantHandler) CreateWorkspaceTemplate(request *restful.Request, response *restful.Response) {
+func (h *tenantHandler) CreateWorkspaceTemplate(req *restful.Request, resp *restful.Response) {
 	var workspace tenantv1alpha2.WorkspaceTemplate
 
-	err := request.ReadEntity(&workspace)
+	err := req.ReadEntity(&workspace)
 
 	if err != nil {
 		klog.Error(err)
-		api.HandleBadRequest(response, request, err)
+		api.HandleBadRequest(resp, req, err)
 		return
 	}
+	requestUser, ok := request.UserFrom(req.Request.Context())
+	if !ok {
+		err := fmt.Errorf("cannot obtain user info")
+		klog.Errorln(err)
+		api.HandleForbidden(resp, req, err)
+	}
 
-	created, err := h.tenant.CreateWorkspaceTemplate(&workspace)
+	created, err := h.tenant.CreateWorkspaceTemplate(requestUser, &workspace)
 
 	if err != nil {
 		klog.Error(err)
 		if errors.IsNotFound(err) {
-			api.HandleNotFound(response, request, err)
+			api.HandleNotFound(resp, req, err)
 			return
 		}
-		api.HandleBadRequest(response, request, err)
+		if errors.IsForbidden(err) {
+			api.HandleForbidden(resp, req, err)
+			return
+		}
+		api.HandleBadRequest(resp, req, err)
 		return
 	}
 
-	response.WriteEntity(created)
+	resp.WriteEntity(created)
 }
 
 func (h *tenantHandler) DeleteWorkspaceTemplate(request *restful.Request, response *restful.Response) {
@@ -253,42 +263,53 @@ func (h *tenantHandler) DeleteWorkspaceTemplate(request *restful.Request, respon
 	response.WriteEntity(servererr.None)
 }
 
-func (h *tenantHandler) UpdateWorkspaceTemplate(request *restful.Request, response *restful.Response) {
-	workspaceName := request.PathParameter("workspace")
+func (h *tenantHandler) UpdateWorkspaceTemplate(req *restful.Request, resp *restful.Response) {
+	workspaceName := req.PathParameter("workspace")
 	var workspace tenantv1alpha2.WorkspaceTemplate
 
-	err := request.ReadEntity(&workspace)
+	err := req.ReadEntity(&workspace)
 
 	if err != nil {
 		klog.Error(err)
-		api.HandleBadRequest(response, request, err)
+		api.HandleBadRequest(resp, req, err)
 		return
 	}
 
 	if workspaceName != workspace.Name {
 		err := fmt.Errorf("the name of the object (%s) does not match the name on the URL (%s)", workspace.Name, workspaceName)
 		klog.Errorf("%+v", err)
-		api.HandleBadRequest(response, request, err)
+		api.HandleBadRequest(resp, req, err)
 		return
 	}
 
-	updated, err := h.tenant.UpdateWorkspaceTemplate(&workspace)
+	requestUser, ok := request.UserFrom(req.Request.Context())
+	if !ok {
+		err := fmt.Errorf("cannot obtain user info")
+		klog.Errorln(err)
+		api.HandleForbidden(resp, req, err)
+	}
+
+	updated, err := h.tenant.UpdateWorkspaceTemplate(requestUser, &workspace)
 
 	if err != nil {
 		klog.Error(err)
 		if errors.IsNotFound(err) {
-			api.HandleNotFound(response, request, err)
+			api.HandleNotFound(resp, req, err)
 			return
 		}
 		if errors.IsBadRequest(err) {
-			api.HandleBadRequest(response, request, err)
+			api.HandleBadRequest(resp, req, err)
 			return
 		}
-		api.HandleInternalError(response, request, err)
+		if errors.IsForbidden(err) {
+			api.HandleForbidden(resp, req, err)
+			return
+		}
+		api.HandleInternalError(resp, req, err)
 		return
 	}
 
-	response.WriteEntity(updated)
+	resp.WriteEntity(updated)
 }
 
 func (h *tenantHandler) DescribeWorkspaceTemplate(request *restful.Request, response *restful.Response) {

pkg/models/gateway/gateway.go

@@ -47,12 +47,13 @@ import (
 )
 
 const (
-	MasterLabel       = "node-role.kubernetes.io/master"
-	SidecarInject     = "sidecar.istio.io/inject"
-	gatewayPrefix     = "kubesphere-router-"
-	workingNamespace  = "kubesphere-controls-system"
-	globalGatewayname = gatewayPrefix + "kubesphere-system"
-	helmPatch         = `{"metadata":{"annotations":{"meta.helm.sh/release-name":"%s-ingress","meta.helm.sh/release-namespace":"%s"},"labels":{"helm.sh/chart":"ingress-nginx-3.35.0","app.kubernetes.io/managed-by":"Helm","app":null,"component":null,"tier":null}},"spec":{"selector":null}}`
+	MasterLabel             = "node-role.kubernetes.io/master"
+	SidecarInject           = "sidecar.istio.io/inject"
+	gatewayPrefix           = "kubesphere-router-"
+	workingNamespace        = "kubesphere-controls-system"
+	globalGatewayNameSuffix = "kubesphere-system"
+	globalGatewayName       = gatewayPrefix + globalGatewayNameSuffix
+	helmPatch               = `{"metadata":{"annotations":{"meta.helm.sh/release-name":"%s-ingress","meta.helm.sh/release-namespace":"%s"},"labels":{"helm.sh/chart":"ingress-nginx-3.35.0","app.kubernetes.io/managed-by":"Helm","app":null,"component":null,"tier":null}},"spec":{"selector":null}}`
 )
 
 type GatewayOperator interface {
@@ -90,6 +91,10 @@ func (c *gatewayOperator) getWorkingNamespace(namespace string) string {
 	if ns == "" {
 		ns = namespace
 	}
+	// Convert the global gateway query parameter
+	if namespace == globalGatewayNameSuffix {
+		ns = workingNamespace
+	}
 	return ns
 }
 
@@ -97,7 +102,7 @@ func (c *gatewayOperator) getWorkingNamespace(namespace string) string {
 func (c *gatewayOperator) overrideDefaultValue(gateway *v1alpha1.Gateway, namespace string) *v1alpha1.Gateway {
 	// override default name
 	gateway.Name = fmt.Sprint(gatewayPrefix, namespace)
-	if gateway.Name != globalGatewayname {
+	if gateway.Name != globalGatewayName {
 		gateway.Spec.Controller.Scope = v1alpha1.Scope{Enabled: true, Namespace: namespace}
 	}
 	gateway.Namespace = c.getWorkingNamespace(namespace)
@@ -108,7 +113,7 @@ func (c *gatewayOperator) overrideDefaultValue(gateway *v1alpha1.Gateway, namesp
 func (c *gatewayOperator) getGlobalGateway() *v1alpha1.Gateway {
 	globalkey := types.NamespacedName{
 		Namespace: workingNamespace,
-		Name:      globalGatewayname,
+		Name:      globalGatewayName,
 	}
 
 	global := &v1alpha1.Gateway{}
@@ -331,7 +336,7 @@ func (c *gatewayOperator) UpgradeGateway(namespace string) (*v1alpha1.Gateway, e
 	if l == nil {
 		return nil, fmt.Errorf("invalid operation, no legacy gateway was found")
 	}
-	if l.Namespace != c.options.Namespace {
+	if l.Namespace != c.getWorkingNamespace(namespace) {
 		return nil, fmt.Errorf("invalid operation, can't upgrade legacy gateway when working namespace changed")
 	}
 

pkg/models/tenant/tenant.go

@@ -82,9 +82,9 @@ type Interface interface {
 	ListWorkspaces(user user.Info, queryParam *query.Query) (*api.ListResult, error)
 	GetWorkspace(workspace string) (*tenantv1alpha1.Workspace, error)
 	ListWorkspaceTemplates(user user.Info, query *query.Query) (*api.ListResult, error)
-	CreateWorkspaceTemplate(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error)
+	CreateWorkspaceTemplate(user user.Info, workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error)
 	DeleteWorkspaceTemplate(workspace string, opts metav1.DeleteOptions) error
-	UpdateWorkspaceTemplate(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error)
+	UpdateWorkspaceTemplate(user user.Info, workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error)
 	PatchWorkspaceTemplate(user user.Info, workspace string, data json.RawMessage) (*tenantv1alpha2.WorkspaceTemplate, error)
 	DescribeWorkspaceTemplate(workspace string) (*tenantv1alpha2.WorkspaceTemplate, error)
 	ListNamespaces(user user.Info, workspace string, query *query.Query) (*api.ListResult, error)
@@ -534,81 +534,53 @@ func (t *tenantOperator) PatchWorkspaceTemplate(user user.Info, workspace string
 	}
 
 	if manageWorkspaceTemplateRequest {
-		deleteWST := authorizer.AttributesRecord{
-			User:            user,
-			Verb:            authorizer.VerbDelete,
-			APIGroup:        tenantv1alpha2.SchemeGroupVersion.Group,
-			APIVersion:      tenantv1alpha2.SchemeGroupVersion.Version,
-			Resource:        tenantv1alpha2.ResourcePluralWorkspaceTemplate,
-			ResourceRequest: true,
-			ResourceScope:   request.GlobalScope,
-		}
-		authorize, reason, err := t.authorizer.Authorize(deleteWST)
+		err := t.checkWorkspaceTemplatePermission(user, workspace)
 		if err != nil {
 			klog.Error(err)
 			return nil, err
 		}
-		if authorize != authorizer.DecisionAllow {
-			err := errors.NewForbidden(tenantv1alpha2.Resource(tenantv1alpha2.ResourcePluralWorkspaceTemplate), workspace, fmt.Errorf(reason))
+	}
+
+	if clusterNames.Len() > 0 {
+		err := t.checkClusterPermission(user, clusterNames.List())
+		if err != nil {
 			klog.Error(err)
 			return nil, err
 		}
 	}
-	// Checking whether the user can manage the cluster requires authentication from two aspects.
-	// First check whether the user has relevant global permissions,
-	// and then check whether the user has relevant cluster permissions in the target cluster
-	if clusterNames.Len() > 0 {
-		for _, clusterName := range clusterNames.List() {
-			deleteCluster := authorizer.AttributesRecord{
-				User:            user,
-				Verb:            authorizer.VerbDelete,
-				APIGroup:        clusterv1alpha1.SchemeGroupVersion.Version,
-				APIVersion:      clusterv1alpha1.SchemeGroupVersion.Version,
-				Resource:        clusterv1alpha1.ResourcesPluralCluster,
-				Cluster:         clusterName,
-				ResourceRequest: true,
-				ResourceScope:   request.GlobalScope,
-			}
-			authorize, reason, err := t.authorizer.Authorize(deleteCluster)
-			if err != nil {
-				klog.Error(err)
-				return nil, err
-			}
-
-			if authorize == authorizer.DecisionAllow {
-				continue
-			}
-
-			list, err := t.getClusterRoleBindingsByUser(clusterName, user.GetName())
-			if err != nil {
-				klog.Error(err)
-				return nil, err
-			}
-
-			allowed := false
-			for _, clusterRolebinding := range list.Items {
-				if clusterRolebinding.RoleRef.Name == iamv1alpha2.ClusterAdmin {
-					allowed = true
-					break
-				}
-			}
-
-			if !allowed {
-				err = errors.NewForbidden(clusterv1alpha1.Resource(clusterv1alpha1.ResourcesPluralCluster), clusterName, fmt.Errorf(reason))
-				klog.Error(err)
-				return nil, err
-			}
-		}
-	}
 
 	return t.ksclient.TenantV1alpha2().WorkspaceTemplates().Patch(context.Background(), workspace, types.JSONPatchType, data, metav1.PatchOptions{})
 }
 
-func (t *tenantOperator) CreateWorkspaceTemplate(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) {
+func (t *tenantOperator) CreateWorkspaceTemplate(user user.Info, workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) {
+	if len(workspace.Spec.Placement.Clusters) != 0 {
+		clusters := make([]string, 0)
+		for _, v := range workspace.Spec.Placement.Clusters {
+			clusters = append(clusters, v.Name)
+		}
+		err := t.checkClusterPermission(user, clusters)
+		if err != nil {
+			klog.Error(err)
+			return nil, err
+		}
+
+	}
 	return t.ksclient.TenantV1alpha2().WorkspaceTemplates().Create(context.Background(), workspace, metav1.CreateOptions{})
 }
 
-func (t *tenantOperator) UpdateWorkspaceTemplate(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) {
+func (t *tenantOperator) UpdateWorkspaceTemplate(user user.Info, workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) {
+	if len(workspace.Spec.Placement.Clusters) != 0 {
+		clusters := make([]string, 0)
+		for _, v := range workspace.Spec.Placement.Clusters {
+			clusters = append(clusters, v.Name)
+		}
+		err := t.checkClusterPermission(user, clusters)
+		if err != nil {
+			klog.Error(err)
+			return nil, err
+		}
+
+	}
 	return t.ksclient.TenantV1alpha2().WorkspaceTemplates().Update(context.Background(), workspace, metav1.UpdateOptions{})
 }
 
@@ -1246,3 +1218,78 @@ func stringContains(str string, subStrs []string) bool {
 	}
 	return false
 }
+
+func (t *tenantOperator) checkWorkspaceTemplatePermission(user user.Info, workspace string) error {
+	deleteWST := authorizer.AttributesRecord{
+		User:            user,
+		Verb:            authorizer.VerbDelete,
+		APIGroup:        tenantv1alpha2.SchemeGroupVersion.Group,
+		APIVersion:      tenantv1alpha2.SchemeGroupVersion.Version,
+		Resource:        tenantv1alpha2.ResourcePluralWorkspaceTemplate,
+		ResourceRequest: true,
+		ResourceScope:   request.GlobalScope,
+	}
+	authorize, reason, err := t.authorizer.Authorize(deleteWST)
+	if err != nil {
+		return err
+	}
+	if authorize != authorizer.DecisionAllow {
+		return errors.NewForbidden(tenantv1alpha2.Resource(tenantv1alpha2.ResourcePluralWorkspaceTemplate), workspace, fmt.Errorf(reason))
+	}
+	return nil
+}
+
+func (t *tenantOperator) checkClusterPermission(user user.Info, clusters []string) error {
+	// Checking whether the user can manage the cluster requires authentication from two aspects.
+	// First check whether the user has relevant global permissions,
+	// and then check whether the user has relevant cluster permissions in the target cluster
+
+	for _, clusterName := range clusters {
+
+		cluster, err := t.ksclient.ClusterV1alpha1().Clusters().Get(context.Background(), clusterName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		if cluster.Labels["cluster.kubesphere.io/visibility"] == "public" {
+			continue
+		}
+
+		deleteCluster := authorizer.AttributesRecord{
+			User:            user,
+			Verb:            authorizer.VerbDelete,
+			APIGroup:        clusterv1alpha1.SchemeGroupVersion.Version,
+			APIVersion:      clusterv1alpha1.SchemeGroupVersion.Version,
+			Resource:        clusterv1alpha1.ResourcesPluralCluster,
+			Cluster:         clusterName,
+			ResourceRequest: true,
+			ResourceScope:   request.GlobalScope,
+		}
+		authorize, _, err := t.authorizer.Authorize(deleteCluster)
+		if err != nil {
+			return err
+		}
+
+		if authorize == authorizer.DecisionAllow {
+			continue
+		}
+
+		list, err := t.getClusterRoleBindingsByUser(clusterName, user.GetName())
+		if err != nil {
+			return err
+		}
+
+		allowed := false
+		for _, clusterRolebinding := range list.Items {
+			if clusterRolebinding.RoleRef.Name == iamv1alpha2.ClusterAdmin {
+				allowed = true
+				break
+			}
+		}
+
+		if !allowed {
+			return errors.NewForbidden(clusterv1alpha1.Resource(clusterv1alpha1.ResourcesPluralCluster), clusterName, fmt.Errorf("user is not allowed to use the cluster %s", clusterName))
+		}
+	}
+
+	return nil
+}