Skip to content

feat: dedicated verify-before-apply email-change flow - #115

Open
felixgateru wants to merge 7 commits into
mainfrom
feat/verified-email-change-flow
Open

feat: dedicated verify-before-apply email-change flow#115
felixgateru wants to merge 7 commits into
mainfrom
feat/verified-email-change-flow

Conversation

@felixgateru

Copy link
Copy Markdown

Summary

Workstream A of #110: a dedicated verify-before-apply email-change flow for global human identities, kept deliberately separate from the generic self-profile path (#109), which excludes email precisely because it is a login/recovery/OAuth-linking identifier, not display metadata.

  • POST /auth/email/change/request — real-session-only (no access token, scoped or unscoped), rejects a session older than ATOM_EMAIL_CHANGE_MAX_SESSION_AGE_SECS (default 900s, the deliberate stand-in for step-up reauthentication since Atom has no dedicated mechanism for it). Mutates nothing; only mints a single-use atomc_-prefixed token bound to the entity, the email captured as current at request time, and the proposed email. Enumeration-resistant: an already-taken proposed email returns the same 202 without minting a token or sending mail.
  • POST /auth/email/change/confirm — unauthenticated (the token, provable only by receipt at the proposed mailbox, is the credential); deliberately does not require the requesting session to still be alive, since every session gets revoked on success anyway and the token may legitimately be opened on a different device. Locks the entity then the canonical entity_emails row (same order as sync_entity_email_from_attrs_in_tx), fails safely — without consuming the token — if the live email drifted since the request, explicitly rechecks case-insensitive uniqueness (independent of the case-sensitive DB index, which only achieves case-insensitivity because every writer normalizes first), then atomically updates the canonical email, the active password credential's identifier, and — only if already present — the attributes.email compatibility mirror; invalidates stale verification/reset tokens; revokes every session; and notifies the old address after commit with no token/link in the message.
  • Publishes the existing frozen entity.update domain event rather than inventing a new event name — domain-event-catalog.json's compatibility rule freezes the event-name set for v1, and a new optional field detail key is within that rule.

A real concurrency fix along the way

identity::service::upsert_oauth_identity's auto-link-by-email lookup only locked the entities row it read (FOR UPDATE OF e), not the joined entity_emails row. That meant it could commit an OAuth link keyed to an email a concurrent write was moving away — not just against this new flow, but against the existing admin sync_entity_email_from_attrs_in_tx path too. Now locks both (FOR UPDATE OF e, ee); a new test proves the lookup actually blocks on a concurrent entity_emails lock holder.

Contract/tooling changes

  • New migration 002_email_change_tokens.sql — the first migration ever added since the v1.0.0 launch squash. scripts/check-v1-contracts.sh's migration check previously required the migrations/ directory to contain exactly the one frozen baseline file, which would have permanently blocked any future migration. Changed it to verify the pinned baseline is present and unmodified, without constraining what else exists — the baseline itself (migrations/001_initial.sql) stays byte-frozen.
  • apidocs/openapi.yaml gains the two new paths and their request schemas; api/v1/deployment-config.json gains the 3 new env vars (and its own cross-checked count in models/enums.rs); contracts-v1.0.0.sha384 hashes recomputed for both.
  • AGENTS.md documents the flow's invariants under Self-Authorization.

Test plan

  • cargo fmt --check, cargo clippy -- -D warnings (only pre-existing, unrelated warnings remain — confirmed identical on a clean checkout of main)
  • cargo test --lib — 338 passed, including the deployment-surface contract cross-check
  • cargo test --test api_contract — 13 passed
  • bash scripts/check-v1-contracts.sh — all contracts and the migration baseline validate
  • cargo test --test m52_email_change -- --ignored against local Postgres — 14/14 passed, covering: request leaves current login untouched; access tokens (scoped and unscoped) rejected; stale session rejected; enumeration resistance including a case-variant probe; same-email rejection and request supersession; wrong/expired/replayed/superseded tokens; atomic canonical-email + credential-identifier + attributes-mirror update; a never-present mirror staying untouched; case-variant collision recheck at confirmation; old-login-fails/new-login-succeeds; stale verification/reset token invalidation; safe (non-consuming) handling of a drifted current email; a lock-contention proof for the OAuth fix; single-connection-pool coverage
  • Broader identity/bootstrap suites re-run for regressions (m5, m15, m21, m23, m25, m26, m51) — 75 passed, no regressions

Closes part of #110 (workstream A only; the legacy-identity audit report and remediation tooling remain separate PRs per the issue's delivery order).

…ability so it matches the migration-seeded database contract

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
…ady hit the undeclared api_endpoint applicability error

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
… verified email-change flow

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
…confirm flow

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
…contracts

Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
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