Merge pull request #16110 from deads2k/server-40-normal-authz

Automatic merge from submit-queue

use the upstream authorization filters

This brings in a config option from upstream and uses it to remove custom handler code we had.

@openshift/sig-security

Author: openshift-merge-robot

pkg/authorization/authorizer/attributes_builder.go

@@ -9,7 +9,7 @@ import (
 
 type browserSafeRequestInfoResolver struct {
 	// infoFactory is used to determine info for the request
-	infoFactory RequestInfoFactory
+	infoFactory apirequest.RequestInfoResolver
 
 	// contextMapper is used to look up the context corresponding to a request
 	// to obtain the user associated with the request
@@ -19,7 +19,7 @@ type browserSafeRequestInfoResolver struct {
 	authenticatedGroups sets.String
 }
 
-func NewBrowserSafeRequestInfoResolver(contextMapper apirequest.RequestContextMapper, authenticatedGroups sets.String, infoFactory RequestInfoFactory) RequestInfoFactory {
+func NewBrowserSafeRequestInfoResolver(contextMapper apirequest.RequestContextMapper, authenticatedGroups sets.String, infoFactory apirequest.RequestInfoResolver) apirequest.RequestInfoResolver {
 	return &browserSafeRequestInfoResolver{
 		contextMapper:       contextMapper,
 		authenticatedGroups: authenticatedGroups,

pkg/authorization/authorizer/browser_safe_request_info_resolver.go

@@ -1,25 +1,14 @@
 package authorizer
 
 import (
-	"net/http"
-
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
-	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 type SubjectLocator interface {
 	GetAllowedSubjects(attributes authorizer.Attributes) (sets.String, sets.String, error)
 }
 
-type AuthorizationAttributeBuilder interface {
-	GetAttributes(request *http.Request) (authorizer.Attributes, error)
-}
-
-type RequestInfoFactory interface {
-	NewRequestInfo(req *http.Request) (*apirequest.RequestInfo, error)
-}
-
 // ForbiddenMessageMaker creates a forbidden message from Attributes
 type ForbiddenMessageMaker interface {
 	MakeMessage(attrs authorizer.Attributes) (string, error)

pkg/authorization/authorizer/interfaces.go

@@ -8,17 +8,18 @@ import (
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/endpoints/request"
+	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	kapi "k8s.io/kubernetes/pkg/api"
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
 )
 
 type personalSARRequestInfoResolver struct {
 	// infoFactory is used to determine info for the request
-	infoFactory RequestInfoFactory
+	infoFactory apirequest.RequestInfoResolver
 }
 
-func NewPersonalSARRequestInfoResolver(infoFactory RequestInfoFactory) RequestInfoFactory {
+func NewPersonalSARRequestInfoResolver(infoFactory apirequest.RequestInfoResolver) apirequest.RequestInfoResolver {
 	return &personalSARRequestInfoResolver{
 		infoFactory: infoFactory,
 	}

pkg/authorization/authorizer/personal_subjectaccessreview.go

@@ -3,21 +3,21 @@ package authorizer
 import (
 	"net/http"
 
-	"k8s.io/apiserver/pkg/endpoints/request"
+	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
 type projectRequestInfoResolver struct {
 	// infoFactory is used to determine info for the request
-	infoFactory RequestInfoFactory
+	infoFactory apirequest.RequestInfoResolver
 }
 
-func NewProjectRequestInfoResolver(infoFactory RequestInfoFactory) RequestInfoFactory {
+func NewProjectRequestInfoResolver(infoFactory apirequest.RequestInfoResolver) apirequest.RequestInfoResolver {
 	return &projectRequestInfoResolver{
 		infoFactory: infoFactory,
 	}
 }
 
-func (a *projectRequestInfoResolver) NewRequestInfo(req *http.Request) (*request.RequestInfo, error) {
+func (a *projectRequestInfoResolver) NewRequestInfo(req *http.Request) (*apirequest.RequestInfo, error) {
 	requestInfo, err := a.infoFactory.NewRequestInfo(req)
 	if err != nil {
 		return requestInfo, err

pkg/authorization/authorizer/project_request_info_resolver.go

@@ -14,10 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	kauthorizer "k8s.io/apiserver/pkg/authorization/authorizer"
-	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	kapi "k8s.io/kubernetes/pkg/api"
-
-	"github.com/openshift/origin/pkg/authorization/authorizer"
 )
 
 type bypassAuthorizer struct {
@@ -38,33 +35,6 @@ func (a bypassAuthorizer) Authorize(attributes kauthorizer.Attributes) (allowed
 	return a.authorizer.Authorize(attributes)
 }
 
-// AuthorizationFilter imposes normal authorization rules
-func AuthorizationFilter(handler http.Handler, authorizer kauthorizer.Authorizer, authorizationAttributeBuilder authorizer.AuthorizationAttributeBuilder, contextMapper apirequest.RequestContextMapper) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		attributes, err := authorizationAttributeBuilder.GetAttributes(req)
-		if err != nil {
-			Forbidden(err.Error(), attributes, w, req)
-			return
-		}
-		if attributes == nil {
-			Forbidden("No attributes", attributes, w, req)
-			return
-		}
-
-		allowed, reason, err := authorizer.Authorize(attributes)
-		if err != nil {
-			Forbidden(err.Error(), attributes, w, req)
-			return
-		}
-		if !allowed {
-			Forbidden(reason, attributes, w, req)
-			return
-		}
-
-		handler.ServeHTTP(w, req)
-	})
-}
-
 // Forbidden renders a simple forbidden error to the response
 func Forbidden(reason string, attributes kauthorizer.Attributes, w http.ResponseWriter, req *http.Request) {
 	kind := ""

pkg/cmd/server/handlers/authorization.go

@@ -59,9 +59,11 @@ import (
 	kversion "k8s.io/kubernetes/pkg/version"
 
 	"github.com/openshift/origin/pkg/api"
+	oauthorizer "github.com/openshift/origin/pkg/authorization/authorizer"
 	"github.com/openshift/origin/pkg/authorization/authorizer/scope"
 	"github.com/openshift/origin/pkg/cmd/flagtypes"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
+	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	"github.com/openshift/origin/pkg/cmd/server/cm"
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/server/election"
@@ -449,6 +451,7 @@ func buildKubeApiserverConfig(
 	genericConfig.DisabledPostStartHooks.Insert("extensions/third-party-resources")
 	genericConfig.AdmissionControl = admissionControl
 	genericConfig.RequestContextMapper = requestContextMapper
+	genericConfig.RequestInfoResolver = openshiftRequestInfoResolver(genericConfig.RequestContextMapper)
 	genericConfig.OpenAPIConfig = defaultOpenAPIConfig(masterConfig)
 	genericConfig.SwaggerConfig = apiserver.DefaultSwaggerConfig()
 	genericConfig.SwaggerConfig.PostBuildHandler = customizeSwaggerDefinition
@@ -781,3 +784,18 @@ func readCAorNil(file string) ([]byte, error) {
 func newMasterLeases(storage storage.Interface, masterEndpointReconcileTTL int) election.Leases {
 	return election.NewLeases(storage, "/masterleases/", uint64(masterEndpointReconcileTTL))
 }
+
+func openshiftRequestInfoResolver(requestContextMapper apirequest.RequestContextMapper) apirequest.RequestInfoResolver {
+	// Default API request info factory
+	requestInfoFactory := &apirequest.RequestInfoFactory{APIPrefixes: sets.NewString("api", "osapi", "oapi", "apis"), GrouplessAPIPrefixes: sets.NewString("api", "osapi", "oapi")}
+	// Wrap with a request info factory that detects unsafe requests and modifies verbs/resources appropriately so policy can address them separately
+	browserSafeRequestInfoResolver := oauthorizer.NewBrowserSafeRequestInfoResolver(
+		requestContextMapper,
+		sets.NewString(bootstrappolicy.AuthenticatedGroup),
+		requestInfoFactory,
+	)
+	personalSARRequestInfoResolver := oauthorizer.NewPersonalSARRequestInfoResolver(browserSafeRequestInfoResolver)
+	projectRequestInfoResolver := oauthorizer.NewProjectRequestInfoResolver(personalSARRequestInfoResolver)
+
+	return projectRequestInfoResolver
+}

pkg/cmd/server/kubernetes/master/master_config.go

@@ -35,6 +35,7 @@ var expectedGroupPreferredVersions []string = []string{
 	"admissionregistration.k8s.io/v1alpha1",
 	"apps/v1beta1,authentication.k8s.io/v1",
 	"authorization.k8s.io/v1",
+	"authorization.openshift.io/v1",
 	"autoscaling/v1",
 	"batch/v1",
 	"certificates.k8s.io/v1beta1",

pkg/cmd/server/kubernetes/master/master_config_test.go

@@ -19,7 +19,6 @@ import (
 	authzwebhook "k8s.io/apiserver/plugin/pkg/authorizer/webhook"
 	clientgoclientset "k8s.io/client-go/kubernetes"
 
-	"github.com/openshift/origin/pkg/authorization/authorizer"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 	serverauthenticator "github.com/openshift/origin/pkg/cmd/server/authenticator"
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
@@ -57,11 +56,10 @@ func RunControllerServer(servingInfo configapi.HTTPServingInfo, kubeExternal cli
 	requestInfoResolver := apiserver.NewRequestInfoResolver(&apiserver.Config{})
 	// the request context mapper for controllers is always separate
 	requestContextMapper := apirequest.NewRequestContextMapper()
-	authorizationAttributeBuilder := authorizer.NewAuthorizationAttributeBuilder(requestContextMapper, requestInfoResolver)
 
 	// we use direct bypass to allow readiness and health to work regardless of the master health
 	authz := serverhandlers.NewBypassAuthorizer(remoteAuthz, "/healthz", "/healthz/ready")
-	handler := serverhandlers.AuthorizationFilter(mux, authz, authorizationAttributeBuilder, requestContextMapper)
+	handler := apifilters.WithAuthorization(mux, requestContextMapper, authz)
 	handler = apifilters.WithAuthentication(handler, requestContextMapper, authn, apifilters.Unauthorized(false))
 	handler = apiserverfilters.WithPanicRecovery(handler)
 	handler = apifilters.WithRequestInfo(handler, requestInfoResolver, requestContextMapper)

pkg/cmd/server/origin/controller/standalone_apiserver.go

@@ -287,7 +287,7 @@ func (c *MasterConfig) buildHandlerChain() (func(apiHandler http.Handler, kc *ap
 
 			// these are all equivalent to the kube handler chain
 			///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-			handler = serverhandlers.AuthorizationFilter(handler, c.Authorizer, c.AuthorizationAttributeBuilder, genericConfig.RequestContextMapper)
+			handler = apifilters.WithAuthorization(handler, c.RequestContextMapper, c.Authorizer)
 			handler = serverhandlers.ImpersonationFilter(handler, c.Authorizer, cache.NewGroupCache(c.UserInformers.User().InternalVersion().Groups()), genericConfig.RequestContextMapper)
 			// audit handler must comes before the impersonationFilter to read the original user
 			if c.Options.AuditConfig.Enabled {
@@ -318,7 +318,7 @@ func (c *MasterConfig) buildHandlerChain() (func(apiHandler http.Handler, kc *ap
 			// execution - updates vs reads, long reads vs short reads, fat reads vs skinny reads.
 			// NOTE: read vs. write is implemented in Kube 1.6+
 			handler = apiserverfilters.WithMaxInFlightLimit(handler, genericConfig.MaxRequestsInFlight, genericConfig.MaxMutatingRequestsInFlight, genericConfig.RequestContextMapper, genericConfig.LongRunningFunc)
-			handler = apifilters.WithRequestInfo(handler, apiserver.NewRequestInfoResolver(genericConfig), genericConfig.RequestContextMapper)
+			handler = apifilters.WithRequestInfo(handler, genericConfig.RequestInfoResolver, genericConfig.RequestContextMapper)
 			handler = apirequest.WithRequestContext(handler, genericConfig.RequestContextMapper)
 			handler = apiserverfilters.WithPanicRecovery(handler)
 			///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////