Skip to content

fix(auth): bind OAuth state to the initiating browser - #31

Closed
albertovincenzi wants to merge 1 commit into
fix/oauth-next-open-redirectfrom
fix/bind-oauth-state
Closed

fix(auth): bind OAuth state to the initiating browser#31
albertovincenzi wants to merge 1 commit into
fix/oauth-next-open-redirectfrom
fix/bind-oauth-state

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

The OAuth state JWT was signed, but it was not tied to the browser that initiated the login. Signing prevents mutation; it does not prevent an attacker from initiating and completing their own Google login, then sending that valid callback URL to a victim. Loading it would replace the victim’s Gate session with the attacker’s identity (login CSRF / session swapping).

Fix

  • set the exact signed state in a five-minute, HttpOnly, SameSite=Lax cookie scoped to the callback path
  • require an exact cookie/query-state match before exchanging the authorization code
  • add Secure on HTTPS deployments
  • delete the transaction cookie after a successful callback
  • reuse one cookie parser for session and OAuth cookies
  • add regression tests for missing/mismatched state and cookie attributes

Notes for Alice

This PR is intentionally stacked on #15 because both touch the OAuth state flow; review/merge #15 first. SameSite=Lax still allows Google’s top-level GET redirect, while the host-only callback-scoped cookie prevents a callback prepared in another browser from being accepted. No provider configuration changes are required.

Verification

  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings

@alice-viola

Copy link
Copy Markdown
Contributor

Landed on master via #67 (merge commit 944ee9b) as part of the 62-PR integration — this PR's head commit a9915c1 is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/oauth-next-open-redirect, not master. Closing as landed.

@alice-viola alice-viola closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants