diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD index 63b81475e68..28aec0fa0ab 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD @@ -73,6 +73,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/discovery:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers/negotiation:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index 23d13adc3d3..695c62b5984 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -28,6 +28,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/discovery" "k8s.io/apiserver/pkg/registry/rest" openapicommon "k8s.io/kube-openapi/pkg/common" @@ -71,6 +72,11 @@ type APIGroupVersion struct { Linker runtime.SelfLinker UnsafeConvertor runtime.ObjectConvertor + // Authorizer determines whether a user is allowed to make a certain request. The Handler does a preliminary + // authorization check using the request URI but it may be necessary to make additional checks, such as in + // the create-on-update case + Authorizer authorizer.Authorizer + Admit admission.Interface MinRequestTimeout time.Duration diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 01597859f5d..21c5d1e53de 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -70,6 +70,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/audit:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers/negotiation:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/metrics:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index d42c0194dd6..a941f6fbaf4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/metrics" "k8s.io/apiserver/pkg/endpoints/request" @@ -54,6 +55,7 @@ type RequestScope struct { Defaulter runtime.ObjectDefaulter Typer runtime.ObjectTyper UnsafeConvertor runtime.ObjectConvertor + Authorizer authorizer.Authorizer TableConvertor rest.TableConvertor OpenAPISchema openapiproto.Schema diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index de242771db9..481a981242b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + "sync" "time" "k8s.io/apimachinery/pkg/api/errors" @@ -27,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -102,6 +104,19 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac }) } + createAuthorizerAttributes := authorizer.AttributesRecord{ + User: userInfo, + ResourceRequest: true, + Path: req.URL.Path, + Verb: "create", + APIGroup: scope.Resource.Group, + APIVersion: scope.Resource.Version, + Resource: scope.Resource.Resource, + Subresource: scope.Subresource, + Namespace: namespace, + Name: name, + } + trace.Step("About to store object in database") wasCreated := false result, err := finishRequest(timeout, func() (runtime.Object, error) { @@ -109,7 +124,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac ctx, name, rest.DefaultUpdatedObjectInfo(obj, transformers...), - rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes), + withAuthorization(rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes), scope.Authorizer, createAuthorizerAttributes), rest.AdmissionToValidateObjectUpdateFunc(admit, staticAdmissionAttributes), ) wasCreated = created @@ -140,3 +155,35 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac transformResponseObject(ctx, scope, req, w, status, result) } } + +func withAuthorization(validate rest.ValidateObjectFunc, a authorizer.Authorizer, attributes authorizer.Attributes) rest.ValidateObjectFunc { + var once sync.Once + var authorizerDecision authorizer.Decision + var authorizerReason string + var authorizerErr error + return func(obj runtime.Object) error { + if a == nil { + return errors.NewInternalError(fmt.Errorf("no authorizer provided, unable to authorize a create on update")) + } + once.Do(func() { + authorizerDecision, authorizerReason, authorizerErr = a.Authorize(attributes) + }) + // an authorizer like RBAC could encounter evaluation errors and still allow the request, so authorizer decision is checked before error here. + if authorizerDecision == authorizer.DecisionAllow { + // Continue to validating admission + return validate(obj) + } + if authorizerErr != nil { + return errors.NewInternalError(authorizerErr) + } + + // The user is not authorized to perform this action, so we need to build the error response + gr := schema.GroupResource{ + Group: attributes.GetAPIGroup(), + Resource: attributes.GetResource(), + } + name := attributes.GetName() + err := fmt.Errorf("%v", authorizerReason) + return errors.NewForbidden(gr, name, err) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 3edd09dcdf9..f3b66479bc1 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -483,6 +483,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Defaulter: a.group.Defaulter, Typer: a.group.Typer, UnsafeConvertor: a.group.UnsafeConvertor, + Authorizer: a.group.Authorizer, // TODO: Check for the interface on storage TableConvertor: tableProvider, diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 41ae90af689..03e9b18aec0 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -464,6 +464,7 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G admissionControl: c.AdmissionControl, Serializer: c.Serializer, AuditBackend: c.AuditBackend, + Authorizer: c.Authorization.Authorizer, delegationTarget: delegationTarget, HandlerChainWaitGroup: c.HandlerChainWaitGroup, diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 8c69491343f..b6e500c6123 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -36,6 +36,7 @@ import ( utilwaitgroup "k8s.io/apimachinery/pkg/util/waitgroup" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/authorization/authorizer" genericapi "k8s.io/apiserver/pkg/endpoints" "k8s.io/apiserver/pkg/endpoints/discovery" "k8s.io/apiserver/pkg/registry/rest" @@ -138,6 +139,11 @@ type GenericAPIServer struct { // auditing. The backend is started after the server starts listening. AuditBackend audit.Backend + // Authorizer determines whether a user is allowed to make a certain request. The Handler does a preliminary + // authorization check using the request URI but it may be necessary to make additional checks, such as in + // the create-on-update case + Authorizer authorizer.Authorizer + // enableAPIResponseCompression indicates whether API Responses should support compression // if the client requests it via Accept-Encoding enableAPIResponseCompression bool @@ -422,6 +428,7 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV MinRequestTimeout: s.minRequestTimeout, EnableAPIResponseCompression: s.enableAPIResponseCompression, OpenAPIConfig: s.openAPIConfig, + Authorizer: s.Authorizer, } } diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 448b51c93a8..6f642a5b113 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -219,6 +219,15 @@ var ( } } } +` + aLimitRange = ` +{ + "apiVersion": "v1", + "kind": "LimitRange", + "metadata": { + "name": "a"%s + } +} ` podNamespace = ` { @@ -246,6 +255,15 @@ var ( "name": "forbidden-namespace"%s } } +` + limitRangeNamespace = ` +{ + "apiVersion": "` + testapi.Groups[api.GroupName].GroupVersion().String() + `", + "kind": "Namespace", + "metadata": { + "name": "limitrange-namespace"%s + } +} ` ) @@ -409,6 +427,40 @@ func TestRBAC(t *testing.T) { {superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "pi", "", http.StatusOK}, }, }, + { + bootstrapRoles: bootstrapRoles{ + clusterRoles: []rbacapi.ClusterRole{ + { + ObjectMeta: metav1.ObjectMeta{Name: "allow-all"}, + Rules: []rbacapi.PolicyRule{ruleAllowAll}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "update-limitranges"}, + Rules: []rbacapi.PolicyRule{ + rbacapi.NewRule("update").Groups("").Resources("limitranges").RuleOrDie(), + }, + }, + }, + clusterRoleBindings: []rbacapi.ClusterRoleBinding{ + { + ObjectMeta: metav1.ObjectMeta{Name: "update-limitranges"}, + Subjects: []rbacapi.Subject{ + {Kind: "User", Name: "limitrange-updater"}, + }, + RoleRef: rbacapi.RoleRef{Kind: "ClusterRole", Name: "update-limitranges"}, + }, + }, + }, + requests: []request{ + // Create the namespace used later in the test + {superUser, "POST", "", "namespaces", "", "", limitRangeNamespace, http.StatusCreated}, + + {"limitrange-updater", "PUT", "", "limitranges", "limitrange-namespace", "a", aLimitRange, http.StatusForbidden}, + {superUser, "PUT", "", "limitranges", "limitrange-namespace", "a", aLimitRange, http.StatusCreated}, + {superUser, "PUT", "", "limitranges", "limitrange-namespace", "a", aLimitRange, http.StatusOK}, + {"limitrange-updater", "PUT", "", "limitranges", "limitrange-namespace", "a", aLimitRange, http.StatusOK}, + }, + }, } for i, tc := range tests { @@ -424,6 +476,7 @@ func TestRBAC(t *testing.T) { "job-writer-namespace": {Name: "job-writer-namespace"}, "nonescalating-rolebinding-writer": {Name: "nonescalating-rolebinding-writer"}, "pod-reader": {Name: "pod-reader"}, + "limitrange-updater": {Name: "limitrange-updater"}, "user-with-no-permissions": {Name: "user-with-no-permissions"}, })) _, s, closeFn := framework.RunAMaster(masterConfig)