Terser authentication logic in front-proxy

pkg/proxy/config.go

@@ -50,9 +50,8 @@ type ExtraConfig struct {
 	RootShardConfig   *rest.Config
 	ShardsConfig      *rest.Config
 
-	AuthenticationInfo    genericapiserver.AuthenticationInfo
-	ServingInfo           *genericapiserver.SecureServingInfo
-	AdditionalAuthEnabled bool
+	AuthenticationInfo genericapiserver.AuthenticationInfo
+	ServingInfo        *genericapiserver.SecureServingInfo
 }
 
 type CompletedConfig struct {
@@ -103,7 +102,5 @@ func NewConfig(ctx context.Context, opts *proxyoptions.Options) (*Config, error)
 	}
 	c.ShardsConfig.Wrap(kcpShardIdentityRoundTripper)
 
-	c.AdditionalAuthEnabled = c.Options.Authentication.AdditionalAuthEnabled()
-
 	return c, nil
 }

pkg/proxy/filters/filters.go

@@ -27,57 +27,44 @@ import (
 	authenticatorunion "k8s.io/apiserver/pkg/authentication/request/union"
 	genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/klog/v2"
 
 	"github.com/kcp-dev/kcp/pkg/authentication"
 )
 
-// WithOptionalAuthentication creates a handler that authenticates a request
-// if a ClientCert is presented but passes through to the next handler if one is
-// not. It will also call the authenticator present in the request context, if
-// any, to support per-workspace authentication.
-// When additionalAuthMethods is true we also attempt to authenticate even when
-// no client cert is detected in the request.
-func WithOptionalAuthentication(handler, failed http.Handler, auth authenticator.Request, additionalAuthMethods bool) http.Handler {
+// WithOptionalAuthentication creates a handler that authenticates
+// a request if credentials are presented but passes through to the next
+// handler if one is not.
+func WithOptionalAuthentication(handler, failed http.Handler, auth authenticator.Request) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		// Do not accidentally modify the variables in the scope of WithOptionalAuthentication().
-		var (
-			authenticator  = auth
-			additionalAuth = additionalAuthMethods
-		)
-
-		// If an authenticator for the workspace exists,
-		// it offers an alternative way to authenticate.
-		reqAuthenticator, ok := authentication.WorkspaceAuthenticatorFrom(req.Context())
-		if ok {
-			if auth != nil {
-				authenticator = authenticatorunion.New(auth, reqAuthenticator)
-			} else {
-				authenticator = reqAuthenticator
-			}
-			additionalAuth = true
+		// Collect all available authenticators: base (client cert, token file, etc.)
+		// plus any per-workspace authenticator resolved earlier in the chain.
+		authenticators := make([]authenticator.Request, 0, 2)
+		if auth != nil {
+			authenticators = append(authenticators, auth)
 		}
 
-		if authenticator == nil {
-			handler.ServeHTTP(w, req)
-			return
+		if wsAuth, ok := authentication.WorkspaceAuthenticatorFrom(req.Context()); ok {
+			authenticators = append(authenticators, wsAuth)
 		}
 
-		if (req.TLS == nil || len(req.TLS.PeerCertificates) == 0) && !additionalAuth {
+		// No authenticators configured
+		if len(authenticators) == 0 {
 			handler.ServeHTTP(w, req)
 			return
 		}
 
-		resp, ok, err := authenticator.AuthenticateRequest(req)
-		if err != nil || !ok {
-			if err != nil {
-				logger := klog.FromContext(req.Context())
-				logger.Error(err, "Unable to authenticate the request")
-			}
-			failed.ServeHTTP(w, req)
-			return
+		// Try to authenticate the request
+		combined := authenticatorunion.New(authenticators...)
+		resp, ok, err := combined.AuthenticateRequest(req)
+		if err == nil && ok {
+			// Add identity if authentication passed
+			req = req.WithContext(request.WithUser(req.Context(), resp.User))
+			// If this failed the request has to propagate to the shard.
+			// The request might be made with a static token which the
+			// front-proxy has no knowledge of.
+			// This will be superfluous in the future, see
+			// https://github.com/kcp-dev/kcp/issues/4100
 		}
-		req = req.WithContext(request.WithUser(req.Context(), resp.User))
 		handler.ServeHTTP(w, req)
 	})
 }

pkg/proxy/options/authentication.go

@@ -55,7 +55,6 @@ type Authentication struct {
 // NewAuthentication creates a default Authentication.
 func NewAuthentication() *Authentication {
 	auth := &Authentication{
-		// Note: when adding new auth methods, also update AdditionalAuthEnabled below
 		BuiltInOptions: kubeoptions.NewBuiltInAuthenticationOptions().
 			WithClientCert().
 			WithOIDC().
@@ -72,19 +71,6 @@ func NewAuthentication() *Authentication {
 	return auth
 }
 
-// When configured to enable auth other than ClientCert, this returns true.
-func (c *Authentication) AdditionalAuthEnabled() bool {
-	return c.tokenAuthEnabled() || c.serviceAccountAuthEnabled() || c.oidcAuthEnabled()
-}
-
-func (c *Authentication) oidcAuthEnabled() bool {
-	return c.BuiltInOptions.OIDC != nil && c.BuiltInOptions.OIDC.IssuerURL != ""
-}
-
-func (c *Authentication) tokenAuthEnabled() bool {
-	return c.BuiltInOptions.TokenFile != nil && c.BuiltInOptions.TokenFile.TokenFile != ""
-}
-
 func (c *Authentication) serviceAccountAuthEnabled() bool {
 	return c.BuiltInOptions.ServiceAccounts != nil && len(c.BuiltInOptions.ServiceAccounts.KeyFiles) != 0
 }

pkg/proxy/server.go

@@ -100,16 +100,11 @@ func NewServer(ctx context.Context, c CompletedConfig) (*Server, error) {
 		return s, err
 	}
 
-	// The optional auth handler will call the underlying authenticator only if
-	// auth methods are configured directly on the front-proxy *or* if there is
-	// a custom workspace authenticator, i.e. the AdditionalAuthEnabled field
-	// only represents the CLI flag state.
 	failedHandler := frontproxyfilters.NewUnauthorizedHandler()
 	handler = frontproxyfilters.WithOptionalAuthentication(
 		handler,
 		failedHandler,
-		s.completedConfig.AuthenticationInfo.Authenticator,
-		s.CompletedConfig.AdditionalAuthEnabled)
+		s.completedConfig.AuthenticationInfo.Authenticator)
 
 	// Make the per-workspace authenticator available to the previous middleware
 	// by hooking up a handler and a runtime index.