From 7f4e5c567695b50c2dd974b78e7b148323922c51 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 7 Mar 2017 11:11:48 -0500 Subject: [PATCH] Use namespace from context --- .../rbac/clusterrolebinding/policybased/BUILD | 1 + .../clusterrolebinding/policybased/storage.go | 9 ++++---- pkg/registry/rbac/escalation_check.go | 2 +- .../rbac/rolebinding/policybased/storage.go | 22 +++++++++++++++---- test/integration/auth/rbac_test.go | 22 ++++++++++++------- 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD b/pkg/registry/rbac/clusterrolebinding/policybased/BUILD index 9033c1170f..31d57583f9 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD +++ b/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", diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go index b45f97db6c..6794f88c69 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go +++ b/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 } diff --git a/pkg/registry/rbac/escalation_check.go b/pkg/registry/rbac/escalation_check.go index 5872504bd4..367e76942f 100644 --- a/pkg/registry/rbac/escalation_check.go +++ b/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 diff --git a/pkg/registry/rbac/rolebinding/policybased/storage.go b/pkg/registry/rbac/rolebinding/policybased/storage.go index f942bd391c..ad53ea3a6c 100644 --- a/pkg/registry/rbac/rolebinding/policybased/storage.go +++ b/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 } diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 67442644b3..6d4cb359b4 100644 --- a/test/integration/auth/rbac_test.go +++ b/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)) }