Implement multi-upstream sequential authorization chain#4283
Conversation
Move the auth server handler from a single-upstream model to a map-of-providers with an ordered list. This is the mechanical refactor that threads multi-upstream support through config validation, Handler construction, AuthorizeHandler, CallbackHandler, server construction, and all existing tests. No new behavior — the chain still completes after the first upstream.
After the handler refactor, wire up the actual chain behavior: the callback handler now walks the upstream order, redirects to the next missing provider, and only issues the authorization code once every upstream has been satisfied. Identity fields from the first upstream carry through the chain via PendingAuthorization, and the refresher dispatches by ProviderID.
Unit tests for chain state inspection and multi-upstream callback flow, plus an integration test that drives two mock OIDC providers through the full sequential chain end-to-end.
Per silent-success convention, successful operations should not log at Info level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace map+slice pair with []NamedUpstream, eliminating the cross-validation logic that kept them in sync. Return error from NewHandler instead of panicking on invalid inputs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4283 +/- ##
==========================================
+ Coverage 68.54% 69.18% +0.64%
==========================================
Files 471 471
Lines 47732 47761 +29
==========================================
+ Hits 32717 33045 +328
+ Misses 12247 12149 -98
+ Partials 2768 2567 -201 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I explained the size, go away now
|
Thanks for the review, I'll add the fixups to the next PR in the interest of moving fast. |
Summary
Handlerfrom a singleupstreamfield to an ordered[]NamedUpstreamslice, wiresSessionIDgeneration intoAuthorizeHandler(threaded across all chain legs), and addscontinueChainOrCompletelogic inCallbackHandlerthat either redirects to the next upstream or issues the final auth code.upstreamByNamelookup.Closes #4137
Type of change
Test plan
task test)task lint-fix)thv-as-vmcp) where the full multi-upstream flow was tested end-to-end with real OIDC providers against the Entra OBO test environment.Changes
pkg/authserver/server/handlers/handler.go[]NamedUpstreamslice; addNamedUpstreamtype,nextMissingUpstream,upstreamByNamehelpers; hardenNewHandlerconstructor with validationpkg/authserver/server/handlers/authorize.goSessionIDat chain start; targetupstreams[0]as first leg; use slice-based provider lookuppkg/authserver/server/handlers/callback.gocontinueChainOrCompletechain logic; carry identity from first leg throughResolvedUser*fields; add identity mismatch detection; clean up tokens on all failure pathspkg/authserver/server/handlers/user.gopkg/authserver/server_impl.gocfg.Upstreams; pass toNewHandler; updateUpstreamTokenRefresherto dispatch by provider namepkg/authserver/config.golen > 1guard on upstreams; update doc commentspkg/authserver/refresher.gopkg/authserver/server/handlers/handler_chain_test.gonextMissingUpstream(all-satisfied, first-missing, partial, storage error)pkg/authserver/server/handlers/callback_test.gopkg/authserver/integration_test.gopkg/authserver/server/handlers/helpers_test.gomultiUpstreamTestSetup,baseTestSetupOption, multi-provider storage mock wiringDoes this introduce a user-facing change?
Multiple upstream IDPs can now be configured. The auth server will sequentially authorize through each before issuing a token.
Special notes for reviewers
PendingAuthorization.ResolvedUser*fields rather than re-resolving.nextMissingUpstreamreturns error (not fail-safe restart): The issue spec suggested returningupstreamOrder[0]on storage error. The implementation instead returns the error, allowing the caller to clean up and return a proper error to the client. A silent restart could loop indefinitely if storage is persistently down.Large PR Justification
The size owes itself mostly to tests
Generated with Claude Code