From 8af2d2422e1d038343c59f95abddca92f2028544 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Mon, 16 Mar 2026 12:51:14 +0000 Subject: [PATCH 01/11] Add disableUpstreamTokenInjection to embedded auth server config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The embedded auth server always injected upstream IdP tokens into requests forwarded to backend MCP servers. This made it impossible to use the embedded auth server for client-facing OAuth flows when the upstream MCP server is public and doesn't require authentication — the injected token caused 401 rejections from the upstream. Add a `disableUpstreamTokenInjection` field to EmbeddedAuthServerConfig that skips the upstream swap middleware while keeping the embedded auth server running for client authentication. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../v1beta1/mcpexternalauthconfig_types.go | 10 +++++ .../pkg/controllerutil/authserver.go | 3 ++ .../pkg/controllerutil/authserver_test.go | 39 +++++++++++++++++++ ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 10 +++++ ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 10 +++++ docs/operator/crd-api.md | 1 + pkg/authserver/config.go | 6 +++ pkg/runner/middleware.go | 10 ++++- pkg/runner/middleware_test.go | 18 +++++++++ 9 files changed, 105 insertions(+), 2 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index 5716fe83b1..fe2ac6bcec 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -230,6 +230,16 @@ type EmbeddedAuthServerConfig struct { // +optional Storage *AuthServerStorageConfig `json:"storage,omitempty"` + // DisableUpstreamTokenInjection prevents the embedded auth server from injecting + // upstream IdP tokens into requests forwarded to the backend MCP server. + // When true, the embedded auth server still handles OAuth flows for clients + // but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + // This is useful when the backend MCP server does not require authentication + // (e.g., public documentation servers) but you still want client authentication. + // +kubebuilder:default=false + // +optional + DisableUpstreamTokenInjection bool `json:"disableUpstreamTokenInjection,omitempty"` + // AllowedAudiences is the list of valid resource URIs that tokens can be issued for. // For an embedded auth server, this can be determined by the servers (MCP or vMCP) it serves. diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 177875da16..eee68565a9 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -581,6 +581,9 @@ func BuildAuthServerRunConfig( } config.Storage = storageCfg + // Wire through upstream token injection flag + config.DisableUpstreamTokenInjection = authConfig.DisableUpstreamTokenInjection + return config, nil } diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index c8cda64f38..bcd8f72eda 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -1423,6 +1423,45 @@ func TestBuildAuthServerRunConfig(t *testing.T) { "DCRConfig should remain nil when only ClientID is set") }, }, + { + name: "DisableUpstreamTokenInjection is wired through", + authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + DisableUpstreamTokenInjection: true, + }, + allowedAudiences: defaultAudiences, + scopesSupported: defaultScopes, + checkFunc: func(t *testing.T, config *authserver.RunConfig) { + t.Helper() + assert.True(t, config.DisableUpstreamTokenInjection, + "DisableUpstreamTokenInjection should be wired from CRD to RunConfig") + }, + }, + { + name: "DisableUpstreamTokenInjection defaults to false", + authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + allowedAudiences: defaultAudiences, + scopesSupported: defaultScopes, + checkFunc: func(t *testing.T, config *authserver.RunConfig) { + t.Helper() + assert.False(t, config.DisableUpstreamTokenInjection, + "DisableUpstreamTokenInjection should default to false") + }, + }, } for _, tt := range tests { diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 7988038f77..83517ad5a3 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1385,6 +1385,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index fb4b731da4..60de7b1760 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1388,6 +1388,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 44e36106de..1ecd2b7d54 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -1135,6 +1135,7 @@ _Appears in:_ | `tokenLifespans` _[api.v1beta1.TokenLifespanConfig](#apiv1beta1tokenlifespanconfig)_ | TokenLifespans configures the duration that various tokens are valid.
If not specified, defaults are applied (access: 1h, refresh: 7d, authCode: 10m). | | Optional: \{\}
| | `upstreamProviders` _[api.v1beta1.UpstreamProviderConfig](#apiv1beta1upstreamproviderconfig) array_ | UpstreamProviders configures connections to upstream Identity Providers.
The embedded auth server delegates authentication to these providers.
MCPServer and MCPRemoteProxy support a single upstream; VirtualMCPServer supports multiple. | | MinItems: 1
Required: \{\}
| | `storage` _[api.v1beta1.AuthServerStorageConfig](#apiv1beta1authserverstorageconfig)_ | Storage configures the storage backend for the embedded auth server.
If not specified, defaults to in-memory storage. | | Optional: \{\}
| +| `disableUpstreamTokenInjection` _boolean_ | DisableUpstreamTokenInjection prevents the embedded auth server from injecting
upstream IdP tokens into requests forwarded to the backend MCP server.
When true, the embedded auth server still handles OAuth flows for clients
but does not swap ToolHive JWTs for upstream tokens on outgoing requests.
This is useful when the backend MCP server does not require authentication
(e.g., public documentation servers) but you still want client authentication. | false | Optional: \{\}
| #### api.v1beta1.EmbeddingResourceOverrides diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index e82f4bdab8..852deacdeb 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -79,6 +79,12 @@ type RunConfig struct { // Storage configures the storage backend for the auth server. // If nil, defaults to in-memory storage. Storage *storage.RunConfig `json:"storage,omitempty" yaml:"storage,omitempty"` + + // DisableUpstreamTokenInjection prevents the upstream swap middleware from being added. + // When true, the embedded auth server handles OAuth flows for clients but does not + // inject upstream IdP tokens into requests forwarded to the backend MCP server. + //nolint:lll // field tags require full JSON+YAML names + DisableUpstreamTokenInjection bool `json:"disable_upstream_token_injection,omitempty" yaml:"disable_upstream_token_injection,omitempty"` } // SigningKeyRunConfig configures where to load signing keys from. diff --git a/pkg/runner/middleware.go b/pkg/runner/middleware.go index be9dd33506..9a1831062d 100644 --- a/pkg/runner/middleware.go +++ b/pkg/runner/middleware.go @@ -330,8 +330,9 @@ func addUsageMetricsMiddleware(middlewares []types.MiddlewareConfig, configDisab // addUpstreamSwapMiddleware adds upstream swap middleware if the embedded auth server is configured. // This middleware exchanges ToolHive JWTs for upstream IdP tokens. -// The middleware is only added when EmbeddedAuthServerConfig is set; if UpstreamSwapConfig -// is nil, default configuration values are used. +// The middleware is only added when EmbeddedAuthServerConfig is set and +// DisableUpstreamTokenInjection is false. If UpstreamSwapConfig is nil, +// default configuration values are used. func addUpstreamSwapMiddleware( middlewares []types.MiddlewareConfig, config *RunConfig, @@ -341,6 +342,11 @@ func addUpstreamSwapMiddleware( return middlewares, nil } + // Skip upstream token injection if explicitly disabled + if config.EmbeddedAuthServerConfig.DisableUpstreamTokenInjection { + return middlewares, nil + } + // Use provided config or defaults upstreamSwapConfig := config.UpstreamSwapConfig if upstreamSwapConfig == nil { diff --git a/pkg/runner/middleware_test.go b/pkg/runner/middleware_test.go index ee890dc52a..b8aa47ab9b 100644 --- a/pkg/runner/middleware_test.go +++ b/pkg/runner/middleware_test.go @@ -285,6 +285,15 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { }, wantAppended: true, }, + { + name: "EmbeddedAuthServerConfig with DisableUpstreamTokenInjection skips middleware", + config: func() *RunConfig { + cfg := createMinimalAuthServerConfig() + cfg.DisableUpstreamTokenInjection = true + return &RunConfig{EmbeddedAuthServerConfig: cfg} + }(), + wantAppended: false, + }, { name: "EmbeddedAuthServerConfig set with explicit UpstreamSwapConfig uses provided config", config: &RunConfig{ @@ -362,6 +371,15 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) { config: &RunConfig{EmbeddedAuthServerConfig: nil}, wantUpstreamSwap: false, }, + { + name: "DisableUpstreamTokenInjection omits upstream-swap", + config: func() *RunConfig { + cfg := createMinimalAuthServerConfig() + cfg.DisableUpstreamTokenInjection = true + return &RunConfig{EmbeddedAuthServerConfig: cfg} + }(), + wantUpstreamSwap: false, + }, { name: "explicit UpstreamSwapConfig is used", config: &RunConfig{ From b0cc5515b851311207ddeee41607c6983f31db53 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Tue, 17 Mar 2026 14:23:20 +0000 Subject: [PATCH 02/11] Fix transparent proxy for remote MCP servers behind redirects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues prevented MCPRemoteProxy from connecting to third-party upstream MCP servers that use HTTP redirects: 1. X-Forwarded-Host leaked the proxy's hostname to the upstream. The upstream used it to construct 307 redirect URLs pointing back to the proxy, creating a redirect loop. Fix: skip SetXForwarded() for remote upstreams (isRemote == true). 2. Go's http.Transport.RoundTrip does not follow redirects, but httputil.ReverseProxy uses Transport directly. Upstream 307/308 redirects (e.g. HTTPS→HTTP scheme changes, path canonicalization) were returned to the MCP client which cannot follow them through the proxy. Fix: add forwardFollowingRedirects that transparently follows up to 10 redirects, preserving method and body for 307/308 (RFC 7538). 3. When disableUpstreamTokenInjection is true, the client's ToolHive JWT was still forwarded to the upstream in the Authorization header. Fix: add strip-auth middleware that removes the Authorization header before forwarding. Also adds debug logging for outbound request headers and upstream response status codes to aid diagnosis of remote proxy issues. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/runner/middleware.go | 46 +++++++++++++++- pkg/runner/middleware_test.go | 54 +++++++++++-------- .../proxy/transparent/transparent_proxy.go | 25 ++++++++- 3 files changed, 101 insertions(+), 24 deletions(-) diff --git a/pkg/runner/middleware.go b/pkg/runner/middleware.go index 9a1831062d..81c4a438c9 100644 --- a/pkg/runner/middleware.go +++ b/pkg/runner/middleware.go @@ -5,6 +5,7 @@ package runner import ( "fmt" + "net/http" "github.com/stacklok/toolhive/pkg/audit" "github.com/stacklok/toolhive/pkg/auth" @@ -45,6 +46,7 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory { headerfwd.HeaderForwardMiddlewareName: headerfwd.CreateMiddleware, validating.MiddlewareType: validating.CreateMiddleware, mutating.MiddlewareType: mutating.CreateMiddleware, + stripAuthMiddlewareType: createStripAuthMiddleware, } } @@ -342,9 +344,10 @@ func addUpstreamSwapMiddleware( return middlewares, nil } - // Skip upstream token injection if explicitly disabled + // When upstream token injection is disabled, strip the Authorization header + // so the client's ToolHive JWT doesn't leak to the upstream server. if config.EmbeddedAuthServerConfig.DisableUpstreamTokenInjection { - return middlewares, nil + return addAuthHeaderStripMiddleware(middlewares) } // Use provided config or defaults @@ -402,6 +405,45 @@ func injectUpstreamProviderIfNeeded( return cedar.InjectUpstreamProvider(authzCfg, providerName) } +// stripAuthMiddlewareType is the type identifier for the auth header stripping middleware. +const stripAuthMiddlewareType = "strip-auth" + +// addAuthHeaderStripMiddleware adds a middleware that removes the Authorization header +// before forwarding to the upstream. This prevents the client's ToolHive JWT from +// leaking to upstream servers that don't expect it. +func addAuthHeaderStripMiddleware( + middlewares []types.MiddlewareConfig, +) ([]types.MiddlewareConfig, error) { + mwConfig, err := types.NewMiddlewareConfig(stripAuthMiddlewareType, struct{}{}) + if err != nil { + return nil, fmt.Errorf("failed to create strip-auth middleware config: %w", err) + } + return append(middlewares, *mwConfig), nil +} + +// createStripAuthMiddleware is the factory function for the auth header stripping middleware. +func createStripAuthMiddleware(_ *types.MiddlewareConfig, runner types.MiddlewareRunner) error { + mw := &stripAuthMiddleware{} + runner.AddMiddleware(stripAuthMiddlewareType, mw) + return nil +} + +// stripAuthMiddleware removes the Authorization header from requests. +type stripAuthMiddleware struct{} + +// Handler returns the middleware function. +func (*stripAuthMiddleware) Handler() types.MiddlewareFunction { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.Header.Del("Authorization") + next.ServeHTTP(w, r) + }) + } +} + +// Close cleans up resources. +func (*stripAuthMiddleware) Close() error { return nil } + // addAWSStsMiddleware adds AWS STS middleware if configured. // Returns an error if AWSStsConfig is set but RemoteURL is empty, because // SigV4 signing is only meaningful for remote MCP servers. diff --git a/pkg/runner/middleware_test.go b/pkg/runner/middleware_test.go index b8aa47ab9b..f2aced92ea 100644 --- a/pkg/runner/middleware_test.go +++ b/pkg/runner/middleware_test.go @@ -271,6 +271,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { name string config *RunConfig wantAppended bool + wantType string // expected middleware type when appended }{ { name: "nil EmbeddedAuthServerConfig returns input unchanged", @@ -284,15 +285,17 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { UpstreamSwapConfig: nil, }, wantAppended: true, + wantType: upstreamswap.MiddlewareType, }, { - name: "EmbeddedAuthServerConfig with DisableUpstreamTokenInjection skips middleware", + name: "DisableUpstreamTokenInjection adds strip-auth middleware instead", config: func() *RunConfig { cfg := createMinimalAuthServerConfig() cfg.DisableUpstreamTokenInjection = true return &RunConfig{EmbeddedAuthServerConfig: cfg} }(), - wantAppended: false, + wantAppended: true, + wantType: stripAuthMiddlewareType, }, { name: "EmbeddedAuthServerConfig set with explicit UpstreamSwapConfig uses provided config", @@ -303,6 +306,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { }, }, wantAppended: true, + wantType: upstreamswap.MiddlewareType, }, { name: "EmbeddedAuthServerConfig with custom header strategy config", @@ -314,6 +318,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { }, }, wantAppended: true, + wantType: upstreamswap.MiddlewareType, }, } @@ -333,20 +338,20 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { // Should have one additional entry. require.Len(t, got, len(initial)+1) added := got[len(got)-1] - assert.Equal(t, upstreamswap.MiddlewareType, added.Type) - - // Verify serialized params contain the expected config. - var params upstreamswap.MiddlewareParams - require.NoError(t, json.Unmarshal(added.Parameters, ¶ms)) + assert.Equal(t, tt.wantType, added.Type) - if tt.config.UpstreamSwapConfig != nil { - // Should use the provided config - require.NotNil(t, params.Config) - assert.Equal(t, tt.config.UpstreamSwapConfig.HeaderStrategy, params.Config.HeaderStrategy) - assert.Equal(t, tt.config.UpstreamSwapConfig.CustomHeaderName, params.Config.CustomHeaderName) - } else { - // Should use defaults (empty config is valid) - require.NotNil(t, params.Config) + // For upstreamswap type, verify serialized params + if tt.wantType == upstreamswap.MiddlewareType { + var params upstreamswap.MiddlewareParams + require.NoError(t, json.Unmarshal(added.Parameters, ¶ms)) + + if tt.config.UpstreamSwapConfig != nil { + require.NotNil(t, params.Config) + assert.Equal(t, tt.config.UpstreamSwapConfig.HeaderStrategy, params.Config.HeaderStrategy) + assert.Equal(t, tt.config.UpstreamSwapConfig.CustomHeaderName, params.Config.CustomHeaderName) + } else { + require.NotNil(t, params.Config) + } } }) } @@ -359,6 +364,7 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) { name string config *RunConfig wantUpstreamSwap bool + wantStripAuth bool wantHeaderStrategy string }{ { @@ -372,13 +378,14 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) { wantUpstreamSwap: false, }, { - name: "DisableUpstreamTokenInjection omits upstream-swap", + name: "DisableUpstreamTokenInjection adds strip-auth instead of upstream-swap", config: func() *RunConfig { cfg := createMinimalAuthServerConfig() cfg.DisableUpstreamTokenInjection = true return &RunConfig{EmbeddedAuthServerConfig: cfg} }(), wantUpstreamSwap: false, + wantStripAuth: true, }, { name: "explicit UpstreamSwapConfig is used", @@ -400,20 +407,25 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) { err := PopulateMiddlewareConfigs(tt.config) require.NoError(t, err) - var found bool + var foundSwap bool + var foundStrip bool var foundConfig *types.MiddlewareConfig for i, mw := range tt.config.MiddlewareConfigs { if mw.Type == upstreamswap.MiddlewareType { - found = true + foundSwap = true foundConfig = &tt.config.MiddlewareConfigs[i] - break + } + if mw.Type == stripAuthMiddlewareType { + foundStrip = true } } - assert.Equal(t, tt.wantUpstreamSwap, found, + assert.Equal(t, tt.wantUpstreamSwap, foundSwap, "upstream-swap middleware presence mismatch") + assert.Equal(t, tt.wantStripAuth, foundStrip, + "strip-auth middleware presence mismatch") // Verify config values if we expect the middleware and have specific expectations - if found && tt.wantHeaderStrategy != "" { + if foundSwap && tt.wantHeaderStrategy != "" { var params upstreamswap.MiddlewareParams require.NoError(t, json.Unmarshal(foundConfig.Parameters, ¶ms)) require.NotNil(t, params.Config) diff --git a/pkg/transport/proxy/transparent/transparent_proxy.go b/pkg/transport/proxy/transparent/transparent_proxy.go index f5d9599b9a..c1b07010fe 100644 --- a/pkg/transport/proxy/transparent/transparent_proxy.go +++ b/pkg/transport/proxy/transparent/transparent_proxy.go @@ -522,6 +522,14 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error) req.Host = req.URL.Host } + slog.Debug("outbound request to upstream", + "method", req.Method, + "url", req.URL.String(), + "host", req.Host, + "accept", req.Header.Get("Accept"), + "content_type", req.Header.Get("Content-Type"), + ) + reqBody := readRequestBody(req) // thv proxy does not provide the transport type, so we need to detect it from the request @@ -611,6 +619,13 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error) return nil, err } + slog.Debug("upstream response received", + "status", resp.StatusCode, + "url", req.URL.String(), + "content_type", resp.Header.Get("Content-Type"), + "mcp_session_id", resp.Header.Get("Mcp-Session-Id"), + ) + // Check for 401 Unauthorized response (bearer token authentication failure) if resp.StatusCode == http.StatusUnauthorized { //nolint:gosec // G706: logging target URI from config @@ -1006,7 +1021,15 @@ func (p *TransparentProxy) Start(ctx context.Context) error { FlushInterval: -1, Rewrite: func(pr *httputil.ProxyRequest) { pr.SetURL(targetURL) - pr.SetXForwarded() + + // Only set X-Forwarded-* headers for local backends. + // For remote upstreams, these headers leak the proxy's hostname + // (X-Forwarded-Host) to third-party servers, which can cause + // 307 redirect loops when the upstream uses that header to + // construct redirect URLs pointing back to the proxy. + if !p.isRemote { + pr.SetXForwarded() + } // Route to the originating backend pod when session metadata contains backend_url. // Falls back to static targetURL when the session doesn't exist or has no backend_url. From 9ee3140f0dec0a7f6eb2be438097bbcf96187076 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Mon, 30 Mar 2026 17:32:00 +0100 Subject: [PATCH 03/11] chore: update crds --- ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 10 ++++++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 20 +++++++++++++++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 10 ++++++++++ 3 files changed, 40 insertions(+) diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 83517ad5a3..491f26eef4 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -214,6 +214,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index b00683e8d3..50ce0a95e3 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -87,6 +87,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing @@ -2723,6 +2733,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index bc484f596b..9c3247ce58 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -90,6 +90,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing From b48b96c803f9339301b676ecf4c7aaea043fb6aa Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Tue, 14 Apr 2026 13:34:44 +0100 Subject: [PATCH 04/11] fix: docs --- docs/server/docs.go | 4 ++++ docs/server/swagger.json | 4 ++++ docs/server/swagger.yaml | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/docs/server/docs.go b/docs/server/docs.go index 1cd3a245da..3f328b38a3 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -558,6 +558,10 @@ const docTemplate = `{ "description": "AuthorizationEndpointBaseURL overrides the base URL used for the authorization_endpoint\nin the OAuth discovery document. When set, the discovery document will advertise\n` + "`" + `{authorization_endpoint_base_url}/oauth/authorize` + "`" + ` instead of ` + "`" + `{issuer}/oauth/authorize` + "`" + `.\nAll other endpoints remain derived from the issuer.", "type": "string" }, + "disable_upstream_token_injection": { + "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients but does not\ninject upstream IdP tokens into requests forwarded to the backend MCP server.", + "type": "boolean" + }, "hmac_secret_files": { "description": "HMACSecretFiles contains file paths to HMAC secrets for signing authorization codes\nand refresh tokens (opaque tokens).\nFirst file is the current secret (must be at least 32 bytes), subsequent files\nare for rotation/verification of existing tokens.\nIf empty, an ephemeral secret will be auto-generated (development only).", "items": { diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 2fa20abe2d..8e37238e98 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -551,6 +551,10 @@ "description": "AuthorizationEndpointBaseURL overrides the base URL used for the authorization_endpoint\nin the OAuth discovery document. When set, the discovery document will advertise\n`{authorization_endpoint_base_url}/oauth/authorize` instead of `{issuer}/oauth/authorize`.\nAll other endpoints remain derived from the issuer.", "type": "string" }, + "disable_upstream_token_injection": { + "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients but does not\ninject upstream IdP tokens into requests forwarded to the backend MCP server.", + "type": "boolean" + }, "hmac_secret_files": { "description": "HMACSecretFiles contains file paths to HMAC secrets for signing authorization codes\nand refresh tokens (opaque tokens).\nFirst file is the current secret (must be at least 32 bytes), subsequent files\nare for rotation/verification of existing tokens.\nIf empty, an ephemeral secret will be auto-generated (development only).", "items": { diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index 46fa4b74a4..087db49b12 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -604,6 +604,12 @@ components: `{authorization_endpoint_base_url}/oauth/authorize` instead of `{issuer}/oauth/authorize`. All other endpoints remain derived from the issuer. type: string + disable_upstream_token_injection: + description: |- + DisableUpstreamTokenInjection prevents the upstream swap middleware from being added. + When true, the embedded auth server handles OAuth flows for clients but does not + inject upstream IdP tokens into requests forwarded to the backend MCP server. + type: boolean hmac_secret_files: description: |- HMACSecretFiles contains file paths to HMAC secrets for signing authorization codes From 49533c1534f7d393caede858aab8e6cc379fa1ef Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Sat, 9 May 2026 02:24:47 -0400 Subject: [PATCH 05/11] chore: manifests --- .../toolhive.stacklok.dev_mcpexternalauthconfigs.yaml | 10 ++++++++++ .../toolhive.stacklok.dev_virtualmcpservers.yaml | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 60de7b1760..6a973c0ea1 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -217,6 +217,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 9c3247ce58..3a25ccec97 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2736,6 +2736,16 @@ spec: Must be a valid HTTPS URL (or HTTP for localhost) without query, fragment, or trailing slash. pattern: ^https?://[^\s?#]+[^/\s?#]$ type: string + disableUpstreamTokenInjection: + default: false + description: |- + DisableUpstreamTokenInjection prevents the embedded auth server from injecting + upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server still handles OAuth flows for clients + but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + This is useful when the backend MCP server does not require authentication + (e.g., public documentation servers) but you still want client authentication. + type: boolean hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing From 7338658b1c7f968ece0cfd342aa77a63d42b50b0 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 13:45:33 +0100 Subject: [PATCH 06/11] Cover strip-auth middleware factory and handler Codecov flagged 31.6% patch coverage on pkg/runner/middleware.go: the existing tests exercised only the middleware selection logic in addUpstreamSwapMiddleware. Add a direct test that the factory registers under the strip-auth type, the handler removes the Authorization header while passing other headers through, and Close is a no-op. Co-Authored-By: Claude Fable 5 --- pkg/runner/middleware_test.go | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/pkg/runner/middleware_test.go b/pkg/runner/middleware_test.go index 2c0d5b8487..e0816f2301 100644 --- a/pkg/runner/middleware_test.go +++ b/pkg/runner/middleware_test.go @@ -5,6 +5,8 @@ package runner import ( "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -12,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" @@ -30,6 +33,7 @@ import ( "github.com/stacklok/toolhive/pkg/telemetry" headerfwd "github.com/stacklok/toolhive/pkg/transport/middleware" "github.com/stacklok/toolhive/pkg/transport/types" + "github.com/stacklok/toolhive/pkg/transport/types/mocks" "github.com/stacklok/toolhive/pkg/webhook" "github.com/stacklok/toolhive/pkg/webhook/mutating" "github.com/stacklok/toolhive/pkg/webhook/validating" @@ -981,3 +985,45 @@ func TestPopulateMiddlewareConfigs_FullCoverage(t *testing.T) { assert.True(t, typeIndex[authz.MiddlewareType]) assert.True(t, typeIndex[audit.MiddlewareType]) } + +// TestStripAuthMiddleware covers the strip-auth middleware end to end: the +// factory registers it on the runner under stripAuthMiddlewareType, the +// handler removes the Authorization header (and nothing else) before calling +// next, and Close is a no-op. +func TestStripAuthMiddleware(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + var registered types.Middleware + mockRunner := mocks.NewMockMiddlewareRunner(ctrl) + mockRunner.EXPECT().AddMiddleware(stripAuthMiddlewareType, gomock.Any()).Do(func(_ string, mw types.Middleware) { + registered = mw + }) + + mwConfig, err := types.NewMiddlewareConfig(stripAuthMiddlewareType, struct{}{}) + require.NoError(t, err) + require.NoError(t, createStripAuthMiddleware(mwConfig, mockRunner)) + require.NotNil(t, registered) + + var gotAuth, gotCustom string + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + gotCustom = r.Header.Get("X-Custom") + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Authorization", "Bearer toolhive-jwt") + req.Header.Set("X-Custom", "kept") + rec := httptest.NewRecorder() + + registered.Handler()(next).ServeHTTP(rec, req) + + assert.Empty(t, gotAuth, "Authorization header must be stripped before reaching the upstream") + assert.Equal(t, "kept", gotCustom, "unrelated headers must pass through") + assert.Equal(t, http.StatusNoContent, rec.Code) + + assert.NoError(t, registered.Close()) +} From dfac8fdb1eca9ce6e0e798b183a8d086f6bad1c0 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 14:32:32 +0100 Subject: [PATCH 07/11] Retrigger CI after flaky e2e failures api-workloads hit the known delete-workload Eventually timeout and llm's BeforeEach mock OIDC server never came up (connection refused). Neither path is touched by this change. Co-Authored-By: Claude Fable 5 From a0478f26e121ab43b49ccdda7718429162eb8398 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 14:43:49 +0100 Subject: [PATCH 08/11] Retrigger CI: unit-test job hit upstream race (#5502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests / Test Go Code failed on the data race in TestMCPAuthzConfigReconciler_watchHandlers introduced on main by PR #4777 — tracked upstream as #5502. Nothing in this branch is involved. Co-Authored-By: Claude Fable 5 From 9071bc80c99ac6933a0da41be78c136970202e12 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 15:19:43 +0100 Subject: [PATCH 09/11] Extract and harden strip-auth middleware Address review feedback on #4168 (F1, F3, F5/F6): - Move the middleware out of the pkg/runner wiring layer into pkg/transport/middleware/strip_auth.go, matching the one-file-per- middleware convention (header_forward.go precedent), with exported StripAuthMiddlewareName and CreateStripAuthMiddleware. - Strip Cookie and Proxy-Authorization alongside Authorization so no client credential carrier reaches a public upstream; the set mirrors what net/http refuses to copy across cross-host redirects. - Reject DisableUpstreamTokenInjection combined with token exchange or AWS STS at config-population time: both run closer to the backend and would re-add credentials after the strip, silently defeating the flag. - Pin the validate-before-strip ordering invariant with TestPopulateMiddlewareConfigs_StripAuthOrdering. Co-Authored-By: Claude Fable 5 --- pkg/runner/middleware.go | 60 ++++++------- pkg/runner/middleware_test.go | 99 +++++++++++++-------- pkg/transport/middleware/strip_auth.go | 52 +++++++++++ pkg/transport/middleware/strip_auth_test.go | 64 +++++++++++++ 4 files changed, 203 insertions(+), 72 deletions(-) create mode 100644 pkg/transport/middleware/strip_auth.go create mode 100644 pkg/transport/middleware/strip_auth_test.go diff --git a/pkg/runner/middleware.go b/pkg/runner/middleware.go index dd71f2fc2c..82397016a4 100644 --- a/pkg/runner/middleware.go +++ b/pkg/runner/middleware.go @@ -5,7 +5,6 @@ package runner import ( "fmt" - "net/http" "github.com/stacklok/toolhive/pkg/audit" "github.com/stacklok/toolhive/pkg/auth" @@ -46,9 +45,9 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory { audit.MiddlewareType: audit.CreateMiddleware, recovery.MiddlewareType: recovery.CreateMiddleware, headerfwd.HeaderForwardMiddlewareName: headerfwd.CreateMiddleware, + headerfwd.StripAuthMiddlewareName: headerfwd.CreateStripAuthMiddleware, validating.MiddlewareType: validating.CreateMiddleware, mutating.MiddlewareType: mutating.CreateMiddleware, - stripAuthMiddlewareType: createStripAuthMiddleware, } } @@ -346,9 +345,26 @@ func addUpstreamSwapMiddleware( return middlewares, nil } - // When upstream token injection is disabled, strip the Authorization header - // so the client's ToolHive JWT doesn't leak to the upstream server. + // When upstream token injection is disabled, strip the client's credential + // headers (Authorization, Cookie, Proxy-Authorization) so they never reach + // the upstream server. Two ordering invariants apply, pinned by + // TestPopulateMiddlewareConfigs_StripAuthOrdering: + // - strip-auth is appended after the auth middleware, so the client JWT + // is fully validated (and the identity stored in the request context + // for authz/audit) before the header is removed; + // - token-injecting middlewares (token exchange, AWS STS) run closer to + // the backend and would re-add an Authorization header after the + // strip, silently defeating the flag — that contradiction is rejected + // here instead. if config.EmbeddedAuthServerConfig.DisableUpstreamTokenInjection { + if config.TokenExchangeConfig != nil { + return nil, fmt.Errorf("disableUpstreamTokenInjection cannot be combined with token exchange: " + + "token exchange would re-add an Authorization header after strip-auth removes it") + } + if config.AWSStsConfig != nil { + return nil, fmt.Errorf("disableUpstreamTokenInjection cannot be combined with AWS STS: " + + "SigV4 signing would re-add credentials after strip-auth removes them") + } return addAuthHeaderStripMiddleware(middlewares) } @@ -407,45 +423,21 @@ func injectUpstreamProviderIfNeeded( return cedar.InjectUpstreamProvider(authzCfg, providerName) } -// stripAuthMiddlewareType is the type identifier for the auth header stripping middleware. -const stripAuthMiddlewareType = "strip-auth" - -// addAuthHeaderStripMiddleware adds a middleware that removes the Authorization header -// before forwarding to the upstream. This prevents the client's ToolHive JWT from -// leaking to upstream servers that don't expect it. +// addAuthHeaderStripMiddleware adds the strip-auth middleware +// (pkg/transport/middleware), which removes the client's credential headers +// before forwarding to the upstream. This prevents the client's ToolHive JWT, +// cookies, and proxy credentials from leaking to upstream servers that don't +// expect them. func addAuthHeaderStripMiddleware( middlewares []types.MiddlewareConfig, ) ([]types.MiddlewareConfig, error) { - mwConfig, err := types.NewMiddlewareConfig(stripAuthMiddlewareType, struct{}{}) + mwConfig, err := types.NewMiddlewareConfig(headerfwd.StripAuthMiddlewareName, struct{}{}) if err != nil { return nil, fmt.Errorf("failed to create strip-auth middleware config: %w", err) } return append(middlewares, *mwConfig), nil } -// createStripAuthMiddleware is the factory function for the auth header stripping middleware. -func createStripAuthMiddleware(_ *types.MiddlewareConfig, runner types.MiddlewareRunner) error { - mw := &stripAuthMiddleware{} - runner.AddMiddleware(stripAuthMiddlewareType, mw) - return nil -} - -// stripAuthMiddleware removes the Authorization header from requests. -type stripAuthMiddleware struct{} - -// Handler returns the middleware function. -func (*stripAuthMiddleware) Handler() types.MiddlewareFunction { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - r.Header.Del("Authorization") - next.ServeHTTP(w, r) - }) - } -} - -// Close cleans up resources. -func (*stripAuthMiddleware) Close() error { return nil } - // addAWSStsMiddleware adds AWS STS middleware if configured. // Returns an error if AWSStsConfig is set but RemoteURL is empty, because // SigV4 signing is only meaningful for remote MCP servers. diff --git a/pkg/runner/middleware_test.go b/pkg/runner/middleware_test.go index e0816f2301..fc85b7bec7 100644 --- a/pkg/runner/middleware_test.go +++ b/pkg/runner/middleware_test.go @@ -5,8 +5,6 @@ package runner import ( "encoding/json" - "net/http" - "net/http/httptest" "os" "path/filepath" "testing" @@ -14,7 +12,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" @@ -28,12 +25,12 @@ import ( "github.com/stacklok/toolhive/pkg/authz/authorizers" "github.com/stacklok/toolhive/pkg/authz/authorizers/cedar" "github.com/stacklok/toolhive/pkg/mcp" + "github.com/stacklok/toolhive/pkg/oauthproto/tokenexchange" "github.com/stacklok/toolhive/pkg/ratelimit" "github.com/stacklok/toolhive/pkg/recovery" "github.com/stacklok/toolhive/pkg/telemetry" headerfwd "github.com/stacklok/toolhive/pkg/transport/middleware" "github.com/stacklok/toolhive/pkg/transport/types" - "github.com/stacklok/toolhive/pkg/transport/types/mocks" "github.com/stacklok/toolhive/pkg/webhook" "github.com/stacklok/toolhive/pkg/webhook/mutating" "github.com/stacklok/toolhive/pkg/webhook/validating" @@ -336,7 +333,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) { return &RunConfig{EmbeddedAuthServerConfig: cfg} }(), wantAppended: true, - wantType: stripAuthMiddlewareType, + wantType: headerfwd.StripAuthMiddlewareName, }, { name: "EmbeddedAuthServerConfig set with explicit UpstreamSwapConfig uses provided config", @@ -456,7 +453,7 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) { foundSwap = true foundConfig = &tt.config.MiddlewareConfigs[i] } - if mw.Type == stripAuthMiddlewareType { + if mw.Type == headerfwd.StripAuthMiddlewareName { foundStrip = true } } @@ -986,44 +983,70 @@ func TestPopulateMiddlewareConfigs_FullCoverage(t *testing.T) { assert.True(t, typeIndex[audit.MiddlewareType]) } -// TestStripAuthMiddleware covers the strip-auth middleware end to end: the -// factory registers it on the runner under stripAuthMiddlewareType, the -// handler removes the Authorization header (and nothing else) before calling -// next, and Close is a no-op. -func TestStripAuthMiddleware(t *testing.T) { +// TestPopulateMiddlewareConfigs_StripAuthOrdering pins the ordering invariant +// for strip-auth: the auth middleware must precede it in the chain so the +// client JWT is fully validated (and the identity stored in the request +// context for authz/audit) before the Authorization header is removed. +func TestPopulateMiddlewareConfigs_StripAuthOrdering(t *testing.T) { t.Parallel() - ctrl := gomock.NewController(t) - defer ctrl.Finish() + authServerCfg := createMinimalAuthServerConfig() + authServerCfg.DisableUpstreamTokenInjection = true + config := &RunConfig{EmbeddedAuthServerConfig: authServerCfg} - var registered types.Middleware - mockRunner := mocks.NewMockMiddlewareRunner(ctrl) - mockRunner.EXPECT().AddMiddleware(stripAuthMiddlewareType, gomock.Any()).Do(func(_ string, mw types.Middleware) { - registered = mw - }) + require.NoError(t, PopulateMiddlewareConfigs(config)) - mwConfig, err := types.NewMiddlewareConfig(stripAuthMiddlewareType, struct{}{}) - require.NoError(t, err) - require.NoError(t, createStripAuthMiddleware(mwConfig, mockRunner)) - require.NotNil(t, registered) - - var gotAuth, gotCustom string - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotAuth = r.Header.Get("Authorization") - gotCustom = r.Header.Get("X-Custom") - w.WriteHeader(http.StatusNoContent) - }) + authIdx, stripIdx := -1, -1 + for i, mw := range config.MiddlewareConfigs { + switch mw.Type { + case auth.MiddlewareType: + authIdx = i + case headerfwd.StripAuthMiddlewareName: + stripIdx = i + } + } + require.GreaterOrEqual(t, authIdx, 0, "auth middleware must be present") + require.GreaterOrEqual(t, stripIdx, 0, "strip-auth middleware must be present") + assert.Less(t, authIdx, stripIdx, + "auth must validate the client JWT before strip-auth removes the Authorization header") +} - req := httptest.NewRequest(http.MethodPost, "/mcp", nil) - req.Header.Set("Authorization", "Bearer toolhive-jwt") - req.Header.Set("X-Custom", "kept") - rec := httptest.NewRecorder() +// TestPopulateMiddlewareConfigs_StripAuthConflicts verifies that +// DisableUpstreamTokenInjection is rejected when combined with middlewares +// that would re-add credentials after the strip (token exchange, AWS STS), +// instead of silently defeating the flag at runtime. +func TestPopulateMiddlewareConfigs_StripAuthConflicts(t *testing.T) { + t.Parallel() - registered.Handler()(next).ServeHTTP(rec, req) + tests := []struct { + name string + mutate func(*RunConfig) + wantErr string + }{ + { + name: "token exchange combination is rejected", + mutate: func(c *RunConfig) { c.TokenExchangeConfig = &tokenexchange.Config{} }, + wantErr: "token exchange", + }, + { + name: "AWS STS combination is rejected", + mutate: func(c *RunConfig) { c.AWSStsConfig = &awssts.Config{}; c.RemoteURL = "https://example.com" }, + wantErr: "AWS STS", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - assert.Empty(t, gotAuth, "Authorization header must be stripped before reaching the upstream") - assert.Equal(t, "kept", gotCustom, "unrelated headers must pass through") - assert.Equal(t, http.StatusNoContent, rec.Code) + authServerCfg := createMinimalAuthServerConfig() + authServerCfg.DisableUpstreamTokenInjection = true + config := &RunConfig{EmbeddedAuthServerConfig: authServerCfg} + tt.mutate(config) - assert.NoError(t, registered.Close()) + err := PopulateMiddlewareConfigs(config) + require.ErrorContains(t, err, "disableUpstreamTokenInjection cannot be combined") + require.ErrorContains(t, err, tt.wantErr) + }) + } } diff --git a/pkg/transport/middleware/strip_auth.go b/pkg/transport/middleware/strip_auth.go new file mode 100644 index 0000000000..ee7ec866f6 --- /dev/null +++ b/pkg/transport/middleware/strip_auth.go @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package middleware + +import ( + "net/http" + + "github.com/stacklok/toolhive/pkg/transport/types" +) + +// StripAuthMiddlewareName is the type constant for the credential stripping middleware. +const StripAuthMiddlewareName = "strip-auth" + +// clientCredentialHeaders are the request headers removed by the strip-auth +// middleware before a request is forwarded to the backend. Authorization +// carries the ToolHive JWT; Cookie and Proxy-Authorization are the other +// headers a client can use to carry credentials. The set mirrors the headers +// net/http refuses to copy across cross-host redirects. +var clientCredentialHeaders = []string{"Authorization", "Cookie", "Proxy-Authorization"} + +// StripAuthMiddleware removes client credential headers from requests so they +// never reach the backend. It is used when clients are authenticated by the +// proxy but the backend itself is public (DisableUpstreamTokenInjection): by +// the time this middleware runs, the auth middleware has already validated +// the client JWT and stored the identity in the request context, so the +// backend receives an unauthenticated request. Credentials injected by +// middlewares that run closer to the backend (e.g. header-forward) are +// unaffected. +type StripAuthMiddleware struct{} + +// Handler returns the middleware function. +func (*StripAuthMiddleware) Handler() types.MiddlewareFunction { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + for _, h := range clientCredentialHeaders { + r.Header.Del(h) + } + next.ServeHTTP(w, r) + }) + } +} + +// Close cleans up resources. The middleware holds none. +func (*StripAuthMiddleware) Close() error { return nil } + +// CreateStripAuthMiddleware is the factory function for the strip-auth +// middleware. It takes no parameters. +func CreateStripAuthMiddleware(config *types.MiddlewareConfig, runner types.MiddlewareRunner) error { + runner.AddMiddleware(config.Type, &StripAuthMiddleware{}) + return nil +} diff --git a/pkg/transport/middleware/strip_auth_test.go b/pkg/transport/middleware/strip_auth_test.go new file mode 100644 index 0000000000..634a856cf7 --- /dev/null +++ b/pkg/transport/middleware/strip_auth_test.go @@ -0,0 +1,64 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive/pkg/transport/types" + typesmocks "github.com/stacklok/toolhive/pkg/transport/types/mocks" +) + +// TestStripAuthMiddleware covers the strip-auth middleware end to end: the +// factory registers it on the runner under the config type, the handler +// removes every client credential header (Authorization, Cookie, +// Proxy-Authorization) while passing unrelated headers through, and Close is +// a no-op. +func TestStripAuthMiddleware(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + var registered types.Middleware + mockRunner := typesmocks.NewMockMiddlewareRunner(ctrl) + mockRunner.EXPECT().AddMiddleware(StripAuthMiddlewareName, gomock.Any()).Do(func(_ string, mw types.Middleware) { + registered = mw + }) + + mwConfig, err := types.NewMiddlewareConfig(StripAuthMiddlewareName, struct{}{}) + require.NoError(t, err) + require.NoError(t, CreateStripAuthMiddleware(mwConfig, mockRunner)) + require.NotNil(t, registered) + + var captured *http.Request + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured = r + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Authorization", "Bearer toolhive-jwt") + req.Header.Set("Cookie", "session=abc") + req.Header.Set("Proxy-Authorization", "Basic Zm9vOmJhcg==") + req.Header.Set("X-Custom", "kept") + rec := httptest.NewRecorder() + + registered.Handler()(next).ServeHTTP(rec, req) + + require.NotNil(t, captured) + for _, h := range clientCredentialHeaders { + assert.Empty(t, captured.Header.Get(h), "%s must be stripped before reaching the backend", h) + } + assert.Equal(t, "kept", captured.Header.Get("X-Custom"), "unrelated headers must pass through") + assert.Equal(t, http.StatusNoContent, rec.Code) + + assert.NoError(t, registered.Close()) +} From 84464ec98517b2159509ea810d8138b358df3e36 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 15:19:55 +0100 Subject: [PATCH 10/11] Drop only X-Forwarded-Host for remote upstreams Address review feedback on #4168 (F2): skipping SetXForwarded() wholesale also removed X-Forwarded-For/-Proto, which remote backends legitimately use for client IP logging and rate limiting. Only X-Forwarded-Host is implicated in the 307 redirect loop. Always call SetXForwarded and delete just X-Forwarded-Host when the upstream is remote, via a setXForwardedHeaders helper that is now unit-tested for both the local and remote paths. Co-Authored-By: Claude Fable 5 --- .../proxy/transparent/transparent_proxy.go | 24 +++++--- .../proxy/transparent/xforwarded_test.go | 58 +++++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 pkg/transport/proxy/transparent/xforwarded_test.go diff --git a/pkg/transport/proxy/transparent/transparent_proxy.go b/pkg/transport/proxy/transparent/transparent_proxy.go index dc58ab750b..bad42d93dc 100644 --- a/pkg/transport/proxy/transparent/transparent_proxy.go +++ b/pkg/transport/proxy/transparent/transparent_proxy.go @@ -1019,6 +1019,20 @@ func (p *TransparentProxy) modifyResponse(resp *http.Response) error { return p.responseProcessor.ProcessResponse(resp) } +// setXForwardedHeaders populates the standard X-Forwarded-* headers on the +// outbound request. For remote upstreams X-Forwarded-Host is removed: a +// third-party server may use it to construct redirect URLs pointing back at +// the proxy, producing 307 redirect loops. X-Forwarded-For and +// X-Forwarded-Proto are kept so remote backends can still log the client IP +// and scheme. Client-supplied X-Forwarded-* values never pass through either +// way — httputil strips them from the outbound request before Rewrite runs. +func (p *TransparentProxy) setXForwardedHeaders(pr *httputil.ProxyRequest) { + pr.SetXForwarded() + if p.isRemote { + pr.Out.Header.Del("X-Forwarded-Host") + } +} + // Start starts the transparent proxy. // nolint:gocyclo // This function handles multiple startup scenarios and is complex by design func (p *TransparentProxy) Start(ctx context.Context) error { @@ -1041,15 +1055,7 @@ func (p *TransparentProxy) Start(ctx context.Context) error { FlushInterval: -1, Rewrite: func(pr *httputil.ProxyRequest) { pr.SetURL(targetURL) - - // Only set X-Forwarded-* headers for local backends. - // For remote upstreams, these headers leak the proxy's hostname - // (X-Forwarded-Host) to third-party servers, which can cause - // 307 redirect loops when the upstream uses that header to - // construct redirect URLs pointing back to the proxy. - if !p.isRemote { - pr.SetXForwarded() - } + p.setXForwardedHeaders(pr) // Route to the originating backend pod when session metadata contains backend_url. // Falls back to static targetURL when the session doesn't exist or has no backend_url. diff --git a/pkg/transport/proxy/transparent/xforwarded_test.go b/pkg/transport/proxy/transparent/xforwarded_test.go new file mode 100644 index 0000000000..358b23192f --- /dev/null +++ b/pkg/transport/proxy/transparent/xforwarded_test.go @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package transparent + +import ( + "net/http" + "net/http/httptest" + "net/http/httputil" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestSetXForwardedHeaders verifies the remote/local split for X-Forwarded-* +// headers: a remote upstream must not receive X-Forwarded-Host (third-party +// servers can echo it into redirect URLs, creating 307 loops back to the +// proxy), while X-Forwarded-For and X-Forwarded-Proto are always set so +// backends can still see the client IP and scheme. Local backends receive +// the full set. +func TestSetXForwardedHeaders(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + isRemote bool + wantHost bool + }{ + {name: "local backend keeps X-Forwarded-Host", isRemote: false, wantHost: true}, + {name: "remote upstream omits X-Forwarded-Host", isRemote: true, wantHost: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + p := NewTransparentProxy( + "127.0.0.1", 0, "", nil, nil, nil, false, + tt.isRemote, "streamable-http", nil, nil, "", false, + ) + + in := httptest.NewRequest(http.MethodPost, "http://proxy.example.com/mcp", nil) + pr := &httputil.ProxyRequest{In: in, Out: in.Clone(in.Context())} + + p.setXForwardedHeaders(pr) + + if tt.wantHost { + assert.Equal(t, "proxy.example.com", pr.Out.Header.Get("X-Forwarded-Host")) + } else { + assert.Empty(t, pr.Out.Header.Get("X-Forwarded-Host"), + "remote upstreams must not see the proxy hostname") + } + assert.NotEmpty(t, pr.Out.Header.Get("X-Forwarded-For"), + "client IP must be forwarded for both local and remote backends") + assert.Equal(t, "http", pr.Out.Header.Get("X-Forwarded-Proto")) + }) + } +} From c641a503709e66298b4b3db57563a99a135e58d2 Mon Sep 17 00:00:00 2001 From: Aron Gates Date: Fri, 12 Jun 2026 15:19:56 +0100 Subject: [PATCH 11/11] Document credential stripping in field docs Address review feedback on #4168 (F4): the CRD and authserver RunConfig comments described only "does not swap ToolHive JWTs for upstream tokens", omitting the load-bearing behavior that the client's credential headers are removed and the backend receives an unauthenticated request. Spell out the stripped header set, point at headerForward for static credentials, and note the token exchange / AWS STS incompatibility. CRD schemas, crd-api.md, and swagger regenerated. Co-Authored-By: Claude Fable 5 --- .../api/v1beta1/mcpexternalauthconfig_types.go | 9 +++++++-- ...ve.stacklok.dev_mcpexternalauthconfigs.yaml | 18 ++++++++++++++---- ...oolhive.stacklok.dev_virtualmcpservers.yaml | 18 ++++++++++++++---- ...ve.stacklok.dev_mcpexternalauthconfigs.yaml | 18 ++++++++++++++---- ...oolhive.stacklok.dev_virtualmcpservers.yaml | 18 ++++++++++++++---- docs/operator/crd-api.md | 2 +- docs/server/docs.go | 2 +- docs/server/swagger.json | 2 +- docs/server/swagger.yaml | 7 +++++-- pkg/authserver/config.go | 7 +++++-- 10 files changed, 76 insertions(+), 25 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index b9f828737b..77a4aadc51 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -281,8 +281,13 @@ type EmbeddedAuthServerConfig struct { // DisableUpstreamTokenInjection prevents the embedded auth server from injecting // upstream IdP tokens into requests forwarded to the backend MCP server. - // When true, the embedded auth server still handles OAuth flows for clients - // but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + // When true, the embedded auth server still handles OAuth flows for clients, + // but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + // the client's credential headers (Authorization, Cookie, Proxy-Authorization) + // after validating the JWT — the backend receives an unauthenticated request. + // Use headerForward to attach static credentials (e.g. an API key) if the + // backend needs them. Cannot be combined with token exchange or AWS STS, + // which would re-add credentials after the strip. // This is useful when the backend MCP server does not require authentication // (e.g., public documentation servers) but you still want client authentication. // +kubebuilder:default=false diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 3be35efa7c..1c005d6a00 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -274,8 +274,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean @@ -1593,8 +1598,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index 7909273fce..178d3b60c6 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -147,8 +147,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean @@ -3137,8 +3142,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 1d896e1eab..db58942062 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -277,8 +277,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean @@ -1596,8 +1601,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index a05919e312..59ab53de26 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -150,8 +150,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean @@ -3140,8 +3145,13 @@ spec: description: |- DisableUpstreamTokenInjection prevents the embedded auth server from injecting upstream IdP tokens into requests forwarded to the backend MCP server. - When true, the embedded auth server still handles OAuth flows for clients - but does not swap ToolHive JWTs for upstream tokens on outgoing requests. + When true, the embedded auth server still handles OAuth flows for clients, + but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS + the client's credential headers (Authorization, Cookie, Proxy-Authorization) + after validating the JWT — the backend receives an unauthenticated request. + Use headerForward to attach static credentials (e.g. an API key) if the + backend needs them. Cannot be combined with token exchange or AWS STS, + which would re-add credentials after the strip. This is useful when the backend MCP server does not require authentication (e.g., public documentation servers) but you still want client authentication. type: boolean diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 6b8f2103c2..70ad8496bf 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -1236,7 +1236,7 @@ _Appears in:_ | `upstreamProviders` _[api.v1beta1.UpstreamProviderConfig](#apiv1beta1upstreamproviderconfig) array_ | UpstreamProviders configures connections to upstream Identity Providers.
The embedded auth server delegates authentication to these providers.
MCPServer and MCPRemoteProxy support a single upstream; VirtualMCPServer supports multiple. | | MinItems: 1
Required: \{\}
| | `primaryUpstreamProvider` _string_ | PrimaryUpstreamProvider names the upstream IDP whose access token Cedar
should read claims from when authorising a request. Must match the name
of one of the entries in UpstreamProviders. When empty, the controller
auto-selects the first entry of UpstreamProviders.
Only meaningful on VirtualMCPServer, where multiple upstream providers
can be configured and Cedar needs to pick which token's claims to
evaluate. The VirtualMCPServer controller validates this field against
UpstreamProviders at admission and rejects unresolvable values.
On MCPServer and MCPRemoteProxy this field is structurally present (the
EmbeddedAuthServerConfig struct is shared) but has no runtime effect:
those CRDs are restricted to a single upstream so there is no choice to
make. Setting it on those CRDs is silently ignored. | | MaxLength: 63
MinLength: 1
Pattern: `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`
Optional: \{\}
| | `storage` _[api.v1beta1.AuthServerStorageConfig](#apiv1beta1authserverstorageconfig)_ | Storage configures the storage backend for the embedded auth server.
If not specified, defaults to in-memory storage. | | Optional: \{\}
| -| `disableUpstreamTokenInjection` _boolean_ | DisableUpstreamTokenInjection prevents the embedded auth server from injecting
upstream IdP tokens into requests forwarded to the backend MCP server.
When true, the embedded auth server still handles OAuth flows for clients
but does not swap ToolHive JWTs for upstream tokens on outgoing requests.
This is useful when the backend MCP server does not require authentication
(e.g., public documentation servers) but you still want client authentication. | false | Optional: \{\}
| +| `disableUpstreamTokenInjection` _boolean_ | DisableUpstreamTokenInjection prevents the embedded auth server from injecting
upstream IdP tokens into requests forwarded to the backend MCP server.
When true, the embedded auth server still handles OAuth flows for clients,
but instead of swapping ToolHive JWTs for upstream tokens the proxy STRIPS
the client's credential headers (Authorization, Cookie, Proxy-Authorization)
after validating the JWT — the backend receives an unauthenticated request.
Use headerForward to attach static credentials (e.g. an API key) if the
backend needs them. Cannot be combined with token exchange or AWS STS,
which would re-add credentials after the strip.
This is useful when the backend MCP server does not require authentication
(e.g., public documentation servers) but you still want client authentication. | false | Optional: \{\}
| | `baselineClientScopes` _string array_ | BaselineClientScopes is a baseline set of OAuth 2.0 scopes guaranteed to be
included in every client registration. The embedded auth server unions these
scopes into the registered set returned by RFC 7591 Dynamic Client
Registration, so a client that narrows the `scope` field at /oauth/register
can still request the baseline scopes at /oauth/authorize. All values must
be present in the upstream-derived scopesSupported set; the auth server
fails to start if any value is missing.
Security: every client registered via /oauth/register will gain the
ability to request these scopes at /oauth/authorize, regardless of what
the client itself requested. Keep the baseline narrow (typically
"openid" and "offline_access"). Adding a privileged scope here — e.g.
"admin:read" — would grant it to every DCR-registered client, including
public clients like Claude Code, Cursor, and VS Code.
When cimd.enabled is true, every dynamically resolved CIMD client will
also gain the ability to request these scopes, including third-party
clients resolved from arbitrary HTTPS URLs. | | MaxItems: 10
items:MinLength: 1
items:Pattern: `^[\x21\x23-\x5B\x5D-\x7E]+$`
Optional: \{\}
| | `cimd` _[api.v1beta1.EmbeddedAuthServerCIMDConfig](#apiv1beta1embeddedauthservercimdconfig)_ | CIMD configures Client ID Metadata Document support. When omitted, CIMD is disabled. | | Optional: \{\}
| diff --git a/docs/server/docs.go b/docs/server/docs.go index a2b6e09488..a7dcdadfdb 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -540,7 +540,7 @@ const docTemplate = `{ "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver.CIMDRunConfig" }, "disable_upstream_token_injection": { - "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients but does not\ninject upstream IdP tokens into requests forwarded to the backend MCP server.", + "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients, but instead of\ninjecting upstream IdP tokens the proxy strips the client's credential headers\n(Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the\nbackend receives an unauthenticated request. Incompatible with token exchange\nand AWS STS, which would re-add credentials after the strip.", "type": "boolean" }, "hmac_secret_files": { diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 9bcd675725..224bd6faa3 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -533,7 +533,7 @@ "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver.CIMDRunConfig" }, "disable_upstream_token_injection": { - "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients but does not\ninject upstream IdP tokens into requests forwarded to the backend MCP server.", + "description": "DisableUpstreamTokenInjection prevents the upstream swap middleware from being added.\nWhen true, the embedded auth server handles OAuth flows for clients, but instead of\ninjecting upstream IdP tokens the proxy strips the client's credential headers\n(Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the\nbackend receives an unauthenticated request. Incompatible with token exchange\nand AWS STS, which would re-add credentials after the strip.", "type": "boolean" }, "hmac_secret_files": { diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index db7c96edcf..f95340c432 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -601,8 +601,11 @@ components: disable_upstream_token_injection: description: |- DisableUpstreamTokenInjection prevents the upstream swap middleware from being added. - When true, the embedded auth server handles OAuth flows for clients but does not - inject upstream IdP tokens into requests forwarded to the backend MCP server. + When true, the embedded auth server handles OAuth flows for clients, but instead of + injecting upstream IdP tokens the proxy strips the client's credential headers + (Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the + backend receives an unauthenticated request. Incompatible with token exchange + and AWS STS, which would re-add credentials after the strip. type: boolean hmac_secret_files: description: |- diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 3bae423dc3..a59ee2b931 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -92,8 +92,11 @@ type RunConfig struct { Storage *storage.RunConfig `json:"storage,omitempty" yaml:"storage,omitempty"` // DisableUpstreamTokenInjection prevents the upstream swap middleware from being added. - // When true, the embedded auth server handles OAuth flows for clients but does not - // inject upstream IdP tokens into requests forwarded to the backend MCP server. + // When true, the embedded auth server handles OAuth flows for clients, but instead of + // injecting upstream IdP tokens the proxy strips the client's credential headers + // (Authorization, Cookie, Proxy-Authorization) after the JWT is validated — the + // backend receives an unauthenticated request. Incompatible with token exchange + // and AWS STS, which would re-add credentials after the strip. //nolint:lll // field tags require full JSON+YAML names DisableUpstreamTokenInjection bool `json:"disable_upstream_token_injection,omitempty" yaml:"disable_upstream_token_injection,omitempty"`