fix(auth): add OAuth state parameter to prevent login CSRF (#268)

[Shevilll]

Fixes #268

What

Adds the OAuth state parameter (RFC 6749 §10.12) to the GitHub verification flow: a cryptographically-random state is generated when the OAuth URL is created, bound to the verification session, and validated in the callback before any account linking happens.

Why

The flow previously bound the Discord verification request to the GitHub callback using only a session id in the redirect URI, and never generated or validated a state. That leaves it open to login CSRF / session fixation — a victim could be tricked into completing a callback that links their Discord account to an attacker's GitHub account. login_with_github already accepted an optional state, but no caller passed it and the callback never checked it.

Changes

• verification.py — create_verification_session() now generates a random state via secrets.token_urlsafe(32), stores it alongside the existing in-memory verification session (same ~5-min TTL), and returns (session_id, oauth_state). New validate_oauth_state() uses a constant-time comparison (secrets.compare_digest) and rejects a missing state, an unknown/expired session, or a mismatch.
• cogs.py — passes the generated state to login_with_github(...) in both the /verify_github command and the onboarding flow.
• auth.py — the callback now accepts the state query param and rejects the request when validation fails, before linking accounts.

Design note

The issue suggested Redis for state storage, but this repo has no application-level Redis (it isn't a declared dependency and isn't used outside the vendored FalkorDB subproject). The established convention for verification sessions is the in-memory store in verification.py with a short TTL, so I bound the state there to keep the change minimal and consistent with the existing code. Migrating session/state storage to Redis would be a separate, broader change.

Verification

• python -m py_compile passes for all changed files.
• Logic-tested validate_oauth_state: correct state accepted; forged, empty, and None states rejected; unknown session rejected.
• No new dependencies; the diff is limited to the CSRF fix.
• Not run: full end-to-end OAuth roundtrip (requires live Supabase/Discord infra).

Summary by CodeRabbit

• Bug Fixes
  • Enhanced OAuth state validation in GitHub account verification flow to ensure secure session management during authentication.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR closes a Login CSRF vulnerability in the GitHub OAuth flow by introducing a cryptographically random oauth_state token. create_verification_session now returns (session_id, oauth_state), storing the state in the in-memory session map. Discord commands pass the state to login_with_github; the callback endpoint validates it via a new validate_oauth_state function before processing the code exchange.

Changes

OAuth CSRF State Protection

Layer / File(s)	Summary
Session tuple expansion and state validation service backend/app/services/auth/verification.py	_verification_sessions values expand from a 2-tuple to a 3-tuple adding oauth_state. create_verification_session generates the state via secrets.token_urlsafe, stores it, and returns (session_id, oauth_state). New validate_oauth_state function performs constant-time comparison against the stored value. find_user_by_session_and_verify and get_verification_session_info updated to unpack the new tuple shape.
Auth callback state validation backend/app/api/v1/auth.py	auth_callback gains state: Optional[str] = Query(None) parameter. Calls validate_oauth_state(session, state) immediately; on failure logs an error and returns an HTML error response before the code exchange. Import block reformatted to include validate_oauth_state.
Discord cog OAuth URL wiring backend/integrations/discord/cogs.py	/verify_github command and the onboarding OAuth path both unpack (session_id, oauth_state) from create_verification_session and forward oauth_state to login_with_github via the state= argument.

Sequence Diagram(s)

sequenceDiagram
  participant User as Discord User
  participant Cog as Discord Cog
  participant VerSvc as verification.py
  participant GitHub as GitHub OAuth
  participant Callback as auth_callback

  User->>Cog: /verify_github
  Cog->>VerSvc: create_verification_session(discord_id)
  VerSvc-->>Cog: (session_id, oauth_state)
  Cog->>GitHub: login_with_github(redirect_to=callback_url, state=oauth_state)
  GitHub-->>User: OAuth authorization URL
  User->>GitHub: authorize
  GitHub->>Callback: GET /callback?code=...&state=oauth_state&session=session_id
  Callback->>VerSvc: validate_oauth_state(session_id, state)
  VerSvc-->>Callback: True / False
  alt state valid
    Callback->>Callback: proceed with code exchange
  else state invalid
    Callback-->>User: HTML error response
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A sneaky CSRF lurked in the flow,
But the rabbit brought secrets to steal the show.
A token is born, stored safe in a tuple,
The callback now checks — no attacker can fool.
🐇 State validated, the OAuth stands tall!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title accurately summarizes the main security fix: adding OAuth state parameter to prevent login CSRF attacks.
Linked Issues check	✅ Passed	PR successfully implements all core requirements: generates cryptographically secure state token, stores it with verification session, passes it to GitHub, and validates it in callback.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing OAuth state parameter validation across verification, cogs, and auth files as required by issue #268.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

backend/app/services/auth/verification.py (1)

114-114: 💤 Low value

Prefix unused variable with underscore for consistency.

expiry_time is unpacked but never used (expiry is validated via the database query). Use _expiry_time to match the pattern used elsewhere in this file (e.g., lines 22, 89, 200).

-        discord_id, expiry_time, _oauth_state = session_data
+        discord_id, _expiry_time, _oauth_state = session_data

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/auth/verification.py` at line 114, The variable
expiry_time is unpacked from session_data in the statement but is never used in
the code since expiry validation happens via database query. Rename expiry_time
to _expiry_time to follow the Python convention of prefixing unused variables
with an underscore, which is consistent with the pattern already used elsewhere
in the file such as with _oauth_state in the same unpacking statement.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@backend/app/services/auth/verification.py`:
- Line 114: The variable expiry_time is unpacked from session_data in the
statement but is never used in the code since expiry validation happens via
database query. Rename expiry_time to _expiry_time to follow the Python
convention of prefixing unused variables with an underscore, which is consistent
with the pattern already used elsewhere in the file such as with _oauth_state in
the same unpacking statement.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 392216ae-3919-4fa8-9260-c1fb00098e19

📥 Commits

Reviewing files that changed from the base of the PR and between db81871 and 1959d85.

📒 Files selected for processing (3)

• backend/app/api/v1/auth.py
• backend/app/services/auth/verification.py
• backend/integrations/discord/cogs.py