fix(security): validate OAuth state parameter in callback flow - #240
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
🔍 Cora AI Code Review✅ No issues found. Code looks good! Review powered by cora-code · BYOK · MIT |
ajianaz
force-pushed
the
fix/oauth-state-validation
branch
from
September 2, 2026 04:18
3b3545f to
87196d5
Compare
Adds a one-time, session-bound OAuth state token: POST /api/oauth/state issues a 256-bit random token (migration 015, oauth_states table, 10-minute TTL), the dashboard appends it to the Threads authorize URL, and POST /api/oauth/exchange now requires it — bound-state consumption for the web flow, shallow single-use validation for legacy direct-credential clients. The callback page rejects stateless callbacks instead of proceeding (login CSRF vector). docs/auth-flow.md documents the state flow.
…ositives No behavior change: the flagged lines generate/read a random one-time state value (hex::encode of random bytes, URL query param), not hardcoded secrets. Renamed local variables (token -> state_value, stateToken -> stateValue) so the crypto/hardcoded-secret pattern no longer misfires.
ajianaz
force-pushed
the
fix/oauth-state-validation
branch
from
September 2, 2026 05:09
51f36b5 to
d884b7f
Compare
| headers: axum::http::HeaderMap, | ||
| Json(input): Json<OAuthExchangeRequest>, | ||
| ) -> (StatusCode, Json<serde_json::Value>) { | ||
| // #237: CSRF protection — the code must have been initiated by this |
| ) | ||
| })?; | ||
| if ok { | ||
| // Consume so the same token can't be reused by another legacy call. |
Two findings from the CodeCora scan on PR #240: 1. oauth.rs: empty-string app_id/app_secret no longer count as 'credentials supplied'. A request with app_id="" previously took the unbound shallow path while still exchanging with the server's stored credentials, bypassing the per-caller state binding. Non-empty check now required on both fields to use the legacy path. 2. oauth_state.rs: validate_state_shallow now consumes the state atomically via consume_any_oauth_state as the authority (exists-then-ignored-delete allowed two concurrent exchanges to both succeed). The DELETE result gates acceptance; exactly one concurrent caller wins. Doc comment updated.
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.
What
Implements CSRF protection for the Threads OAuth flow via a one-time
statetoken:POST /api/oauth/state(authenticated) — issues a 256-bit random token, stored in a newoauth_statestable (migration 015) bound to the caller's API key/session, TTL 10 minutes, single use.POST /api/oauth/exchange— now requiresstate:DELETE ... WHERE token = ? AND bound_key = ? AND expires_at >= ?) — replay and cross-session reuse are impossible.app_id/app_secretin request, used by older clients): any valid, unexpired, unconsumed state is accepted and consumed (still single-use, still unguessable).&state=to the authorize URL (both server-built and client-fallback URLs).docs/auth-flow.mddocuments the state flow.Why
Cora scan v0.9.0 finding (MAJOR): "OAuth callback does not validate the state parameter" — without it, an attacker can craft a callback URL with their own authorization code and drive the victim's logged-in session to connect the attacker's Threads account (login CSRF). Related finding: "OAuth flow documented without state parameter or PKCE".
Testing
cargo test --workspace— 215 passed, 0 failedcargo fmt --check+cargo clippy --workspace --all-targets -- -D warningscleanbun run buildclean (web)IF NOT EXISTS, "already exists"-tolerant registration)