diff --git a/pkg/proxy/config.go b/pkg/proxy/config.go index c29f691b2bb..d4029fe2a32 100644 --- a/pkg/proxy/config.go +++ b/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 } diff --git a/pkg/proxy/filters/filters.go b/pkg/proxy/filters/filters.go index a38a05fcb24..72bcaaa5fce 100644 --- a/pkg/proxy/filters/filters.go +++ b/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) }) } diff --git a/pkg/proxy/options/authentication.go b/pkg/proxy/options/authentication.go index 7fb5f3f235c..d8b0754b7e6 100644 --- a/pkg/proxy/options/authentication.go +++ b/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 } diff --git a/pkg/proxy/server.go b/pkg/proxy/server.go index e0020b4f87e..c1a9316f73f 100644 --- a/pkg/proxy/server.go +++ b/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.