Merge pull request #42652 from liggitt/ctx-namespace

pkg/registry/rbac/clusterrolebinding/policybased/BUILD

@@ -16,6 +16,7 @@ go_library(
         "//pkg/registry/rbac:go_default_library",
         "//pkg/registry/rbac/validation:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
         "//vendor:k8s.io/apiserver/pkg/endpoints/request",

pkg/registry/rbac/clusterrolebinding/policybased/storage.go

@@ -19,6 +19,7 @@ package policybased
 
 import (
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
@@ -48,11 +49,11 @@ func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (run
 	}
 
 	clusterRoleBinding := obj.(*rbac.ClusterRoleBinding)
-	if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace, s.authorizer) {
+	if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, metav1.NamespaceNone, s.authorizer) {
 		return s.StandardStorage.Create(ctx, obj)
 	}
 
-	rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace)
+	rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, metav1.NamespaceNone)
 	if err != nil {
 		return nil, err
 	}
@@ -71,12 +72,12 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up
 		clusterRoleBinding := obj.(*rbac.ClusterRoleBinding)
 
 		// if we're explicitly authorized to bind this clusterrole, return
-		if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace, s.authorizer) {
+		if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, metav1.NamespaceNone, s.authorizer) {
 			return obj, nil
 		}
 
 		// Otherwise, see if we already have all the permissions contained in the referenced clusterrole
-		rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace)
+		rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, metav1.NamespaceNone)
 		if err != nil {
 			return nil, err
 		}

pkg/registry/rbac/escalation_check.go

@@ -85,7 +85,7 @@ func BindingAuthorized(ctx genericapirequest.Context, roleRef rbac.RoleRef, bind
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf(
 			"error authorizing user %#v to bind %#v in namespace %s: %v",
-			roleRef, bindingNamespace, user, err,
+			user, roleRef, bindingNamespace, err,
 		))
 	}
 	return ok

pkg/registry/rbac/rolebinding/policybased/storage.go

@@ -47,12 +47,19 @@ func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (run
 		return s.StandardStorage.Create(ctx, obj)
 	}
 
+	// Get the namespace from the context (populated from the URL).
+	// The namespace in the object can be empty until StandardStorage.Create()->BeforeCreate() populates it from the context.
+	namespace, ok := genericapirequest.NamespaceFrom(ctx)
+	if !ok {
+		return nil, errors.NewBadRequest("namespace is required")
+	}
+
 	roleBinding := obj.(*rbac.RoleBinding)
-	if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, roleBinding.Namespace, s.authorizer) {
+	if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, namespace, s.authorizer) {
 		return s.StandardStorage.Create(ctx, obj)
 	}
 
-	rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, roleBinding.Namespace)
+	rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -68,15 +75,22 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up
 	}
 
 	nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) {
+		// Get the namespace from the context (populated from the URL).
+		// The namespace in the object can be empty until StandardStorage.Update()->BeforeUpdate() populates it from the context.
+		namespace, ok := genericapirequest.NamespaceFrom(ctx)
+		if !ok {
+			return nil, errors.NewBadRequest("namespace is required")
+		}
+
 		roleBinding := obj.(*rbac.RoleBinding)
 
 		// if we're explicitly authorized to bind this role, return
-		if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, roleBinding.Namespace, s.authorizer) {
+		if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, namespace, s.authorizer) {
 			return obj, nil
 		}
 
 		// Otherwise, see if we already have all the permissions contained in the referenced role
-		rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, roleBinding.Namespace)
+		rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, namespace)
 		if err != nil {
 			return nil, err
 		}

test/integration/auth/rbac_test.go

@@ -352,8 +352,16 @@ func TestRBAC(t *testing.T) {
 					},
 					{
 						ObjectMeta: metav1.ObjectMeta{Name: "create-rolebindings", Namespace: "job-namespace"},
-						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer-namespace"}},
-						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "create-rolebindings"},
+						Subjects: []rbacapi.Subject{
+							{Kind: "User", Name: "job-writer-namespace"},
+							{Kind: "User", Name: "any-rolebinding-writer-namespace"},
+						},
+						RoleRef: rbacapi.RoleRef{Kind: "ClusterRole", Name: "create-rolebindings"},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "bind-any-clusterrole", Namespace: "job-namespace"},
+						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "any-rolebinding-writer-namespace"}},
+						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "bind-any-clusterrole"},
 					},
 				},
 			},
@@ -384,6 +392,8 @@ func TestRBAC(t *testing.T) {
 
 				// cannot bind role anywhere
 				{"user-with-no-permissions", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
+				// can only bind role in namespace where they have explicit bind permission
+				{"any-rolebinding-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "forbidden-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
 				// can only bind role in namespace where they have covering permissions
 				{"job-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "forbidden-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
 				{"job-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusCreated},
@@ -396,6 +406,8 @@ func TestRBAC(t *testing.T) {
 				// can bind role because they have explicit bind permission
 				{"any-rolebinding-writer", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusCreated},
 				{superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "pi", "", http.StatusOK},
+				{"any-rolebinding-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusCreated},
+				{superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "pi", "", http.StatusOK},
 			},
 		},
 	}
@@ -434,12 +446,6 @@ func TestRBAC(t *testing.T) {
 						sub += fmt.Sprintf(",\"resourceVersion\": \"%v\"", resVersion)
 					}
 				}
-				// For any creation requests, add the namespace to the object meta.
-				if r.verb == "POST" || r.verb == "PUT" {
-					if r.namespace != "" {
-						sub += fmt.Sprintf(",\"namespace\": %q", r.namespace)
-					}
-				}
 				body = strings.NewReader(fmt.Sprintf(r.body, sub))
 			}