-
Notifications
You must be signed in to change notification settings - Fork 198
Implement multi-provider upstream token storage layer (RFC-0052 Phase 1) #4136
Description
Description
Add a providerName dimension to the UpstreamTokenStorage interface so that multiple upstream IDPs can have their tokens stored per-session without overwriting each other. This is the foundational storage layer change for RFC-0052 — all subsequent phases (handler chain, config/operator, identity enrichment) break at compile time without these new interface signatures landing first.
Context
RFC-0052 extends the embedded OAuth authorization server to support multiple upstream IDPs in a single instance. Today, UpstreamTokenStorage stores exactly one set of tokens per session (map[sessionID]*timedEntry), which makes it impossible to accumulate tokens from a second provider during a sequential authorization chain. This task restructures the storage layer to accommodate N providers per session by keying on (sessionID, providerName).
This is the root task of the RFC-0052 implementation plan. TASK-002 (Handler and Server Layer), TASK-003 (Config and Operator), and TASK-004 (Identity Enrichment) all depend on the new interface signatures produced here.
Dependencies: None (root task)
Blocks: TASK-002, TASK-003, TASK-004
Breaking Change — Redis key pattern: Upstream tokens stored under the old Redis key pattern
{prefix}upstream:{sessionID}(a single key per session) will be unreachable after upgrade. There is no automatic migration. Existing sessions will require re-authentication after deploying this change. This must be documented in the PR description.
Acceptance Criteria
-
UpstreamTokenStorageinterface inpkg/authserver/storage/types.gohas updated signatures:StoreUpstreamTokens(ctx, sessionID, providerName string, tokens *UpstreamTokens),GetUpstreamTokens(ctx, sessionID, providerName string),GetAllUpstreamTokens(ctx, sessionID string) (map[string]*UpstreamTokens, error), andDeleteUpstreamTokens(ctx, sessionID string)(session-scoped delete, signature unchanged) -
PendingAuthorizationstruct inpkg/authserver/storage/types.gohas two new fields:UpstreamProviderName stringandSessionID stringwith doc comments matching the architecture spec -
MemoryStorage.upstreamTokensfield is restructured frommap[string]*timedEntry[*UpstreamTokens]tomap[string]map[string]*timedEntry[*UpstreamTokens](outer key = sessionID, inner key = providerName) -
MemoryStorage.StoreUpstreamTokensandGetUpstreamTokensaccept and useproviderName; both validate thatproviderNameis non-empty and return a clear error if it is -
MemoryStorage.GetAllUpstreamTokensis implemented: returns amap[string]*UpstreamTokens(defensive copies) for all providers under the given sessionID; returns an empty map (not an error) for an unknown sessionID -
MemoryStorageeviction TTL for upstream tokens usesmax(accessToken.ExpiresAt, refreshToken.ExpiresAt)instead oftokens.ExpiresAt.Add(DefaultRefreshTokenTTL) - Redis storage uses new key pattern
{prefix}upstream:{sessionID}:{providerName}for each provider key (via newredisUpstreamKeyhelper inredis_keys.go) - Redis storage maintains a session-scoped index SET at
{prefix}upstream:idx:{sessionID}(using SADD) for efficientGetAllUpstreamTokensand session-level delete -
RedisStorage.StoreUpstreamTokensuses a Lua script for atomic write of the provider key + SADD to the session index SET -
RedisStorage.DeleteUpstreamTokensuses a Lua script for atomic SMEMBERS + DEL of all provider keys + DEL of the index SET -
RedisStorage.GetAllUpstreamTokensis implemented using SMEMBERS + MGET (two round-trips); returns empty map for unknown session -
RedisStorage.StoreUpstreamTokensandGetUpstreamTokensvalidate thatproviderNameis non-empty and return a clear error if it is - All call sites of
StoreUpstreamTokensandGetUpstreamTokenswithinpkg/authserver/are updated to pass aproviderNameargument:pkg/authserver/server/handlers/callback.goandpkg/authserver/refresher.go - Storage mocks in
pkg/authserver/storage/mocks/mock_storage.goare regenerated viamockgento reflect the new interface signatures -
memory_test.gois updated: existing upstream token tests passproviderName; new test cases cover multi-provider store/get, session delete wipes all providers,GetAllUpstreamTokensreturns all providers (including those with expired tokens),GetAllUpstreamTokensreturns empty map for unknown session, and eviction TTL usesmax(access.ExpiresAt, refresh.ExpiresAt) -
redis_test.goandredis_integration_test.goare updated with equivalent coverage for the Redis backend (miniredis and real Redis respectively) -
task license-checkpasses (all new/modified.gofiles have SPDX headers) -
task lint-fixpasses with no remaining lint errors -
task testpasses (unit tests) - PR description documents the Redis breaking change (old key pattern
{prefix}upstream:{sessionID}becomes unreachable; sessions require re-authentication) -
upstreamtoken.Serviceinterface inpkg/auth/upstreamtoken/types.gohas updatedGetValidTokenssignature:GetValidTokens(ctx context.Context, sessionID, providerName string) (*UpstreamCredential, error) -
InProcessService.GetValidTokensinpkg/auth/upstreamtoken/service.goaccepts and usesproviderNamewhen callingstorage.GetUpstreamTokens;refreshOrFailis updated to threadproviderNamethrough its call chain -
upstreamswapmiddlewareConfigstruct inpkg/auth/upstreamswap/middleware.gogains aProviderName stringfield;validateConfigreturns an error ifProviderNameis empty -
upstreamswapmiddleware passescfg.ProviderNameas theproviderNameargument tosvc.GetValidTokensincreateMiddlewareFunc - The proxy runner derives
ProviderNamefrom the single upstream provider name in the embedded auth server config and stores it in theMiddlewareConfigparameters for theupstreamswapmiddleware during wiring (the proxy runner only ever has one upstream provider, so a singleProviderNameinMiddlewareConfigis sufficient for Phase 1) - Mocks for
upstreamtoken.Service(if generated) are regenerated to reflect the newGetValidTokenssignature -
pkg/auth/upstreamswap/middleware_test.gois updated: existing tests passproviderNametoGetValidTokens; new test cases cover emptyProviderNameconfig validation and correct forwarding ofProviderNameto the service
Technical Approach
Recommended Implementation
Start with pkg/authserver/storage/types.go — update the UpstreamTokenStorage interface and PendingAuthorization struct first so the compiler immediately surfaces every call site that needs updating. Then update memory.go (restructure the nested map and TTL logic), then redis_keys.go (add redisUpstreamKey), then redis.go (new key pattern, session index, Lua scripts, GetAllUpstreamTokens). Update the two call sites in callback.go and refresher.go last (they will need a providerName argument — derive this from pending.UpstreamProviderName in the callback handler, and from expired.ProviderID in the refresher). Regenerate mocks after all interface changes are complete.
Next, propagate providerName up through the token service layer: update upstreamtoken.Service.GetValidTokens in pkg/auth/upstreamtoken/types.go and InProcessService.GetValidTokens in pkg/auth/upstreamtoken/service.go. Finally, wire the provider name into the upstreamswap middleware: add ProviderName to Config, enforce it in validateConfig, pass it to GetValidTokens in createMiddlewareFunc, and update the proxy runner to derive the provider name from the embedded auth server config when constructing the middleware config.
Patterns and Frameworks
- Defensive copies on read and write: Follow the existing pattern in
MemoryStorage.StoreUpstreamTokensandGetUpstreamTokens— make a full struct copy before storing and before returning, so external mutations do not affect stored state. Preserve this when restructuring to the nested map. - Lua scripts for atomicity in Redis: Follow the existing
storeUpstreamTokensScriptpattern inredis.go. The newStoreUpstreamTokensLua script must atomically write the provider key and SADD to the session index SET in one round-trip. The newDeleteUpstreamTokensLua script must atomically SMEMBERS, DEL all member keys, and DEL the index SET. warnOnCleanupErrfor best-effort secondary index cleanup: Use this existing helper when non-critical index cleanup operations fail (seeredis.go).redisKey/redisProviderKey/redisSetKeykey generation conventions: Follow the samefmt.Sprintfformat inredis_keys.gofor the newredisUpstreamKeyhelper.errors.Is()/fmt.Errorfwith%w: Use for error wrapping and inspection throughout; never swallow errors silently.- Table-driven tests with
require.NoError: Follow patterns inmemory_test.goandredis_test.go; userequire.NoError(t, err)rather thant.Fatal. go.uber.org/mock(mockgen): Regenerate mocks withmockgen -destination=mocks/mock_storage.go -package=mocks -source=types.go Storage,PendingAuthorizationStorage,ClientRegistry,UpstreamTokenStorage,UpstreamTokenRefresher,UserStorage.- SPDX license headers: All new or modified
.gofiles must have SPDX headers; runtask license-fixto add them automatically.
Code Pointers
pkg/authserver/storage/types.go— Primary file to modify:UpstreamTokenStorageinterface signatures (addproviderNameparam toStoreUpstreamTokens/GetUpstreamTokens, addGetAllUpstreamTokens), andPendingAuthorizationstruct (addUpstreamProviderNameandSessionIDfields). The//go:generate mockgendirective at the top of this file controls mock regeneration.pkg/authserver/storage/memory.golines 48–80 —MemoryStoragestruct definition: changeupstreamTokens map[string]*timedEntry[*UpstreamTokens]tomap[string]map[string]*timedEntry[*UpstreamTokens].pkg/authserver/storage/memory.golines 685–777 — CurrentStoreUpstreamTokens,GetUpstreamTokens,DeleteUpstreamTokens: shows the defensive copy pattern,timedEntrywrapper, eviction TTL calculation, and error wrapping to replicate when restructuring. TTL calculation on line 699 usestokens.ExpiresAt.Add(DefaultRefreshTokenTTL)— this must change tomax(accessToken.ExpiresAt, refreshToken.ExpiresAt)(note:UpstreamTokenscurrently has only oneExpiresAtfield; confirm whether the RFC intent is to trackRefreshTokenexpiry separately or to use existingExpiresAtas the access token expiry).pkg/authserver/storage/redis_keys.go— AddredisUpstreamKey(prefix, sessionID, providerName string) stringreturning"{prefix}upstream:{sessionID}:{providerName}". The session index SET key should follow theredisSetKeypattern:redisSetKey(prefix, "upstream:idx", sessionID)→"{prefix}upstream:idx:{sessionID}".pkg/authserver/storage/redis.golines 889–1004 — Current RedisStoreUpstreamTokens,GetUpstreamTokens,DeleteUpstreamTokensandstoreUpstreamTokensScriptLua script. The new implementation must replace the single-key pattern with the two-key pattern (provider key + session index SET). ThemarshalUpstreamTokensWithTTLhelper (line ~885) is reusable.pkg/authserver/storage/mocks/mock_storage.go— Generated file; do not edit by hand. Regenerate after all interface changes.pkg/authserver/server/handlers/callback.goline 127 — Call site forStoreUpstreamTokens; must addproviderNameargument. At this call site,providerNameshould come frompending.UpstreamProviderName(set by the authorize handler when it creates thePendingAuthorization). Note: in the current single-upstream handler,pending.UpstreamProviderNamedoes not exist yet — adding it toPendingAuthorizationhere is sufficient for Phase 1 correctness; Phase 2 will drive multi-upstream chain logic that populates it per-leg.pkg/authserver/server/handlers/callback.goline 141 —DeleteUpstreamTokenscall: signature is unchanged (session-scoped).pkg/authserver/refresher.goline 70 — Call site forStoreUpstreamTokens; must addproviderNameargument. Useexpired.ProviderIDas theproviderName(the refresher already has access to the expired token struct which carries the provider ID).pkg/authserver/storage/memory_test.goline 429 —TestMemoryStorage_UpstreamTokenstest suite: update all test cases with the newproviderNameparameter and add new multi-provider andGetAllUpstreamTokenscases.pkg/authserver/storage/redis_test.goandpkg/authserver/storage/redis_integration_test.go— Redis equivalents; follow the same test pattern updates.pkg/auth/upstreamtoken/types.go—Serviceinterface: updateGetValidTokenssignature to addproviderName stringas the second parameter (aftersessionID).pkg/auth/upstreamtoken/service.goline 50 —InProcessService.GetValidTokens: addproviderName stringparameter; pass it tostorage.GetUpstreamTokensand thread it throughrefreshOrFail(which callsRefreshAndStore— the refresher signature itself is not changing in Phase 1, butproviderNamemust be available soStoreUpstreamTokensinside the refresher can use it).pkg/auth/upstreamswap/middleware.goline 32 —Configstruct: addProviderName string \json:"provider_name" yaml:"provider_name"`` field representing the single upstream provider this proxy runner instance serves.pkg/auth/upstreamswap/middleware.goline 96 —validateConfig: add a check thatcfg.ProviderName != ""; return a descriptive error if empty.pkg/auth/upstreamswap/middleware.goline 190 —svc.GetValidTokens(r.Context(), tsid)call site: update tosvc.GetValidTokens(r.Context(), tsid, cfg.ProviderName).- Proxy runner wiring code (search for
upstreamswaporMiddlewareConfigconstruction inpkg/runner/orcmd/thv/) — locate where theupstreamswapmiddleware config is built and addProviderNamederived from the single upstream IDP provider name in the embedded auth server config.
Component Interfaces
// pkg/authserver/storage/types.go
// UpstreamTokenStorage — updated interface
type UpstreamTokenStorage interface {
// StoreUpstreamTokens stores upstream IDP tokens for (sessionID, providerName).
// Returns an error if sessionID or providerName is empty.
StoreUpstreamTokens(ctx context.Context, sessionID, providerName string, tokens *UpstreamTokens) error
// GetUpstreamTokens retrieves upstream IDP tokens for (sessionID, providerName).
// Returns ErrNotFound if the entry does not exist.
// Returns ErrExpired (with token data) if the access token has expired — caller
// may use the refresh token for transparent renewal.
// Returns ErrInvalidBinding if binding validation fails.
GetUpstreamTokens(ctx context.Context, sessionID, providerName string) (*UpstreamTokens, error)
// GetAllUpstreamTokens returns all upstream tokens for the session, keyed by
// provider name. Returns an empty map (not an error) for an unknown sessionID.
// Expired tokens are included; callers that need freshness checks should use
// GetUpstreamTokens per-provider.
GetAllUpstreamTokens(ctx context.Context, sessionID string) (map[string]*UpstreamTokens, error)
// DeleteUpstreamTokens removes all upstream tokens for the session (all providers).
// Returns ErrNotFound if the session does not exist.
DeleteUpstreamTokens(ctx context.Context, sessionID string) error
}
// PendingAuthorization — new fields added to existing struct
type PendingAuthorization struct {
// ... existing fields unchanged ...
// UpstreamProviderName is the provider being authenticated in this chain leg.
// Set by the authorize handler when creating the PendingAuthorization;
// read by the callback handler to key token storage.
UpstreamProviderName string
// SessionID is the TSID for the session being accumulated across all chain legs.
// Generated server-side only using rand.Text(); never accepted from client input.
SessionID string
}// pkg/authserver/storage/redis_keys.go — new helper
// redisUpstreamKey generates a Redis key for a per-provider upstream token entry.
// Format: "{prefix}upstream:{sessionID}:{providerName}"
// Note: sessionID and providerName must not contain colons in practice
// (sessionID is rand.Text() output; providerName is a config-supplied name).
func redisUpstreamKey(prefix, sessionID, providerName string) string {
return fmt.Sprintf("%s%s:%s:%s", prefix, KeyTypeUpstream, sessionID, providerName)
}
// Session index SET key (use existing redisSetKey):
// redisSetKey(prefix, "upstream:idx", sessionID) → "{prefix}upstream:idx:{sessionID}"// pkg/authserver/storage/memory.go — MemoryStorage struct field change
// upstreamTokens maps sessionID -> providerName -> timedEntry.
// Outer key is the session TSID; inner key is the provider name.
upstreamTokens map[string]map[string]*timedEntry[*UpstreamTokens]// pkg/auth/upstreamtoken/types.go — updated Service interface
// Service owns the upstream token lifecycle: read, refresh, error handling.
type Service interface {
// GetValidTokens returns a valid upstream credential for (sessionID, providerName).
// It transparently refreshes expired access tokens using the refresh token.
//
// Returns:
// - *UpstreamCredential on success
// - ErrSessionNotFound if no upstream tokens exist for the session
// - ErrNoRefreshToken if the access token is expired and no refresh token is available
// - ErrRefreshFailed if the refresh attempt fails (e.g., revoked refresh token)
GetValidTokens(ctx context.Context, sessionID, providerName string) (*UpstreamCredential, error)
}// pkg/auth/upstreamswap/middleware.go — Config struct update
// Config holds configuration for upstream swap middleware.
type Config struct {
// ProviderName is the upstream IDP provider name used to key token storage.
// For the proxy runner this is derived from the single upstream provider in
// the embedded auth server config. Required — validation returns an error if empty.
ProviderName string `json:"provider_name" yaml:"provider_name"`
// HeaderStrategy determines how to inject the token: "replace" (default) or "custom".
HeaderStrategy string `json:"header_strategy,omitempty" yaml:"header_strategy,omitempty"`
// CustomHeaderName is the header name when HeaderStrategy is "custom".
CustomHeaderName string `json:"custom_header_name,omitempty" yaml:"custom_header_name,omitempty"`
}Testing Strategy
Unit Tests — pkg/authserver/storage/memory_test.go
-
TestMemoryStorage_UpstreamTokens: update all existing subtests to passproviderName(e.g.,"provider-a") - Multi-provider store/get: store tokens for two providers under the same sessionID; verify each
GetUpstreamTokenscall returns the correct tokens for each provider independently - Cross-provider isolation: storing tokens for provider A does not overwrite or affect tokens for provider B
-
GetAllUpstreamTokens— two providers: store two providers, callGetAllUpstreamTokens, verify map has exactly two entries with correct defensive copies -
GetAllUpstreamTokens— unknown session: callGetAllUpstreamTokensfor a sessionID that has never been stored; verify returns empty map and no error -
GetAllUpstreamTokens— includes expired tokens: store a token with an expiredExpiresAt; verifyGetAllUpstreamTokensincludes it (no expiry filtering at bulk-read level) - Session delete wipes all providers: store two providers, call
DeleteUpstreamTokens, verify both providers are gone (subsequentGetUpstreamTokensfor each returnsErrNotFound) - Eviction TTL uses
max(access.ExpiresAt, refresh.ExpiresAt): confirm the cleanup loop evicts entries at the correct deadline (test using manipulatedtime.Nowor very short TTLs) -
StoreUpstreamTokenswith emptyproviderNamereturns a non-nil error -
GetUpstreamTokenswith emptyproviderNamereturns a non-nil error
Unit Tests — pkg/authserver/storage/redis_test.go (miniredis)
- Same multi-provider store/get coverage as memory tests above
-
GetAllUpstreamTokensreturns empty map for unknown session (SMEMBERS on non-existent key) - Session index SET (
{prefix}upstream:idx:{sessionID}) is populated on store and cleaned up atomically on delete -
StoreUpstreamTokenswith emptyproviderNamereturns a non-nil error -
GetUpstreamTokenswith emptyproviderNamereturns a non-nil error
Integration Tests — pkg/authserver/storage/redis_integration_test.go
- Multi-provider round-trip: store two providers, retrieve individually and via
GetAllUpstreamTokens, verify correct values (requires//go:build integrationtag and real Redis via testcontainers) - Session delete wipes all providers atomically in Redis
- TTL expiry: keys expire from Redis after the configured TTL (verify via
PTTLor waiting)
Unit Tests — pkg/auth/upstreamswap/middleware_test.go
- Update existing tests: mock
svc.GetValidTokensnow expects(ctx, sessionID, providerName)— update expectations to pass the configuredProviderName - Empty
ProviderNamein config:CreateMiddleware(orvalidateConfig) returns a non-nil error -
ProviderNameforwarding: verify the middleware passescfg.ProviderNametoGetValidTokens(not a hardcoded value)
Edge Cases
-
GetAllUpstreamTokenswith a sessionID that has one expired and one valid provider: verify both are returned (expiry is per-provider at theGetUpstreamTokenslevel, not filtered at bulk-read) - Concurrent
StoreUpstreamTokensfor different providers under the same sessionID (memory backend): verify no data corruption under thesync.RWMutex - Redis Lua script atomicity: verify that a
StoreUpstreamTokensfailure partway through does not leave the session index SET in an inconsistent state (e.g., SADD succeeds but key write fails — Lua rollback)
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 Identity.UpstreamTokensenrichment: covered in TASK-004- Full Redis migration tooling for tokens stored under the old
{prefix}upstream:{sessionID}key pattern UpstreamTokenRefresher.RefreshAndStoremulti-provider support (the refresher inpkg/authserver/refresher.gocurrently takes a singlesessionID; Phase 1 only updates theStoreUpstreamTokenscall within it to passproviderName— no signature change toRefreshAndStoreitself)- Transparent token refresh or distributed locking for concurrent refresh races
- Dynamic upstream IDP registration at runtime
- Multi-provider
upstreamswapmiddleware support (TASK-004 will handle the case where the proxy runner needs to serve multiple upstream providers; Phase 1 configures a singleProviderNameper middleware instance)
References
- RFC:
docs/proposals/THV-0052-multi-upstream-idp-authserver.md - Parent epic: Auth Server: multi-upstream provider support #3924
- Architecture doc for auth server storage:
docs/arch/11-auth-server-storage.md - Related Stacklok epic: https://github.com/stacklok/stacklok-epics/issues/251