fix(auth): validate mobile OAuth redirect URI against scheme allowlist#390
Open
Ridanshi wants to merge 1 commit into
Open
fix(auth): validate mobile OAuth redirect URI against scheme allowlist#390Ridanshi wants to merge 1 commit into
Ridanshi wants to merge 1 commit into
Conversation
…mbedding in OAuth state
The GitHub and Google OAuth initiation endpoints accepted an arbitrary
mobile_redirect_uri query parameter and embedded it — base64url-encoded —
into the OAuth state string without any validation. At callback time
getMobileRedirectUri() decoded it and the handler used it verbatim as the
redirect destination for a freshly-issued 30-day JWT:
return reply.redirect(`${mobileRedirect}#token=${token}`);
An attacker who crafts a link such as
/auth/github?state=mobile_x&mobile_redirect_uri=https://attacker.com/steal
and convinces a victim to click it receives the victim's JWT via the
Location header fragment. The existing CSRF cookie check does not prevent
this because the attacker initiates a legitimate flow — they do not forge a
request on behalf of the victim.
Fix:
- Add ALLOWED_MOBILE_SCHEMES constant in authService.ts listing the only
permitted custom schemes: devcard:// (production) and exp:// (Expo Go
local development), matching the scheme registered in apps/mobile/app.json
and the MOBILE_REDIRECT_URI example in .env.example.
- Export isSafeMobileRedirectUri() for use in both buildOAuthState() and
getMobileRedirectUri() so validation is enforced at embed time and again
at decode time (defence in depth).
- buildOAuthState() now silently drops an unsafe URI rather than embedding
it; the callback falls back to the server-configured MOBILE_REDIRECT_URI.
- getMobileRedirectUri() re-validates the decoded URI on the way out so that
a tampered state string constructed outside buildOAuthState() cannot slip
a forbidden https:// URI past the initial check.
Add 19 unit tests in authService.test.ts covering:
- isSafeMobileRedirectUri: allowed schemes, forbidden schemes (https, http,
javascript:, embedded-scheme-in-path), empty string
- buildOAuthState: safe URI embedded correctly, unsafe URI dropped, empty
mobile URI, uniqueness of nonce across calls
- getMobileRedirectUri: non-mobile state, safe round-trip, tampered state
with https:// URI rejected, malformed base64 rejected, exp:// round-trip
Harxhit
added
the
gssoc:approved
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
2 participants
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.
Closes #185
Problem
The GitHub and Google OAuth initiation endpoints accept a
mobile_redirect_uriquery parameter and embed it into the OAuth state without validation. During the callback flow, the value is decoded and used as the redirect destination for the user's JWT.This allows an attacker to supply an arbitrary redirect URI and receive a victim's authentication token after a successful OAuth login flow.
Example:
In this scenario, a victim who completes authentication could be redirected to an attacker-controlled destination containing their JWT.
Root Cause
mobile_redirect_uriwas accepted directly from user-controlled input and embedded into the OAuth state. The callback flow later trusted and used the decoded value without validating whether it was a legitimate mobile application redirect target.Solution
Introduce explicit allowlist validation for mobile redirect URIs.
Allowed schemes:
devcard://exp://The validation is applied in two places for defense in depth:
If a supplied URI does not match the allowlist, it is ignored and the flow falls back to
MOBILE_REDIRECT_URI.Changes
isSafeMobileRedirectUri()helper.Tests
Added 19 unit tests covering:
http,https,javascript, malformed values)All existing and new tests pass successfully.
Impact
Low risk.
The change only affects untrusted redirect URIs supplied through
mobile_redirect_uri. Existing valid mobile authentication flows continue to function normally, while unsafe redirect destinations are rejected.Security Benefit
Prevents authentication token leakage through attacker-controlled redirect destinations and ensures OAuth tokens are only delivered to trusted mobile application schemes.