fix(auth): validate OAuth state cookie to prevent CSRF attacks#171
Open
MehtabSandhu11 wants to merge 1 commit into
Open
fix(auth): validate OAuth state cookie to prevent CSRF attacks#171MehtabSandhu11 wants to merge 1 commit into
MehtabSandhu11 wants to merge 1 commit into
Conversation
Collaborator
|
@ShantKhatri You can do the final review looks good to me. |
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
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.
Summary
This PR fixes a broken CSRF protection vulnerability in the OAuth flow. The server was
generating a
stateparameter during the GitHub and Google OAuth redirects but nevervalidating it in the callback handlers — making the CSRF protection completely
non-functional. The fix stores the generated state in a short-lived
httpOnlycookiebefore the redirect, then verifies it matches the returned state in both callback
handlers before processing the token exchange. If the state is missing, expired, or
mismatched, the server immediately returns a
400 Bad Requestand halts the flow.Closes #147
Type of Change
What Changed
apps/backend/src/routes/auth.ts—/githubroute: stores the generatedstateina short-lived
httpOnlycookie (maxAge: 600) before redirecting to GitHub.apps/backend/src/routes/auth.ts—/github/callbackroute: readsstatefrom queryand
oauth_statefrom cookies, returns400on mismatch/missing, clears the cookieon success.
apps/backend/src/routes/auth.ts—/googleroute: same cookie storage logic asthe GitHub redirect.
apps/backend/src/routes/auth.ts—/google/callbackroute: same CSRF validationlogic as the GitHub callback.
state?.startsWith('mobile_')) is fully preserved — validation happensbefore the mobile check so nothing in that path is broken.
How to Test
the login completes successfully and you are redirected to the dashboard.
statevalue and theoauth_statecookie, then manually call the callback endpoint with a different ormissing
statequery parameter — confirm the server returns400with"Invalid or missing OAuth state".confirm the server returns
400because the cookie was cleared after first use.state=mobile_xxx) — confirm the login stillcompletes and redirects to the mobile URI correctly.
Checklist
console.logor debug statements left in the code.Additional Context
The
oauth_statecookie is intentionally plainhttpOnly+sameSite: laxrather thansigned, since
@fastify/cookiesigning is not currently configured project-wide. This isstill secure:
httpOnlyprevents JS access,sameSite: laxblocks cross-originrequests from attaching the cookie, and the 10-minute
maxAgelimits the exposurewindow. If the project adopts a cookie secret in future, the cookie can be upgraded to
signed with a one-line change. The
(request.cookies as any)cast is a minor typeworkaround since
FastifyRequestdoesn't include.cookiesin its default type withoutexplicit
@fastify/cookietype augmentation — functional at runtime, can be cleaned upif type declarations are added later.