Accept localhost dynamic-port loopback redirect_uris

[jhrozek]

Summary

• Why: native MCP clients registered through DCR (VS Code, Claude Code) register a portless loopback redirect_uri like http://localhost/callback, then listen on an ephemeral port. RFC 8252 §7.3 requires the authorization server to allow any port for loopback redirects, but fosite's matcher recognises only IP literals (127.0.0.1, [::1]) — localhost is compared by exact string equality, so http://localhost:54321/callback is rejected and the flow can never complete.
• What: /authorize rewrites the form's redirect_uri to the client's registered portless literal so fosite's exact-match validation accepts it, then restores the requested dynamic-port URI for the pending authorization. The code stays bound to the exact requested port, so the /token check is unchanged.
• Error paths keep the client's real listener. A small wrapper hands fosite's error writer the dynamic-port URI and widens its redirect_uri validity check to cover the localhost loopback case it cannot recognise. The wrapper only ever widens that answer, never narrows it — so a client whose redirect_uri fosite would have accepted unaided still gets a proper error redirect instead of a bare JSON body.
• Matching is stricter than fosite's: escaped paths compared literally (an encoded separator can't impersonate a real one), a bare ? is significant, fragments and userinfo rejected. Exact registered matches take precedence over dynamic-port matches, so a client that pinned a port is never rewritten to a different registered entry. Dynamic-port matching is restricted to public clients.
• IP-literal loopback clients are untouched — fosite already matches those natively on both success and error paths.
• Net −42 lines of production code: a single-method interface with two implementations collapsed into one free function, and a wrapper type that no longer carried any behaviour deleted along with the branch that built it.

Fixes #6189

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

New and updated coverage:

• End-to-end through /authorize → callback → /token asserting the dynamic port survives all three legs, and that a different port and a different path are both rejected at token exchange.
• Error redirects on both /authorize (post-redirect_uri validation failure) and callback (upstream IdP denies consent) land on the client's real dynamic port. Both assert the full host:port prefix — a substring check would have passed before the fix too.
• The validity override: nil client, genuine dynamic-port match, unregistered path, and a confidential client whose exact-match redirect_uri must stay valid (this last one fails if the override ever narrows again).
• Exact-match precedence in both registration orders, so the test pins order-independence rather than list luck.
• Loopback matcher hardening: encoded path separators, ForceQuery, fragment and userinfo rejection, case-insensitive LOCALHOST, confidential-client rejection.
• A public client round-tripped through Redis still gets dynamic-port matching despite being reconstructed as a different concrete type.

Changes

File	Change
handlers/authorize.go	redirect_uri rewrite; loopbackAuthorizeRequester wrapper for error redirects; warn on a failed client lookup
handlers/callback.go	error requester carries the real dynamic-port URI from the pending authorization
registration/client.go	RegisteredLoopbackRedirectURI free function replacing a single-method interface; exact-match precedence; escaped-path/ForceQuery/fragment/userinfo hardening; behaviourless LoopbackClient deleted
storage/redis.go	per-backend matcher method dropped — the free function works on any fosite.Client
storage/cimd_decorator.go	dropped the branch (and helper) that existed only to build the deleted wrapper type
server/doc.go	package doc corrected for the removed type

Does this introduce a user-facing change?

Yes. Native OAuth clients that register a portless localhost loopback redirect_uri and listen on an ephemeral port can now complete the authorization flow against the embedded auth server. Previously these requests were rejected with redirect_uri mismatch. OAuth errors now also reach such a client's real listener rather than being delivered to whatever holds port 80 (or not delivered at all).

Special notes for reviewers

• Why the form rewrite rather than a fosite hook: fosite exposes no client-side extension point for loopback matching — MatchRedirectURIWithClientRedirectURIs never consults a Client's own matcher, and its loopback exception is IP-literal-only. Rewriting the form into its exact-match branch is the only seam available without forking fosite.
• The wrapper's widen-only invariant is load-bearing. An earlier revision let the override consult only the public-clients-only loopback matcher, which would have degraded error redirects to a JSON body for any confidential client. Not reachable today (both registrars mint public clients) but it is pinned by a test now.
• Known, accepted side effect: the wrapper doesn't satisfy fosite's G11NContext, so error messages fall back to language.English. Harmless here — no MessageCatalog is configured — but it is a real difference from an unwrapped requester.
• Pre-existing, not addressed here (worth its own issue): a client with exactly one registered redirect_uri that omits redirect_uri at /authorize gets invalid_grant at /token, because fosite's defaulted value is recorded into the pending authorization and then demanded back. Present on main; only reachable for non-OIDC scope sets, since fosite rejects an omitted redirect_uri whenever openid is requested.
• Separately worth a look: IP-literal loopback clients still go through fosite's own matcher, which compares decoded paths and ignores userinfo — so a client registering http://127.0.0.1/cb%2Fchild can have a code delivered to /cb/child. Mandatory S256 PKCE bounds the impact, and closing it means either extending the rewrite to IP literals (losing fosite's correct error-path port handling) or a reject-only pre-check. Out of scope here.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 84.50704% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.44%. Comparing base (3d44435) to head (0bd9c5c).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/server/handlers/authorize.go	77.27%	7 Missing and 3 partials ⚠️
pkg/authserver/server/registration/client.go	96.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6215      +/-   ##
==========================================
- Coverage   72.63%   72.44%   -0.19%     
==========================================
  Files         736      739       +3     
  Lines       76355    76748     +393     
==========================================
+ Hits        55463    55603     +140     
- Misses      16955    17179     +224     
- Partials     3937     3966      +29     

☔ 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.