Skip to content

fix(auth): cascade DELETE /auth/me to oauth_account + CORS on 500s - #45

Merged
amrtgaber merged 1 commit into
mainfrom
fix/account-deletion-cascade-and-cors-on-500
May 27, 2026
Merged

fix(auth): cascade DELETE /auth/me to oauth_account + CORS on 500s#45
amrtgaber merged 1 commit into
mainfrom
fix/account-deletion-cascade-and-cors-on-500

Conversation

@amrtgaber

Copy link
Copy Markdown
Contributor

Summary

  • DELETE /auth/me returns 500 for any user with a linked OAuth account. Adding cascade="all, delete" to User.oauth_accounts makes the ORM delete children first instead of trying to NULL the FK.
  • Unhandled-exception 500s ship without CORS headers, so browsers misreport them as "No Access-Control-Allow-Origin header" instead of surfacing the real failure. A new Exception handler re-derives the CORS headers on the response itself.

Root causes

Cascade. User.oauth_accounts had no cascade= set, so SQLAlchemy's default (save-update, merge) fired on session.delete(user). The ORM emitted UPDATE oauth_account SET user_id = NULL ... as a "disassociate children" step before the parent delete; the NOT NULL constraint on oauth_account.user_id rejected it with NotNullViolationError, surfaced as a 500.

Verified in Cloud Run logs for a real failing request:

sqlalchemy.exc.IntegrityError: ...NotNullViolationError:
null value in column "user_id" of relation "oauth_account" violates not-null constraint

Postgres' ON DELETE CASCADE on the FK would handle this cleanly if the ORM stayed out of the way, but the ORM steps in first. Two ways to fix it: cascade="all, delete" (ORM emits explicit DELETEs for children) or passive_deletes=True (ORM yields to the DB cascade). I went with just the first — passive_deletes=True would silently break the SQLite test harness, which defaults to foreign_keys=OFF. The slight perf cost (one DELETE per linked oauth_account row) is negligible for an account-deletion endpoint.

CORS on 500. Starlette's build_middleware_stack splits exception handlers into two buckets:

for key, value in self.exception_handlers.items():
    if key in (500, Exception):
        error_handler = value           # → ServerErrorMiddleware (outermost)
    else:
        exception_handlers[key] = value # → ExceptionMiddleware (innermost)

ServerErrorMiddleware is the absolute outermost layer of any Starlette app — Starlette wires it that way deliberately. The 500 it emits travels through the outer ASGI send, never the wrapped send that CORSMiddleware uses to inject its headers. Result: every unhandled exception comes back to the browser without CORS headers, and the real failure (server-side bug) is hidden behind a generic CORS complaint.

The fix re-derives the CORS headers (matching cors_origin_list / cors_origin_regex from app/config.py) inside the exception handler and attaches them to the response. Same logic CORSMiddleware runs, just emitted at the response level instead of the middleware level.

Tests

tests/test_delete_account.py covers both:

  • test_delete_me_succeeds_for_user_with_linked_oauth — seeds a Steam-OAuth user + linked oauth_account row, hits DELETE /auth/me, asserts 204 and that both rows are gone. Reproduces the original 500 if the cascade is removed.
  • test_unhandled_exception_response_includes_cors_headers — forces an uncaught RuntimeError from a dependency, asserts the 500 carries Access-Control-Allow-Origin and Access-Control-Allow-Credentials. Uses ASGITransport(raise_app_exceptions=False) because Starlette's ServerErrorMiddleware re-raises after sending the response (a debug-tooling convenience that httpx propagates by default).

Test plan

  • CI passes
  • On a deployed test environment, sign in as a Steam-OAuth user and DELETE /auth/me from the auth frontend — expect 204 and the toast clears
  • Force a 5xx (e.g. point at a broken DB once) and verify the browser surfaces "Internal Server Error", not a CORS message

`DELETE /auth/me` 500s for any user with linked OAuth accounts.
SQLAlchemy's default ORM cascade ("save-update, merge") tries to
disassociate the children with `UPDATE oauth_account SET user_id =
NULL` before deleting the parent, which the `NOT NULL` on
`oauth_account.user_id` rejects. Adding `cascade="all, delete"` to
the `User.oauth_accounts` relationship makes the ORM emit DELETE
statements for the children first, so the parent delete succeeds.
Not using `passive_deletes=True` deliberately: it would skip the
ORM-side DELETEs and rely on the DB cascade, which is true in
prod (Postgres) but false in the SQLite test harness without
`PRAGMA foreign_keys=ON`, and would mask regressions.

The bug surfaced as a CORS error in the browser ("No
'Access-Control-Allow-Origin' header is present on the requested
resource") rather than a server error — because Starlette routes
`Exception` and `500` handlers to `ServerErrorMiddleware`, which
is hardcoded outside all user middleware and emits the 500 via the
outer ASGI `send`, bypassing `CORSMiddleware`. Registering an
`Exception` handler that re-derives the CORS headers and attaches
them to the response itself keeps the diagnostic info intact for
any future 500.

Both issues covered by regression tests in `test_delete_account.py`.
@amrtgaber
amrtgaber merged commit 267c8f5 into main May 27, 2026
2 checks passed
@amrtgaber
amrtgaber deleted the fix/account-deletion-cascade-and-cors-on-500 branch May 27, 2026 17:48
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