Skip to content

fix(auth): require a same-origin POST to delete an account (H1 #3906522) - #2222

Open
Gr1dlock wants to merge 1 commit into
mainfrom
Gr1dlock/brasilia-v1
Open

fix(auth): require a same-origin POST to delete an account (H1 #3906522)#2222
Gr1dlock wants to merge 1 commit into
mainfrom
Gr1dlock/brasilia-v1

Conversation

@Gr1dlock

@Gr1dlock Gr1dlock commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

PR Type

  • Regular Task
  • Bug Fix
  • QA Tests

Description

/api/auth/delete-account was mounted as GET (app/api/auth/delete-account/route.ts) and gated solely on the presence of an Auth0 session cookie before calling the irreversible managementClient.users.delete(); that cookie is SameSite=Lax (SDK v4 default at abstract-session-store.js:14lib/auth0.ts sets no session.cookie override), and Lax cookies ride cross-site top-level navigations, so any attacker page could destroy a logged-in developer's Auth0 identity with a single link, window.open or meta-refresh (HackerOne #3906522) — the legitimate UI flow was itself a top-level GET navigation, so the attack was just the real flow triggered from someone else's page. proxy.ts:370 returns authRes for all of /api/auth/* before any guard and no edge control fronts the host, so the handler is the only place this can be stopped.

The route is now POST-only (Lax never sends the cookie on a cross-site POST; GET returns 405, which also closes the reporter's first-party /api/auth/login?returnTo=%2Fapi%2Fauth%2Fdelete-account delivery variant since the post-login redirect lands as a GET) and is gated by a new api/helpers/csrf.ts guard that requires Sec-Fetch-Site: same-origin, falls back to an Origin allow-list, and fails closed when neither header is present — the guard is the layer that survives a future change to the cookie's SameSite attribute. It returns 204 instead of the old 307 to /api/auth/logout, because the caller is now a fetch() that would otherwise replay the POST against logout; both dialogs share a new deleteAuth0Identity() helper and drive the logout navigation themselves, so a failed Auth0 deletion surfaces as an error toast instead of rendering a raw JSON 500 at the user.

Blast radius of the original bug was bounded — only the Auth0 leg is deleted, the Hasura row survives (it is removed by a separate client-side mutation the attacker cannot reach) and login-callback re-links World ID / email-OTP users on next login, so permanent loss was confined to password-connection users plus everyone's MFA/passkey enrollments and user_metadata.

Verified against a running dev server, not just tests:

request before after
GET + Sec-Fetch-Site: cross-site (the exploit) 307, identity deleted 405
POST + Sec-Fetch-Site: cross-site 403 cross_origin_request
POST + Origin: https://evil.example 403
POST with no origin metadata 403 (fail-closed)
POST + Sec-Fetch-Site: same-origin 401 (reaches the auth gate; flow intact)
POST + Origin: <APP_BASE_URL> 401 (Origin fallback works)

Not addressed here, flagged rather than silently scoped in: the client deletes the Hasura row before the Auth0 identity, so a failure of the second call still leaves a partial state — this PR makes that failure visible (error toast) rather than making the pair atomic — and no step-up re-authentication was added, since that is a product decision and not required to close the CSRF.

Checklist

  • I have self-reviewed this PR.
  • I have left comments in the code for clarity.
  • I have added necessary unit tests.
  • I have updated the documentation as needed.

🤖 Generated with Claude Code

`/api/auth/delete-account` was mounted as GET and gated solely on the presence
of an Auth0 session cookie. That cookie is `SameSite=Lax` (SDK default; no
`session.cookie` override in `lib/auth0.ts`), so the browser attaches it to
cross-site top-level navigations — any attacker page could irreversibly delete
a logged-in developer's Auth0 identity with a single `<a href>`, `window.open`
or meta-refresh. `proxy.ts` passes `/api/auth/*` through untouched and no edge
control fronts the host, so the handler is the only place this can be stopped.

- Mount the route on POST only. `Lax` never sends the cookie on a cross-site
  POST, and GET now returns 405 — which also closes the reporter's first-party
  `/api/auth/login?returnTo=/api/auth/delete-account` delivery variant, since
  the post-login redirect lands as a GET.
- Add `api/helpers/csrf.ts`: reject anything the browser does not report as
  same-origin, falling back to an `Origin` allow-list check and failing closed
  when neither header is present. This is the layer that survives a future
  change to the cookie's `SameSite` attribute.
- Return 204 instead of a 307 to `/api/auth/logout`: the caller is now a
  `fetch()`, and a 307 would make it replay the POST against logout. The two
  dialogs share `deleteAuth0Identity()` and drive the logout navigation
  themselves, so a failed Auth0 deletion now surfaces as an error toast
  instead of rendering a raw JSON 500.

Co-Authored-By: Claude <noreply@anthropic.com>
@Gr1dlock

Gr1dlock commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: e8d377171b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

1 participant