Merge pull request #292 from maxsokolovsky/validate-prtb-project-name…

…-and-namespace

Don't allow PRTB namespace and the project ID of projectName to differ

docs.md

@@ -182,10 +182,13 @@ This is to prevent privilege escalation.
 
 Users cannot create ProjectRoleTemplateBindings that violate the following constraints:
 
+- The `ProjectName` field must be:
+    - Provided as a non-empty value
+    - Specified using the format of `cluster-id:project-id`
+    - `project-id`, in particular, must match the namespace of the ProjectRoleTemplateBinding
 - Either a user subject (through `UserName` or `UserPrincipalName`), or a group subject (through `GroupName`
   or `GroupPrincipalName`), or a service account subject (through `ServiceAccount`) must be specified. Exactly one
   subject type of the three must be provided.
-- `ProjectName` must be specified
 - The roleTemplate indicated in `RoleTemplateName` must be:
     - Provided as a non-empty value
     - Valid (there must exist a `roleTemplate` object of given name in the `management.cattle.io/v3` API group)

pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md

@@ -9,10 +9,13 @@ This is to prevent privilege escalation.
 
 Users cannot create ProjectRoleTemplateBindings that violate the following constraints:
 
+- The `ProjectName` field must be:
+    - Provided as a non-empty value
+    - Specified using the format of `cluster-id:project-id`
+    - `project-id`, in particular, must match the namespace of the ProjectRoleTemplateBinding
 - Either a user subject (through `UserName` or `UserPrincipalName`), or a group subject (through `GroupName`
   or `GroupPrincipalName`), or a service account subject (through `ServiceAccount`) must be specified. Exactly one
   subject type of the three must be provided.
-- `ProjectName` must be specified
 - The roleTemplate indicated in `RoleTemplateName` must be:
     - Provided as a non-empty value
     - Valid (there must exist a `roleTemplate` object of given name in the `management.cattle.io/v3` API group)

pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator.go

@@ -104,8 +104,6 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp
 		}
 	}
 
-	clusterNS, projectNS := clusterFromProject(prtb.ProjectName)
-
 	roleTemplate, err := a.roleTemplateResolver.RoleTemplateCache().Get(prtb.RoleTemplateName)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
@@ -121,6 +119,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp
 		return nil, fmt.Errorf("failed to get rules from referenced roleTemplate '%s': %w", roleTemplate.Name, err)
 	}
 
+	clusterNS, projectNS := clusterAndProjectID(prtb.ProjectName)
 	err = auth.ConfirmNoEscalation(request, rules, clusterNS, a.clusterResolver)
 	if err == nil {
 		return &admissionv1.AdmissionResponse{Allowed: true}, nil
@@ -132,8 +131,8 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp
 	return response, nil
 }
 
-func clusterFromProject(project string) (string, string) {
-	pieces := strings.Split(project, ":")
+func clusterAndProjectID(projectName string) (string, string) {
+	pieces := strings.Split(projectName, ":")
 	if len(pieces) < 2 {
 		return "", ""
 	}
@@ -199,6 +198,11 @@ func (a *admitter) validateCreateFields(newPRTB *apisv3.ProjectRoleTemplateBindi
 		return field.NotSupported(fieldPath.Child("roleTemplate", "context"), roleTemplate.Context, []string{projectContext})
 	}
 
+	_, projectNS := clusterAndProjectID(newPRTB.ProjectName)
+	if projectNS != newPRTB.Namespace {
+		return field.Forbidden(fieldPath, "namespace and the project ID part of projectName must match")
+	}
+
 	return nil
 }
 

pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go

@@ -849,7 +849,7 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() {
 			allowed: false,
 		},
 		{
-			name: "create with unset project name",
+			name: "unset project name",
 			args: args{
 				username: adminUser,
 				oldPRTB: func() *apisv3.ProjectRoleTemplateBinding {
@@ -863,11 +863,28 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() {
 			},
 			allowed: false,
 		},
+		{
+			name: "namespace and the project id part of the project name differ",
+			args: args{
+				username: adminUser,
+				oldPRTB: func() *apisv3.ProjectRoleTemplateBinding {
+					return nil
+				},
+				newPRTB: func() *apisv3.ProjectRoleTemplateBinding {
+					basePRTB := newBasePRTB()
+					basePRTB.ObjectMeta.Namespace = "default"
+					basePRTB.ProjectName = fmt.Sprintf("%s:%s", clusterID, "p-cgtq4")
+					return basePRTB
+				},
+			},
+			allowed: false,
+		},
 	}
 
 	for i := range tests {
 		test := tests[i]
 		p.Run(test.name, func() {
+			p.T().Parallel()
 			req := createPRTBRequest(p.T(), test.args.oldPRTB(), test.args.newPRTB(), test.args.username)
 			admitters := validator.Admitters()
 			p.Len(admitters, 1)
@@ -926,7 +943,7 @@ func newBasePRTB() *apisv3.ProjectRoleTemplateBinding {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:              "PRTB-new",
 			GenerateName:      "PRTB-",
-			Namespace:         "p-namespace",
+			Namespace:         projectID,
 			UID:               "6534e4ef-f07b-4c61-b88d-95a92cce4852",
 			ResourceVersion:   "1",
 			Generation:        1,

tests/integration/projectRoleTemplateBinding_test.go

@@ -1,6 +1,8 @@
 package integration_test
 
 import (
+	"fmt"
+
 	v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
 	rbacv1 "k8s.io/api/rbac/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -17,7 +19,7 @@ func (m *IntegrationSuite) TestProjectRoleTemplateBinding() {
 		},
 		UserName:         "bruce-wayne",
 		RoleTemplateName: rtName,
-		ProjectName:      "gotham:city",
+		ProjectName:      fmt.Sprintf("%s:%s", "gotham", testNamespace),
 	}
 	invalidCreate := func() *v3.ProjectRoleTemplateBinding {
 		invalidCreate := validCreateObj.DeepCopy()