Skip to content

fix(auth): protect logout from cross-site requests - #40

Closed
albertovincenzi wants to merge 1 commit into
fix/random-oauth-noncefrom
fix/protect-logout
Closed

fix(auth): protect logout from cross-site requests#40
albertovincenzi wants to merge 1 commit into
fix/random-oauth-noncefrom
fix/protect-logout

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

GET /api/auth/logout cleared the session cookie and every /api/auth/* path bypassed the session middleware. A third-party page could navigate a signed-in operator to that URL and log them out. Using GET for a state-changing action also made crawlers/prefetchers capable of ending a session.

Fix

  • make logout an authenticated POST
  • exempt only the Google login and callback bootstrap routes, not the whole auth prefix
  • allow viewers to end their own session without treating logout as an admin control-plane mutation
  • require a JSON null body; cross-origin HTML forms cannot produce that content type and cross-origin JS must pass a CORS preflight
  • return 204, clear the cookie, and have the console reload explicitly
  • hide the meaningless logout control in GATE_DEV_EMAIL mode and show same-origin failures in the sidebar

Review notes for Alice

This PR is stacked after #39 in the auth chain (#15#31#39 → this PR) to keep each security issue isolated. The defence is layered: the route now requires a valid SameSite session, and its JSON extractor prevents same-site sibling origins from using a plain HTML form. Google login/callback remain reachable without a session by necessity. Existing bookmarks to the old GET logout endpoint will now receive 405.

Verification

  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo fmt --check
  • npm run build

@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 6f27834 is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/random-oauth-nonce, 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