Filter upstream auth chain via callback hook

[tgrunnagle]

Summary

The embedded OAuth authorization server walks every configured upstream provider
on every authorization. There is no way for a consumer to say that only a subset
of the configured upstreams is relevant for a given authorization, so the flow
prompts for upstreams the authorization does not need and stores upstream tokens
that are never used — widening the stored-token surface for no benefit.

This PR adds an optional filter hook, consulted once in the callback handler after
the first upstream resolves, that narrows the remaining chain to a subset of the
configured upstreams. The filter is given the identity established by the first
upstream — the canonical user plus the authorization-relevant claims — so it can
decide the subset based on who authenticated. The change is fully backward
compatible: with no filter configured, the handler walks all configured upstreams
exactly as before.

• Why: let consumers scope an authorization to only the upstreams it needs —
  keyed on the authenticated principal — avoiding needless prompts and reducing
  the stored-token surface.
• What: a new UpstreamFilter interface and WithUpstreamFilter construction
  option. The filter receives the canonical platform user ID and the resolved
  auth.PrincipalInfo (claim-mapped subject, name, email, and the Claims map
  used by authorization policies) alongside the non-first configured upstream
  names. The effective chain is computed once on the first leg and carried in
  PendingAuthorization so the filter is not re-run per leg; subsequent legs reuse
  the carried chain after validating it against the configured upstreams. The
  missing-leg walk and the cross-leg identity check operate on the effective chain
  rather than the raw config. Upstream ID-token / userinfo claims — previously
  discarded — are now captured into upstream.Identity so the filter can use them.

Closes #5724

Large PR Justification

This PR trips the >1000-line gate almost entirely because of test coverage, not
feature code:

• Non-test code: ~371 changed lines (+334 / -37) — within the 400-line
  guideline. This is the cohesive UpstreamFilter hook plus the review-driven
  hardening (in-order chain validation, duplicate-upstream-name rejection at
  construction, and the identity/claims plumbing through the callback).
• Tests: ~849 added lines (~69% of the diff) — the bulk of the size. Most were
  added at reviewers' request across review rounds: an OIDC-provider integration
  test for ID-token claim capture, callback/chain integration tests driven through
  the real embedded auth server, a validateChain subsequence table test, a
  duplicate-name constructor test, and storage round-trip / legacy-record
  backward-compatibility tests.

The feature and its tests are one logical change — cheaper to review and safer to
revert together than fragmented across PRs — and the change already carries a human
approval (@jhrozek).

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Unit and integration tests cover claim capture, the callback flow, chain
computation, identity propagation, and storage round-trips:

• OIDC provider integration (real OIDCProviderImpl + mock IdP): an ID token
  carrying email and a multi-valued groups claim is captured into
  Identity.Claims.
• Callback → filter propagation: the first upstream's subject and claims, and
  the canonical platform user ID, reach FilterUpstreams during callback handling.
• Filter drops the second leg → authorization completes at the first upstream and
  issues the code without prompting for the dropped leg.
• Filter keeps the second leg → the effective chain is carried forward in the
  pending authorization.
• Filter error → the authorization fails cleanly with a server error (no fallback
  to a full walk).
• Second leg reuses the carried chain and does not re-run the filter.
• A subsequent-leg pending that lacks a computed chain (a rolling-upgrade window)
  is rejected and fails closed rather than recomputing against a later leg.
• computeChain table cases: no-filter full walk in order, single upstream skips
  the filter, subset kept with the first upstream always leading, empty keep set
  leaves only the first upstream, filter return order ignored in favor of
  configured order, unknown / first-upstream names in the keep set ignored, filter
  error propagated, and the principal passed through to the filter verbatim.
• nextMissingUpstream respects the chain subset rather than the raw config.
• Storage round-trip of ChainUpstreams (in-memory and Redis), plus a
  backward-compatibility test loading a legacy Redis pending without
  chain_upstreams.

Changes

File	Change
pkg/authserver/server/handlers/handler.go	Add UpstreamFilter interface (takes the platform user ID + auth.PrincipalInfo + configured upstreams), WithUpstreamFilter option, and computeChain / resolveChain / validateChain; nextMissingUpstream now walks the effective chain
pkg/authserver/server/handlers/callback.go	Build the first-leg principal and thread it to the filter; compute the effective chain once and carry it forward; reject a stale subsequent-leg pending (fail closed); extract verifyChainIdentity (with structured mismatch logging) gating on the effective chain
pkg/authserver/storage/types.go	Add ChainUpstreams field to PendingAuthorization
pkg/authserver/storage/memory.go, redis.go	Persist/restore ChainUpstreams
pkg/authserver/upstream/types.go	Add Claims to upstream.Identity
pkg/authserver/upstream/oidc.go, oauth2.go	Capture claims from the validated ID token (OIDC) and userinfo response (OAuth2)
pkg/authserver/**/*_test.go	Tests for claim capture, the filter hook + identity propagation, chain computation/validation, and storage round-trips

Does this introduce a user-facing change?

No end-user-facing change to the thv CLI. For consumers embedding the auth
server, this adds a new optional construction option (WithUpstreamFilter). It is
opt-in and behavior is unchanged when unset.

Special notes for reviewers

• First upstream is never filtered. An authorization always begins at
  upstreams[0]; it is not passed to the filter and cannot be removed by it.
• Filter can only narrow, never reorder or extend. computeChain iterates the
  configured order and ignores any returned name that is not a non-first configured
  upstream, so a filter cannot reorder the chain or inject unknown providers.
• Identity is passed explicitly, not via context. The callback deliberately
  avoids placing a bearer-less Identity in ctx (no ToolHive bearer has been
  minted yet). The filter receives the credential-free auth.PrincipalInfo
  (Subject = claim-mapped upstream subject; Claims = ID-token/userinfo claims,
  nil for identityFromToken / synthetic OAuth2) and must treat it as read-only.
• No silent fallback on error. A filter error fails the authorization with a
  server error rather than reverting to walking every upstream.
• Stored chain is validated on reuse. A reused ChainUpstreams must be
  non-empty, lead with the first configured upstream, and name only configured
  upstreams — so a tampered pending row cannot shrink the chain to skip legs (which
  would also disable the cross-leg identity check).
• Rolling-upgrade safety. A subsequent-leg pending written before the
  ChainUpstreams field existed is rejected (fail closed, forcing a fresh
  authorization) rather than recomputed against the wrong leg's context, preserving
  the "filter runs once, on the first leg" guarantee.
• Non-goals preserved. SingleLeg flows are unaffected and the
  upstream-token storage key contracts are unchanged.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.50000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.68%. Comparing base (fb69e00) to head (4dffa43).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/upstream/oidc.go	33.33%	3 Missing and 1 partial ⚠️
pkg/authserver/server/handlers/callback.go	92.10%	2 Missing and 1 partial ⚠️
pkg/authserver/server/handlers/handler.go	96.36%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5725      +/-   ##
==========================================
+ Coverage   70.65%   70.68%   +0.03%     
==========================================
  Files         682      682              
  Lines       68854    68949      +95     
==========================================
+ Hits        48651    48739      +88     
- Misses      16657    16663       +6     
- Partials     3546     3547       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Multi-Agent Consensus Review

Agents consulted: security, architecture-go, storage-persistence, test-coverage, general-quality

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Legacy-pending recompute can use the wrong leg's request context for the filter	9/10	MEDIUM	Fix
2	Stored ChainUpstreams is trusted without validation against configured upstreams	7/10	MEDIUM	Fix
3	Identity-mismatch log loses structured fields after the verifyChainIdentity extraction	7/10	MEDIUM	Fix
4	Missing backward-compatibility test for legacy Redis pending records lacking chain_upstreams	7/10	MEDIUM	Fix

Overall

This adds an optional UpstreamFilter hook, injected via WithUpstreamFilter, that narrows a multi-upstream OAuth authorization chain to a subset after the first upstream resolves. The implementation matches the linked issue's proposed shape closely, computes the effective chain once and carries it forward in PendingAuthorization.ChainUpstreams rather than re-invoking the filter per leg, and fails closed (server error, no fallback to a full walk) on filter error. computeChain's narrowing logic is small, pure, and has thorough table-driven coverage of every scenario called out in the linked issue. For existing no-filter deployments, behavior is unchanged end to end.

The findings below are all MEDIUM and none block merging. The most notable one (#1) is a narrow edge case in the "empty ChainUpstreams" recompute branch: it can't distinguish a genuine first leg from a non-first leg whose pending predates this field (a realistic rolling-deploy scenario for the Redis backend), so a context-sensitive filter could see the wrong leg's request context during that window. The other three are smaller and self-contained: a defense-in-depth gap where a chain loaded from storage isn't cross-checked against the configured upstream list, a logging regression where the identity-mismatch check lost its structured slog fields during extraction into verifyChainIdentity, and a test-coverage gap for the legacy-Redis-record backward-compatibility case (the repo already has a precedent for exactly this kind of test at redis_test.go:1109).

Documentation

UpstreamFilter's doc comment (pkg/authserver/server/handlers/handler.go) states it receives "the request context of the first leg's callback" — worth adding a note there about the rolling-upgrade edge case in finding #1, and mentioning that the context it receives already carries auth.WithPlatformUser, since that's the most concrete signal a real filter implementation would key off of.

---

Generated with Claude Code

[jhrozek]

Went through this carefully, including tracing the storage / replay / cross-leg interleave angles. Overall it's solid: session isolation holds via the fresh per-/authorize sessionID, the filter can only narrow (never reorder or extend the chain), every failure path fails closed with token cleanup, and ChainUpstreams is cloned on all four storage crossings. Build and the authserver package tests pass locally.

Approving. A few non-blocking things:

• validateChain doesn't fully enforce what its doc comment claims — the one substantive item, left inline.
• NewHandler (pkg/authserver/server/handlers/handler.go) doesn't reject duplicate upstream names, even though NamedUpstream's doc says they must be unique. upstreamByName returns first-match and tokens key by name, and this PR now keys the chain itself by name, so the invariant is newly load-bearing. Worth a dedup check in the existing validation loop so it fails loudly at construction. (Pre-existing, so noting it here rather than inline since that block isn't in the diff.)
• Two doc clarifications inline (the filter Claims contract, and the verifyChainIdentity scope wording).

[tgrunnagle]

Re: the duplicate upstream-name observation in your review body — addressed in 3dd8352. NewHandler now rejects duplicate upstream names in its existing validation loop (fails loudly at construction), since the uniqueness invariant is now load-bearing: the chain is keyed by name, as are token storage and upstreamByName. Added TestNewHandler_ErrorsOnDuplicateUpstreamNames.

[github-actions]

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 transformation

Alternative:

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.

[tgrunnagle]

Added a Large PR Justification section to the description per the large-PR gate. In short: non-test code is ~371 lines (within the 400 guideline); the remaining ~849 lines are reviewer-requested test coverage for a cohesive feature. This review should auto-dismiss now that the section is present.

Large PR justification has been provided. Thank you!

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.