Skip to content

fix(connect): validate OAuth state nonce server-side to prevent CSRF (#223)#558

Open
Ridanshi wants to merge 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/oauth-csrf-state-nonce
Open

fix(connect): validate OAuth state nonce server-side to prevent CSRF (#223)#558
Ridanshi wants to merge 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/oauth-csrf-state-nonce

Conversation

@Ridanshi

@Ridanshi Ridanshi commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #223.

The GitHub connect flow (/api/connect/github) accepted userId directly from the unverified state callback parameter and used Math.random() to generate the state value. An attacker could forge a callback with a crafted state, causing the server to associate the victim's account with an attacker-controlled GitHub credential.

The login flow (/auth/github, /auth/google) is addressed separately via the cookie-based double-submit CSRF fix already present on main (#481).

Changes

apps/backend/src/routes/connect.ts

  • Generate a 32-byte cryptographically secure nonce on /connect/github initiation
  • Store { userId } in Redis under oauth:connect-nonce:<nonce> with a 10-minute TTL
  • Pass only the nonce as the state parameter — userId never travels through the callback URL
  • On callback: validate nonce format (64 lowercase hex chars), look up Redis, reject unknown or expired entries, fail closed if Redis is unavailable, delete nonce immediately after first use (replay protection)
  • Replace Math.random() with randomBytes(32)
  • Use dedicated github_follow platform key to prevent auth-token overwrites
  • Fail closed — no Redis bypass path

apps/backend/src/__tests__/connect.test.ts

  • 14 tests covering the full callback security surface

apps/backend/src/__tests__/oauth-scope.test.ts

  • Updated buildConnectApp and makeConnectState to match new nonce-only state format

Security properties

Property Before After
State entropy ~44 bits (Math.random) 256 bits (randomBytes(32))
userId source in callback Trusted from state param Looked up from Redis by nonce
CSRF protection None Server-side nonce validation
Replay protection None Nonce deleted on first use
Nonce expiry None 10 minutes (Redis TTL)
Redis unavailable N/A Fails closed (rejects request)

Tests (connect.test.ts — 14 tests)

  • Valid connect flow end-to-end with github_follow platform assertion
  • Nonce stored in Redis with correct userId and TTL
  • State is 64 lowercase hex chars
  • Missing code parameter
  • Missing state parameter
  • Malformed state (too short)
  • Malformed state (non-hex characters)
  • Forged base64-JSON state rejected as malformed
  • Unknown nonce (not in Redis)
  • Expired nonce
  • Forged nonce (not in Redis)
  • Replay attack — second callback with same nonce is rejected
  • Corrupt Redis data
  • GitHub token exchange failure

Test plan

  • npx vitest run src/__tests__/connect.test.ts src/__tests__/oauth-scope.test.ts src/__tests__/follow.test.ts src/__tests__/profiles.test.ts — 52/52 pass
  • Pre-existing failures in event.test.ts and analytics.test.ts are unrelated to this change (upstream test infrastructure issue with request.user not being set by the jwtVerify mock)

@vercel

vercel Bot commented Jun 12, 2026

Copy link
Copy Markdown

@Ridanshi is attempting to deploy a commit to the Prashantkumar Khatri's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown

Hi @Ridanshi,

Thanks for opening this pull request.

This PR has been automatically classified based on the files modified.

Applied Labels

  • backend

Primary Review Area

  • backend

Reviewer

@Harxhit has been identified as the primary reviewer for this pull request.

If you have any questions regarding the affected area or implementation details, feel free to reach out to the assigned reviewer.

Thank you for your contribution!

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

CI — Checks Failed

Backend — FAIL

Check Result
Lint FAIL
Test PASS
Typecheck PASS

Mobile — SKIP

Check Result
Lint -
Test -

Web — SKIP

Check Result
Check -
Build -

Last updated: Fri, 12 Jun 2026 20:34:06 GMT

Ridanshi added 2 commits June 13, 2026 00:37
…loses Dev-Card#223)

Replace client-side-only state validation in the GitHub connect flow with
server-side nonce verification backed by Redis.

- Generate a 32-byte cryptographically secure nonce on connect initiation
- Store {userId} in Redis under oauth:connect-nonce:<nonce> with a 10-minute TTL
- State parameter carries only the nonce; userId is never trusted from the callback
- On callback: validate nonce format, look up Redis, reject unknown/expired nonces
- Delete nonce immediately after first successful validation (replay protection)
- Drop Math.random()-based state generation in favour of randomBytes(32)
- Add 13 tests covering: valid flow, missing params, malformed state, unknown nonce,
  expired nonce, forged state, replay attack, corrupt Redis data, token exchange failure
…ormat

Update the connect-route test harness in oauth-scope.test.ts to match the
new state format: state now carries only a 64-char hex nonce, with userId
stored server-side in Redis. Add a Redis mock to buildConnectApp so the
callback route can look up the userId during tests.
@Ridanshi Ridanshi force-pushed the fix/oauth-csrf-state-nonce branch from 47763f7 to 544181d Compare June 12, 2026 20:33
@Ridanshi Ridanshi changed the title fix(auth): validate OAuth state nonce server-side to prevent CSRF (#223) fix(connect): validate OAuth state nonce server-side to prevent CSRF (#223) Jun 12, 2026
@Harxhit Harxhit added the gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. label Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth CSRF — State Nonce Never Validated Server-Side

2 participants