domain: password-reset job retries mint additional valid reset tokens #59

New issue

Closed

opened 2026-07-03 01:42:22 +00:00 by rosa · 1 comment

rosa commented

2026-07-03 01:42:22 +00:00

Owner

Found during a code review of the domain crate.

Location: crates/domain/src/services.rs:292 (run_password_reset)
Severity: Security / Correctness

Problem

run_password_reset inserts a password-reset confirmation, then enqueues the email as a separate call; the apalis job wrapper maps every failure to Retryable, and prior reset tokens are not deleted when a new one is minted. A mailer-enqueue failure after the insert re-runs the whole flow and mints an additional simultaneously-valid token per retry.

Failure scenario

create_password_reset_confirmation succeeds, the mailer enqueue fails transiently, the job retries, and a second (and third) valid 30-minute reset token accumulates for the same user, widening the reset window.

Suggested fix

Delete prior reset tokens before minting a new one (or make insert+enqueue transactional/idempotent), so at most one reset token is valid at a time.

Found during a code review of the `domain` crate. **Location:** `crates/domain/src/services.rs:292` (`run_password_reset`) **Severity:** Security / Correctness ## Problem `run_password_reset` inserts a password-reset confirmation, then enqueues the email as a separate call; the apalis job wrapper maps every failure to Retryable, and prior reset tokens are not deleted when a new one is minted. A mailer-enqueue failure after the insert re-runs the whole flow and mints an additional simultaneously-valid token per retry. ## Failure scenario `create_password_reset_confirmation` succeeds, the mailer enqueue fails transiently, the job retries, and a second (and third) valid 30-minute reset token accumulates for the same user, widening the reset window. ## Suggested fix Delete prior reset tokens before minting a new one (or make insert+enqueue transactional/idempotent), so at most one reset token is valid at a time.

rosa added the

needs-triage

label

2026-07-03 01:43:56 +00:00

rosa commented

2026-07-03 20:55:18 +00:00

Author

Owner

This was generated by AI during triage.

Verified during triage by code inspection: the queued reset flow inserts a confirmation, then enqueues the email as a second fallible call; the apalis handler maps every failure to a retryable error; and the confirmation insert is a plain INSERT that never clears prior password-reset tokens. A transient mailer-enqueue failure therefore re-runs the whole flow and mints an additional simultaneously-valid 30-minute token on each retry.

Agent Brief

Category: bug
Summary: Guarantee at most one valid password-reset token per user, so job retries converge instead of accumulating valid tokens

Current behavior:
run_password_reset looks up the user, inserts a new password-reset confirmation, and then enqueues the reset email. The apalis job wrapper treats every failure as retryable. Prior password-reset confirmations for the user are never removed. If the mailer enqueue fails after the insert, the retry mints another valid token — each retry widens the window of simultaneously-valid reset links.

Desired behavior:
Minting a password-reset confirmation invalidates any prior password-reset confirmations for that user, so at most one reset link is valid per user at any time. A retried job converges: however many times it runs, exactly one valid token exists afterwards, and it is the one whose link was (eventually) emailed. Confirmations for other action types (email confirmation) are untouched. The lookup-miss behavior (unknown email quietly succeeds) is unchanged.

Key interfaces:

The repository operation that creates a password-reset confirmation should atomically displace prior ones for the same user and action type — either a delete-then-insert in one transaction or an equivalent single-statement upsert keyed per user+action.
The in-memory mock repository must mirror the displacement semantics (issue #63 tracks broader mock divergence; keep this method in sync here regardless).
ConfirmationAction::PasswordReset is the discriminator — only rows with that action type are displaced.

Acceptance criteria:

Requesting a second password reset invalidates the first: the older token fails verification (not-found or equivalent), the newer one succeeds.
Running run_password_reset twice for the same user (simulating a retry) leaves exactly one valid password-reset confirmation.
A pending email-confirmation for the same user survives a password-reset request.
A repository conformance test covers the displacement behavior.
The ci mise task passes.

Out of scope:

Rate-limiting or throttling reset requests.
Changing the apalis retry policy or making the mailer enqueue transactional with the insert (displacement makes retries safe without this).
Changing the 30-minute TTL.
The reset-submission flow (issue #54 covers its error mapping).

> *This was generated by AI during triage.* Verified during triage by code inspection: the queued reset flow inserts a confirmation, then enqueues the email as a second fallible call; the apalis handler maps every failure to a retryable error; and the confirmation insert is a plain INSERT that never clears prior password-reset tokens. A transient mailer-enqueue failure therefore re-runs the whole flow and mints an additional simultaneously-valid 30-minute token on each retry. ## Agent Brief **Category:** bug **Summary:** Guarantee at most one valid password-reset token per user, so job retries converge instead of accumulating valid tokens **Current behavior:** `run_password_reset` looks up the user, inserts a new password-reset confirmation, and then enqueues the reset email. The apalis job wrapper treats every failure as retryable. Prior password-reset confirmations for the user are never removed. If the mailer enqueue fails after the insert, the retry mints another valid token — each retry widens the window of simultaneously-valid reset links. **Desired behavior:** Minting a password-reset confirmation invalidates any prior password-reset confirmations for that user, so at most one reset link is valid per user at any time. A retried job converges: however many times it runs, exactly one valid token exists afterwards, and it is the one whose link was (eventually) emailed. Confirmations for other action types (email confirmation) are untouched. The lookup-miss behavior (unknown email quietly succeeds) is unchanged. **Key interfaces:** - The repository operation that creates a password-reset confirmation should atomically displace prior ones for the same user and action type — either a delete-then-insert in one transaction or an equivalent single-statement upsert keyed per user+action. - The in-memory mock repository must mirror the displacement semantics (issue #63 tracks broader mock divergence; keep this method in sync here regardless). - `ConfirmationAction::PasswordReset` is the discriminator — only rows with that action type are displaced. **Acceptance criteria:** - [ ] Requesting a second password reset invalidates the first: the older token fails verification (not-found or equivalent), the newer one succeeds. - [ ] Running `run_password_reset` twice for the same user (simulating a retry) leaves exactly one valid password-reset confirmation. - [ ] A pending email-confirmation for the same user survives a password-reset request. - [ ] A repository conformance test covers the displacement behavior. - [ ] The `ci` mise task passes. **Out of scope:** - Rate-limiting or throttling reset requests. - Changing the apalis retry policy or making the mailer enqueue transactional with the insert (displacement makes retries safe without this). - Changing the 30-minute TTL. - The reset-submission flow (issue #54 covers its error mapping).

rosa added

bug

ready-for-agent

and removed

needs-triage

labels

2026-07-03 20:55:32 +00:00

rosa referenced this issue from a commit

2026-07-03 21:13:15 +00:00

fix: Displace prior password-reset tokens when minting a new one

rosa referenced this issue from a pull request that will close it,

2026-07-03 21:13:45 +00:00