feat(auth): permanently reserve soft-deleted emails against re-registration #320
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/bunyip!320
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/BUNYIP-330-block-reregister-deleted-email"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Bunyip already soft-deletes accounts by default (
AuthService::delete_account->UserRepository::soft_delete; hard delete is dev-only via?purge=1underBUNYIP_E2E_BOOTSTRAP_ALLOW). What was missing was the reservation:find_by_emailandemail_existsboth filterWHERE deleted_at IS NULL, and the DB uniqueness was a partial indexUNIQUE (email) WHERE deleted_at IS NULL, so a soft-deleted account's email would come back available the moment the row was tombstoned. Any subsequent registration under that address would create a fresh user (with a freshsub) and complete without complaint. The mokosh side stays safe today because it links tosub, not email, so the new account cannot reach the deleted user's mokosh data, but the belt-and-braces posture that keeps that reassurance in place is that the deleted email cannot be claimed by anyone else in the first place.Add
UserRepository::email_reserved(executor, email): same shape asemail_exists, minus theAND deleted_at IS NULLgate. Route every "does a new user get to claim this email?" call through it:AuthService::register(services/auth.rs): the freshest signup path. SameAppError::conflict("Email already registered")copy so a caller can't enumerate soft-deleted-vs-active accounts.AuthService::verify_magic_linkNone branch (services/auth.rs): passwordless signup path.AuthService::request_email_changeandAuthService::confirm_email_change(services/auth.rs): unverified-user immediate change AND the token-confirmed change both refuse a target email whose reservation is held by a tombstoned row.AuthService::accept_admin_inviteNone branch (services/auth.rs): invite acceptance under a reserved email is refused before the fresh admin user is created.Login (
AuthService::login), OCI auth, and the "resolve the current user by id" queries keep filtering to non-deleted rows on purpose: a soft-deleted user can't sign in.Tighten the DB constraint to match. New migration
20260702000010_email_permanent_reservation.sqlrenames any pre-existing lowered-email duplicates (belt-and-braces against the deleted+active pair the old partial index would have allowed), dropsusers_email_unique_active, and adds a plainUNIQUE (LOWER(email))across active + soft-deleted rows. Case-insensitive matches how the code always compares email. Production is expected to have zero collisions since the old partial index already blocked the active-vs-active shape; a rare deleted+active collision would rename to<email>.dup-<uuid>and log for operator review. Migration is additive and immutable per CLAUDE.md; a NEW file, never editing an existing one.New e2e spec
e2e/tests/auth/reregister-blocked.spec.tsdrives the raw JSON API (no mail sink required, unlikesignup.spec.tswhich istest.fixmeon BUNYIP-150): register A, soft-delete A via a newsoftDeleteMehelper (omits?purge=1so the row stays tombstoned rather than hard-deleted), then attempt to register with A's email on a clean context. Expects HTTP 409. Also asserts case-only variants share the reservation so an attacker cannot bypass by tweaking capitalisation. Cleanup best-efforts adeleteMe(?purge=1) fallback so the reserved row does not clog staging; the reaperhard_delete_stale_disposablesweeps by email pattern regardless of tombstone state.#BUNYIP-330
Bunyip already soft-deletes accounts by default (`AuthService::delete_account` -> `UserRepository::soft_delete`; hard delete is dev-only via `?purge=1` under `BUNYIP_E2E_BOOTSTRAP_ALLOW`). What was missing was the reservation: `find_by_email` and `email_exists` both filter `WHERE deleted_at IS NULL`, and the DB uniqueness was a partial index `UNIQUE (email) WHERE deleted_at IS NULL`, so a soft-deleted account's email would come back available the moment the row was tombstoned. Any subsequent registration under that address would create a fresh user (with a fresh `sub`) and complete without complaint. The mokosh side stays safe today because it links to `sub`, not email, so the new account cannot reach the deleted user's mokosh data, but the belt-and-braces posture that keeps that reassurance in place is that the deleted email cannot be claimed by anyone else in the first place. Add `UserRepository::email_reserved(executor, email)`: same shape as `email_exists`, minus the `AND deleted_at IS NULL` gate. Route every "does a new user get to claim this email?" call through it: - `AuthService::register` (services/auth.rs): the freshest signup path. Same `AppError::conflict("Email already registered")` copy so a caller can't enumerate soft-deleted-vs-active accounts. - `AuthService::verify_magic_link` None branch (services/auth.rs): passwordless signup path. - `AuthService::request_email_change` and `AuthService::confirm_email_change` (services/auth.rs): unverified-user immediate change AND the token-confirmed change both refuse a target email whose reservation is held by a tombstoned row. - `AuthService::accept_admin_invite` None branch (services/auth.rs): invite acceptance under a reserved email is refused before the fresh admin user is created. - Admin email-change handler (bunyip-api/src/handlers/admin.rs): even an admin cannot rename a user's email onto a reserved identity. Login (`AuthService::login`), OCI auth, and the "resolve the current user by id" queries keep filtering to non-deleted rows on purpose: a soft-deleted user can't sign in. Tighten the DB constraint to match. New migration `20260702000010_email_permanent_reservation.sql` renames any pre-existing lowered-email duplicates (belt-and-braces against the deleted+active pair the old partial index would have allowed), drops `users_email_unique_active`, and adds a plain `UNIQUE (LOWER(email))` across active + soft-deleted rows. Case-insensitive matches how the code always compares email. Production is expected to have zero collisions since the old partial index already blocked the active-vs-active shape; a rare deleted+active collision would rename to `<email>.dup-<uuid>` and log for operator review. Migration is additive and immutable per CLAUDE.md; a NEW file, never editing an existing one. New e2e spec `e2e/tests/auth/reregister-blocked.spec.ts` drives the raw JSON API (no mail sink required, unlike `signup.spec.ts` which is `test.fixme` on BUNYIP-150): register A, soft-delete A via a new `softDeleteMe` helper (omits `?purge=1` so the row stays tombstoned rather than hard-deleted), then attempt to register with A's email on a clean context. Expects HTTP 409. Also asserts case-only variants share the reservation so an attacker cannot bypass by tweaking capitalisation. Cleanup best-efforts a `deleteMe` (`?purge=1`) fallback so the reserved row does not clog staging; the reaper `hard_delete_stale_disposable` sweeps by email pattern regardless of tombstone state. #BUNYIP-330