Merge pull request #22728 from enj/enj/i/oauth_server_auth

Bug 1703506: Move OAuth server to standard handler chain

Author: openshift-merge-robot

pkg/cmd/openshift-integrated-oauth-server/server.go

@@ -3,7 +3,9 @@ package openshift_integrated_oauth_server
 import (
 	"errors"
 
+	"k8s.io/apiserver/pkg/authentication/user"
 	genericapiserver "k8s.io/apiserver/pkg/server"
+	genericapiserveroptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 
 	osinv1 "github.com/openshift/api/osin/v1"
@@ -48,20 +50,41 @@ func newOAuthServerConfig(osinConfig *osinv1.OsinServerConfig) (*oauthserver.OAu
 	// the oauth-server must only run in http1 to avoid http2 connection re-use problems when improperly re-using a wildcard certificate
 	genericConfig.Config.SecureServing.HTTP1Only = true
 
+	authenticationOptions := genericapiserveroptions.NewDelegatingAuthenticationOptions()
+	authenticationOptions.ClientCert.ClientCA = osinConfig.ServingInfo.ClientCA
+	authenticationOptions.RemoteKubeConfigFile = osinConfig.KubeClientConfig.KubeConfig
+	if err := authenticationOptions.ApplyTo(&genericConfig.Authentication, genericConfig.SecureServing, genericConfig.OpenAPIConfig); err != nil {
+		return nil, err
+	}
+
+	authorizationOptions := genericapiserveroptions.NewDelegatingAuthorizationOptions().
+		// TODO better formalize / generate this list as trailing * matters
+		WithAlwaysAllowPaths( // The five sections are:
+			"/healthz", "/healthz/", // 1. Health checks (root, no wildcard)
+			"/oauth/*",           // 2. OAuth (wildcard)
+			"/login", "/login/*", // 3. Login (both root and wildcard)
+			"/logout", "/logout/", // 4. Logout (root, no wildcard)
+			"/oauth2callback/*", // 5. OAuth callbacks (wildcard)
+		).
+		WithAlwaysAllowGroups(user.SystemPrivilegedGroup)
+	authorizationOptions.RemoteKubeConfigFile = osinConfig.KubeClientConfig.KubeConfig
+	if err := authorizationOptions.ApplyTo(&genericConfig.Authorization); err != nil {
+		return nil, err
+	}
+
 	// TODO You need real overrides for rate limiting
 	kubeClientConfig, err := helpers.GetKubeConfigOrInClusterConfig(osinConfig.KubeClientConfig.KubeConfig, osinConfig.KubeClientConfig.ConnectionOverrides)
 	if err != nil {
 		return nil, err
 	}
 
-	oauthServerConfig, err := oauthserver.NewOAuthServerConfig(osinConfig.OAuthConfig, kubeClientConfig)
+	oauthServerConfig, err := oauthserver.NewOAuthServerConfig(osinConfig.OAuthConfig, kubeClientConfig, genericConfig)
 	if err != nil {
 		return nil, err
 	}
 
 	// TODO you probably want to set this
 	oauthServerConfig.GenericConfig.CorsAllowedOriginList = osinConfig.CORSAllowedOrigins
-	oauthServerConfig.GenericConfig.SecureServing = genericConfig.SecureServing
 	//oauthServerConfig.GenericConfig.AuditBackend = genericConfig.AuditBackend
 	//oauthServerConfig.GenericConfig.AuditPolicyChecker = genericConfig.AuditPolicyChecker
 

pkg/cmd/openshift-kube-apiserver/openshiftkubeapiserver/patch_oauthserver.go

@@ -11,7 +11,7 @@ import (
 // TODO this is taking a very large config for a small piece of it.  The information must be broken up at some point so that
 // we can run this in a pod.  This is an indication of leaky abstraction because it spent too much time in openshift start
 func NewOAuthServerConfigFromMasterConfig(genericConfig *genericapiserver.Config, oauthConfig *osinv1.OAuthConfig) (*oauthserver.OAuthServerConfig, error) {
-	oauthServerConfig, err := oauthserver.NewOAuthServerConfig(*oauthConfig, genericConfig.LoopbackClientConfig)
+	oauthServerConfig, err := oauthserver.NewOAuthServerConfig(*oauthConfig, genericConfig.LoopbackClientConfig, nil)
 	if err != nil {
 		return nil, err
 	}

pkg/oauthserver/oauthserver/oauth_apiserver.go

@@ -12,11 +12,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
-	genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
-	"k8s.io/apiserver/pkg/features"
 	genericapiserver "k8s.io/apiserver/pkg/server"
-	genericfilters "k8s.io/apiserver/pkg/server/filters"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	kclientset "k8s.io/client-go/kubernetes"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
@@ -47,7 +43,7 @@ func init() {
 
 // TODO we need to switch the oauth server to an external type, but that can be done after we get our externally facing flag values fixed
 // TODO remaining bits involve the session file, LDAP util code, validation, ...
-func NewOAuthServerConfig(oauthConfig osinv1.OAuthConfig, userClientConfig *rest.Config) (*OAuthServerConfig, error) {
+func NewOAuthServerConfig(oauthConfig osinv1.OAuthConfig, userClientConfig *rest.Config, genericConfig *genericapiserver.RecommendedConfig) (*OAuthServerConfig, error) {
 	// TODO: there is probably some better way to do this
 	decoder := codecs.UniversalDecoder(osinv1.GroupVersion)
 	for i, idp := range oauthConfig.IdentityProviders {
@@ -62,7 +58,11 @@ func NewOAuthServerConfig(oauthConfig osinv1.OAuthConfig, userClientConfig *rest
 		oauthConfig.IdentityProviders[i].Provider.Object = idpObject
 	}
 
-	genericConfig := genericapiserver.NewRecommendedConfig(codecs)
+	// this leaves the embedded OAuth server code path alone
+	if genericConfig == nil {
+		genericConfig = genericapiserver.NewRecommendedConfig(codecs)
+	}
+
 	genericConfig.LoopbackClientConfig = userClientConfig
 
 	userClient, err := userclient.NewForConfig(userClientConfig)
@@ -277,21 +277,26 @@ func (c completedOAuthConfig) New(delegationTarget genericapiserver.DelegationTa
 }
 
 func (c *OAuthServerConfig) buildHandlerChainForOAuth(startingHandler http.Handler, genericConfig *genericapiserver.Config) http.Handler {
+	// add OAuth handlers on top of the generic API server handlers
 	handler, err := c.WithOAuth(startingHandler)
 	if err != nil {
-		// the existing errors all cause the server to die anyway
+		// the existing errors all cause the OAuth server to die anyway
 		panic(err)
 	}
-	if utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) {
-		handler = genericapifilters.WithAudit(handler, genericConfig.AuditBackend, genericConfig.AuditPolicyChecker, genericConfig.LongRunningFunc)
-	}
 
-	handler = genericfilters.WithMaxInFlightLimit(handler, genericConfig.MaxRequestsInFlight, genericConfig.MaxMutatingRequestsInFlight, genericConfig.LongRunningFunc)
-	handler = genericfilters.WithCORS(handler, genericConfig.CorsAllowedOriginList, nil, nil, nil, "true")
-	handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, genericConfig.LongRunningFunc, genericConfig.RequestTimeout)
-	handler = genericapifilters.WithRequestInfo(handler, genericapiserver.NewRequestInfoResolver(genericConfig))
+	// add back the Authorization header so that WithOAuth can use it even after WithAuthentication deletes it
+	// WithOAuth sees users' passwords and can mint tokens so this is not really an issue
+	handler = headers.WithRestoreAuthorizationHeader(handler)
+
+	// this is the normal kube handler chain
+	handler = genericapiserver.DefaultBuildHandlerChain(handler, genericConfig)
+
+	// store a copy of the Authorization header for later use
+	handler = headers.WithPreserveAuthorizationHeader(handler)
+
+	// protected endpoints should not be cached
 	handler = headers.WithStandardHeaders(handler)
-	handler = genericfilters.WithPanicRecovery(handler)
+
 	return handler
 }
 

pkg/oauthserver/server/headers/oauthbasic.go

@@ -0,0 +1,29 @@
+package headers
+
+import "net/http"
+
+const (
+	authzHeader     = "Authorization"
+	copyAuthzHeader = "oauth.openshift.io:" + authzHeader // will never conflict because : is not a valid header key
+)
+
+func WithPreserveAuthorizationHeader(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if vv, ok := r.Header[authzHeader]; ok {
+			r.Header[copyAuthzHeader] = vv // capture the values before they are deleted
+		}
+
+		handler.ServeHTTP(w, r)
+	})
+}
+
+func WithRestoreAuthorizationHeader(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if vv, ok := r.Header[copyAuthzHeader]; ok {
+			r.Header[authzHeader] = vv // add them back afterwards for use in OAuth flows
+			delete(r.Header, copyAuthzHeader)
+		}
+
+		handler.ServeHTTP(w, r)
+	})
+}