-
Notifications
You must be signed in to change notification settings - Fork 192
Description
Description
Wire upstream token enrichment into the auth middleware and simplify the upstreamswap middleware by removing its storage dependency. After this change, the TokenValidator eagerly loads all upstream access tokens into Identity.UpstreamTokens at JWT validation time, so upstreamswap only needs to read identity.UpstreamTokens[cfg.ProviderName] directly — no storage call, no context extraction, no ServiceGetter pattern.
Context
RFC-0052 introduces a sequential authorization chain that accumulates tokens from multiple upstream IDPs per session. The current upstreamswap middleware retrieves the correct upstream token by calling InProcessService.GetValidTokens(ctx, tsid) at request time, which couples the middleware to the UpstreamTokenStorage dependency via a ServiceGetter function. This phase decouples that by moving the storage read into the auth middleware layer — a single GetAllUpstreamTokens bulk read happens once per request during JWT validation and the results are stored in the Identity struct. All downstream middleware (including upstreamswap) then read from the pre-enriched Identity with no additional storage round-trips.
This is Phase 4 of the RFC-0052 implementation. It depends on Phase 1 (#4136), which introduced the GetAllUpstreamTokens bulk-read method on UpstreamTokenStorage. It may be developed in parallel with TASK-002 and TASK-003 after TASK-001 merges.
Dependencies: #4136
Blocks: None (terminal phase)
Acceptance Criteria
-
Identitystruct inpkg/auth/identity.gohas a newUpstreamTokens map[string]stringfield (providerName → access token) with a doc comment stating it is populated by auth middleware after JWT validation, contains expired tokens as-is, and is empty for non-embedded-AS requests -
Identity.MarshalJSON()redactsUpstreamTokensvalues (replaces each token value with"REDACTED") while keeping provider name keys visible; provider names are safe to log but token values are not -
Identity.String()does not leak upstream token values (the existing formatIdentity{Subject:%q}is sufficient; no additional changes required unless token values would otherwise appear) -
TokenValidatorstruct inpkg/auth/token.gohas a new optional fieldupstreamStorage storage.UpstreamTokenStorage(nil = no enrichment; used for non-embedded-AS deployments where no storage exists) - Between
claimsToIdentity(~line 1087) andWithIdentity(~line 1096) inTokenValidator.Middleware, whenv.upstreamStorageis non-nil and the identity has atsidclaim,GetAllUpstreamTokens(r.Context(), tsid)is called and the result is used to populateidentity.UpstreamTokens - When
GetAllUpstreamTokensreturns an error, the middleware logs at WARN level and continues with an empty (or nil)UpstreamTokensmap — the error is non-fatal and the request proceeds - When the
tsidclaim is absent orv.upstreamStorageis nil,identity.UpstreamTokensremains nil/empty and the request proceeds without error -
GetAuthenticationMiddlewareinpkg/auth/utils.goaccepts an optionalstorage.UpstreamTokenStorageparameter and threads it through toNewTokenValidator -
CreateMiddlewareinpkg/auth/middleware.goextracts the storage dependency from the runner and passes it toGetAuthenticationMiddleware -
upstreamswap.Configinpkg/auth/upstreamswap/middleware.gohas a newProviderName stringfield with a comment stating it is derived fromUpstreams[0].Nameat construction time and defaults to"default" -
ServiceGettertype and all references toserviceGetter/storageGetterare removed frompkg/auth/upstreamswap/middleware.go;createMiddlewareFuncno longer accepts aServiceGetterparameter -
upstreamswapmiddleware readsidentity.UpstreamTokens[cfg.ProviderName]directly from the pre-enrichedIdentity; when the token is absent or empty it returns HTTP 401 with aWWW-Authenticate: Bearer error="invalid_token"header -
addUpstreamSwapMiddlewareinpkg/runner/middleware.goderivesProviderNamefromconfig.EmbeddedAuthServerConfig.Upstreams[0].Name, falling back to"default"when the name is empty; the derivedProviderNameis set onupstreamswap.Configbefore serialization -
GetUpstreamTokenService()is removed from theMiddlewareRunnerinterface inpkg/transport/types/transport.goif no other middleware callers remain after this refactor; the mock atpkg/transport/types/mocks/mock_transport.gois regenerated -
pkg/auth/middleware_test.gois updated: tests verifyGetAllUpstreamTokensis called whentsidclaim is present, expired tokens are included inUpstreamTokensas-is, andUpstreamTokensis nil/empty when notsidclaim is present -
pkg/auth/upstreamswap/middleware_test.gois updated: tests verifyProviderNameconfig field, middleware reads fromidentity.UpstreamTokensdirectly, middleware returns 401 when the named provider token is absent, andStorageGetteris no longer part of the test setup -
pkg/runner/middleware_test.gois updated: tests verifyProviderNameis derived fromUpstreams[0].Name, fallback to"default"when name is empty, andStorageGetteris no longer injected into middleware params -
task license-checkpasses (all new/modified.gofiles have SPDX headers) -
task lint-fixpasses with no remaining lint errors -
task testpasses (unit tests)
Technical Approach
Recommended Implementation
Start with the data model in pkg/auth/identity.go — add the UpstreamTokens field and update MarshalJSON to redact token values. This is a self-contained change with no dependencies.
Next, add the upstreamStorage field to TokenValidator and insert the enrichment call in TokenValidator.Middleware. The insertion point is between the existing claimsToIdentity call and the WithIdentity call — this two-line gap (lines ~1087–1096) is where the new block belongs. Use an optional field pattern (nil = no enrichment) to preserve existing behavior for all non-embedded-AS deployments.
Then thread UpstreamTokenStorage from the runner through to TokenValidator by updating GetAuthenticationMiddleware in pkg/auth/utils.go (add an optional parameter or a functional option) and CreateMiddleware in pkg/auth/middleware.go (the runner already provides IDPTokenStorage() indirectly via embeddedAuthServer).
The threading path requires the runner to expose IDPTokenStorage() to the auth.CreateMiddleware factory. Currently the runner exposes GetUpstreamTokenService() on the MiddlewareRunner interface. After this phase, assess whether GetUpstreamTokenService() has any remaining callers — if upstreamswap no longer calls it, remove it from the interface and regenerate the mock.
Finally, simplify pkg/auth/upstreamswap/middleware.go: remove ServiceGetter, add ProviderName to Config, and rewrite createMiddlewareFunc to read identity.UpstreamTokens[cfg.ProviderName] instead of calling the service. Update addUpstreamSwapMiddleware in pkg/runner/middleware.go to derive ProviderName from config.EmbeddedAuthServerConfig.Upstreams[0].Name with fallback to "default".
Patterns & Frameworks
- Optional field pattern for storage dependency: Add
upstreamStorage storage.UpstreamTokenStorageas an optional field onTokenValidator(nil = feature disabled). This is consistent with Go's convention of zero-value-safe types. Existing deployments without an embedded auth server pass nil and observe no behavior change. - Enrichment is non-fatal: Per architecture doc core principle Implement secret store #4: if
GetAllUpstreamTokensfails, log at WARN withslog.WarnContextand continue with emptyUpstreamTokens. Do not fail the entire request — the downstream backend will produce its own auth error. - No token values in logs: Per core principle Implement secret injection #5:
identity.UpstreamTokensvalues (access tokens) must be redacted inMarshalJSON. Provider name keys are safe to log. TheString()method currently returns onlyIdentity{Subject:%q}— this format is already safe. slogstructured logging: Useslog.WarnContext(r.Context(), "failed to load upstream tokens", "err", err)for the non-fatal error log. Do not useslog.Warn(use the context-aware variant).- Immutable assignment in middleware: Use an immediately-invoked anonymous function for any conditional value derivation (e.g., when computing the fallback provider name), per ToolHive Go coding style.
go.uber.org/mock(mockgen): IfGetUpstreamTokenService()is removed fromMiddlewareRunner, regeneratepkg/transport/types/mocks/mock_transport.gousinggo generate ./pkg/transport/types/...or the directive at the top oftransport.go.- SPDX license headers: All new or modified
.gofiles must have// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.and// SPDX-License-Identifier: Apache-2.0. Runtask license-fixto add them automatically. - Table-driven tests with
require.NoError: Follow the existing patterns inmiddleware_test.goandupstreamswap/middleware_test.go. Userequire.NoError(t, err)rather thant.Fatal.
Code Pointers
pkg/auth/identity.go— AddUpstreamTokens map[string]stringfield toIdentitystruct and updateMarshalJSONto redact values. TheSafeIdentityshadow struct pattern already handles redaction ofToken; add a parallel map with redacted values forUpstreamTokens.pkg/auth/token.golines 354–383 —TokenValidatorstruct definition. AddupstreamStorage storage.UpstreamTokenStorageas the last field. Lines 1086–1097 — the insertion point for the enrichment block betweenclaimsToIdentityandWithIdentity.pkg/auth/token.golines 557–655 —NewTokenValidatorconstructor. Add aWithUpstreamStorage(stor storage.UpstreamTokenStorage) TokenValidatorOptionfunctional option (or equivalent) to wire the storage dependency without breaking the existing call signature.pkg/auth/utils.goline 64–76 —GetAuthenticationMiddleware. ThreadUpstreamTokenStoragethrough here toNewTokenValidator. The function currently createsNewTokenValidator(ctx, *oidcConfig)— add an optional parameter or a functional option for the storage.pkg/auth/middleware.golines 47–74 —CreateMiddlewarefactory. The runner exposesGetUpstreamTokenService()ontypes.MiddlewareRunnertoday; after this change a new method (or repurposed method) is needed to exposeIDPTokenStorage(). Evaluate whether to add aGetUpstreamTokenStorage() storage.UpstreamTokenStoragemethod toMiddlewareRunneror reuse the existing interface extension point.pkg/auth/upstreamswap/middleware.go— Complete rewrite ofcreateMiddlewareFunc. RemoveServiceGettertype (line 47) and theserviceGetterparameter. AddProviderNametoConfigstruct (line 32–38). In the middleware handler, replace the four-steptsid → serviceGetter → GetValidTokens → injectflow with a two-stepUpstreamTokens[cfg.ProviderName] → injectread.pkg/runner/middleware.golines 252–282 —addUpstreamSwapMiddleware. DeriveProviderNamefromconfig.EmbeddedAuthServerConfig.Upstreams[0].Name, defaulting to"default"when the name is empty. Set it on theupstreamswap.Configbefore creating the middleware config.pkg/transport/types/transport.golines 65–87 —MiddlewareRunnerinterface. After removing theServiceGetterdependency fromupstreamswap, assess whetherGetUpstreamTokenService()(line 86) has any remaining callers. If none remain, remove it and add aGetUpstreamTokenStorage() storage.UpstreamTokenStoragemethod if needed by the auth middleware enrichment path.pkg/transport/types/mocks/mock_transport.go— Regenerated file; do not edit by hand. Rungo generate ./pkg/transport/types/...after any interface changes.pkg/auth/upstreamswap/middleware_test.go— Reference for the existing test patterns. After removingServiceGetter, simplifyrequestWithIdentityto includeUpstreamTokensin theIdentitydirectly rather than using mock storage. RemoveserviceGetterFromMocksandnilServiceGetterhelpers that are no longer needed.pkg/runner/middleware_test.golines 22–41 —createMinimalAuthServerConfig()helper withUpstreams[0].Name = "test-upstream". New test cases forProviderNamederivation should verify that the serialized params containProviderName: "test-upstream".
Component Interfaces
// pkg/auth/identity.go — updated Identity struct
type Identity struct {
Subject string
Name string
Email string
Groups []string
Claims map[string]any
Token string
TokenType string
Metadata map[string]string
// UpstreamTokens holds upstream access tokens keyed by provider name.
// Populated by auth middleware after JWT validation when the embedded auth
// server is in use (tsid claim present + upstreamStorage configured).
// Expired tokens are included as-is; callers must not attempt refresh.
// Empty for non-embedded-AS requests.
UpstreamTokens map[string]string // providerName → access token
}
// MarshalJSON must redact UpstreamTokens values:
// SafeIdentity.UpstreamTokens: build map[string]string with same keys but "REDACTED" values// pkg/auth/token.go — TokenValidator enrichment insertion point
// (between claimsToIdentity and WithIdentity)
if v.upstreamStorage != nil {
if tsid, ok := identity.Claims[upstreamtoken.TokenSessionIDClaimKey].(string); ok && tsid != "" {
allTokens, err := v.upstreamStorage.GetAllUpstreamTokens(r.Context(), tsid)
if err != nil {
slog.WarnContext(r.Context(), "failed to load upstream tokens", "err", err)
} else {
identity.UpstreamTokens = make(map[string]string, len(allTokens))
for name, t := range allTokens {
identity.UpstreamTokens[name] = t.AccessToken // include expired
}
}
}
}// pkg/auth/upstreamswap/middleware.go — updated Config and createMiddlewareFunc
type Config struct {
HeaderStrategy string `json:"header_strategy,omitempty" yaml:"header_strategy,omitempty"`
CustomHeaderName string `json:"custom_header_name,omitempty" yaml:"custom_header_name,omitempty"`
// ProviderName is the upstream provider whose token to inject.
// Derived from Upstreams[0].Name at construction time; defaults to "default".
ProviderName string `json:"provider_name" yaml:"provider_name"`
}
// ServiceGetter type and storageGetter field are removed entirely.
// createMiddlewareFunc: simplified handler reads from identity.UpstreamTokens
func createMiddlewareFunc(cfg *Config) types.MiddlewareFunction {
// ... injectToken determined at startup time (unchanged) ...
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
identity, ok := auth.IdentityFromContext(r.Context())
if !ok {
next.ServeHTTP(w, r)
return
}
// No tsid check needed — if UpstreamTokens is empty, behave same as absent
token, exists := identity.UpstreamTokens[cfg.ProviderName]
if !exists || token == "" {
writeUpstreamAuthRequired(w)
return
}
injectToken(r, token)
next.ServeHTTP(w, r)
})
}
}// pkg/runner/middleware.go — addUpstreamSwapMiddleware ProviderName derivation
func addUpstreamSwapMiddleware(
middlewares []types.MiddlewareConfig,
config *RunConfig,
) ([]types.MiddlewareConfig, error) {
if config.EmbeddedAuthServerConfig == nil {
return middlewares, nil
}
providerName := func() string {
if len(config.EmbeddedAuthServerConfig.Upstreams) > 0 &&
config.EmbeddedAuthServerConfig.Upstreams[0].Name != "" {
return config.EmbeddedAuthServerConfig.Upstreams[0].Name
}
return "default"
}()
upstreamSwapConfig := config.UpstreamSwapConfig
if upstreamSwapConfig == nil {
upstreamSwapConfig = &upstreamswap.Config{}
}
upstreamSwapConfig.ProviderName = providerName
// ... rest unchanged ...
}Testing Strategy
Unit Tests — pkg/auth/middleware_test.go
-
GetAllUpstreamTokensis called whentsidclaim is present andupstreamStorageis non-nil; tokens are populated inidentity.UpstreamTokenskeyed by provider name - Expired tokens are included in
UpstreamTokensas-is (no filtering by expiry at enrichment time) -
UpstreamTokensis nil/empty when thetsidclaim is absent from the JWT -
UpstreamTokensis nil/empty whenupstreamStorageis nil (non-embedded-AS deployment) - When
GetAllUpstreamTokensreturns an error, the middleware logs WARN and proceeds with emptyUpstreamTokens(does not return HTTP error) -
MarshalJSONonIdentitywith populatedUpstreamTokensredacts token values to"REDACTED"while preserving provider name keys
Unit Tests — pkg/auth/upstreamswap/middleware_test.go
- Middleware reads
identity.UpstreamTokens[cfg.ProviderName]and injects the token when present - Middleware returns HTTP 401 with
WWW-Authenticate: Bearer error="invalid_token"whenidentity.UpstreamTokens[cfg.ProviderName]is absent - Middleware returns HTTP 401 when the token value is an empty string
-
ProviderNameconfig field is respected — using"provider-a"readsUpstreamTokens["provider-a"], notUpstreamTokens["provider-b"] - Middleware proceeds without swap when
identityhas noUpstreamTokensmap at all (nil map) — returns 401 since token is required for an embedded-AS-configured runner -
CreateMiddlewarefactory no longer callsGetUpstreamTokenService()on the runner mock -
TestCreateMiddlewaretest cases do not expectmockRunner.EXPECT().GetUpstreamTokenService()to be called
Unit Tests — pkg/runner/middleware_test.go
-
addUpstreamSwapMiddlewarewithUpstreams[0].Name = "test-upstream"producesupstreamswap.Config{ProviderName: "test-upstream"} -
addUpstreamSwapMiddlewarewithUpstreams[0].Name = ""producesupstreamswap.Config{ProviderName: "default"} -
addUpstreamSwapMiddlewarewith emptyUpstreamsslice producesupstreamswap.Config{ProviderName: "default"} - Serialized
upstreamswap.MiddlewareParams.Config.ProviderNamematches the derived name in all existingTestAddUpstreamSwapMiddlewarecases
Edge Cases
-
Identity.MarshalJSON()with an emptyUpstreamTokensmap (map[string]string{}) serializes to an empty object or omitted field, not"REDACTED" -
upstreamswapmiddleware withProviderName = ""behaves deterministically (either returns 401 becauseUpstreamTokens[""]is absent, orvalidateConfigrejects an emptyProviderName— document the chosen behavior) -
TokenValidatorwithupstreamStorage = niland a JWT containing atsidclaim does not panic and does not call any storage method
Out of Scope
- Handler layer changes (sequential chain logic,
nextMissingUpstream, multi-upstream authorize/callback): covered in TASK-002 - Config and operator validation changes (removing/moving
len > 1guard): covered in TASK-003 - Storage layer changes (
UpstreamTokenStorageinterface, backends): covered in TASK-001 (Implement multi-provider upstream token storage layer (RFC-0052 Phase 1) #4136) - Transparent token refresh or re-authorization during middleware enrichment (deferred to a later RFC)
- Distributed locking for concurrent token refresh operations
- Multi-upstream IDP support in the CLI proxy runner or
MCPServer/MCPRemoteProxyK8s workload types - Dynamic upstream IDP registration at runtime
References
- RFC:
docs/proposals/THV-0052-multi-upstream-idp-authserver.md - Parent epic: Auth Server: multi-upstream provider support #3924
- Phase 1 (Storage Layer): Implement multi-provider upstream token storage layer (RFC-0052 Phase 1) #4136
- Architecture overview:
docs/arch/11-auth-server-storage.md - Related Stacklok epic: https://github.com/stacklok/stacklok-epics/issues/251