feat(auth): derive the OAuth redirect_uri from the request host - #73
Merged
Conversation
The app answers on more than one hostname -- moshcoding.com and pit.moshcode.sh both route to this service -- but redirect_uri was pinned to APP_BASE_URL, so CoinPay login only worked on moshcoding.com. Starting login from pit.moshcode.sh was not merely landing on the wrong host, it could not succeed at all. /auth/login sets cp_pkce and cp_state as host-only cookies on pit.moshcode.sh, then the IdP sends the browser to moshcoding.com, where those cookies are absent -- so the callback failed "state mismatch" before reaching the token exchange. Both the authorize step and the callback now resolve the origin through lib/oauth-origin, so the redirect_uri matches byte for byte at token exchange, and the post-login redirect returns to the host the user started from, where the session cookie actually applies. The Host header is attacker-controlled, so it is allowlisted before it can become a redirect_uri: APP_BASE_URL's own host always, plus anything in OAUTH_ALLOWED_HOSTS. An unlisted host falls back to APP_BASE_URL rather than being reflected, which keeps an authorization code from being sent to a spoofed host. The IdP's own redirect_uri registration is a second line of defense here, not the first. OAUTH_REDIRECT_URI still pins a single URI for anyone who wants the old behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes "Log in with CoinPay" work on
pit.moshcode.sh, not justmoshcoding.com.The bug
redirect_uriwas pinned toAPP_BASE_URL(lib/oauth.ts:7), so starting login onpit.moshcode.shproduced:This isn't just "lands on the wrong host" — it cannot succeed at all:
/auth/loginsetscp_pkce/cp_stateas host-only cookies (path:"/", nodomain) onpit.moshcode.sh.moshcoding.com.Login failed: state mismatchbefore the token exchange.A shared cookie can't fix it:
moshcoding.comandmoshcode.share different registrable domains.The fix
New
lib/oauth-origin.ts. Authorize and callback both resolve the origin the same way, so theredirect_urimatches byte for byte at token exchange (OAuth requires this), and the post-login redirect returns to the host the user started on — where the session cookie actually applies.authorizeUrl()andexchangeCode()take an optionalredirectUri, defaulting to the old constant, so nothing else changes behaviour.Security
The
Hostheader is attacker-controlled. Reflecting it into aredirect_uriunchecked would hand the authorization code to whatever host an attacker names, so a host is allowlisted before it can become one:APP_BASE_URL's own host — alwaysOAUTH_ALLOWED_HOSTS(comma-separated)APP_BASE_URLrather than being reflectedThe IdP's own
redirect_uriregistration is a second line of defense here, not the first.x-forwarded-hostis preferred overhost(the platform edge sets it; barehostcan be the internal upstream) but is allowlisted just the same.Covered by tests: suffix attacks (
evilpit.moshcode.sh,pit.moshcode.sh.evil.example), appended ports, comma-joined header spoofs in both positions, case folding, empty allowlist entries, and a malformedAPP_BASE_URL.Verification
bun test tests/→ 62 pass, 0 fail (16 new)tsc --noEmit→ cleanbun run build→ cleanDeploy steps (not done by this PR)
https://pit.moshcode.sh/auth/coinpay/callbackas a second redirect URI on CoinPay clientcp_f246043e2ae8a463673d5ece.OAUTH_ALLOWED_HOSTS=pit.moshcode.shon themoshcoding.comRailway service.Until both are done, pit falls back to the current behaviour — no regression.
🤖 Generated with Claude Code