feat: add e-mail change with confirmation and undo
Page at /email asks for the new address and the current password (shared failure counter with the password page, five requests per hour) and mails a one-hour confirmation link to the new address. Confirming writes the address and mails the old address a 24-hour undo link; undoing restores it, ends every session of the user and notifies the Admin (ADMIN_NOTIFY_EMAIL). Link pages show on GET and apply on POST; tokens are stored only as hashes, single use, and are blanked from every log line. New table owned by profile, German text, tests; review findings logged in the plan. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FocPJ75XPma7TcuYEMy8zm
This commit is contained in:
Ebbe Baß
co-authored by
Claude Sonnet 5.5
Claude Sonnet 5.5
parent
3fd8f27127
commit
94c1d39c9d
+131
@@ -0,0 +1,131 @@
|
||||
---
|
||||
title: 'E-mail change with confirmation and undo'
|
||||
type: 'feature'
|
||||
ticket: '3'
|
||||
epic: 'epic-account-self-service'
|
||||
created: '2026-10-19'
|
||||
status: 'built'
|
||||
baseline_revision: '3fd8f27127e2ef6ba2b800245fdee17901659106'
|
||||
route: 'full'
|
||||
route_source: 'auto'
|
||||
risk: 'high'
|
||||
review: 'quick'
|
||||
review_source: 'pinned'
|
||||
lenses_ran: ['quick']
|
||||
review_loop_iteration: 0
|
||||
context:
|
||||
- '{project-root}/_bmad-output/architecture/architecture-ping-mee-self-service-platform-2026-10-15/ARCHITECTURE-SPINE.md'
|
||||
- '{project-root}/_bmad-output/ux-designs/ux-ping-mee-self-service-platform-2026-10-15/EXPERIENCE.md'
|
||||
---
|
||||
|
||||
<frozen-after-approval reason="human-owned intent — do not modify unless human renegotiates">
|
||||
|
||||
## Intent
|
||||
|
||||
**Problem:** The e-mail address in AD is where announcements and security notices go, yet a user cannot change it, and an address taken over by someone else would go unnoticed. An address change must prove the new mailbox, and the old one must be able to reverse it.
|
||||
|
||||
**Approach:** A page `/email` in the `profile` app asks for the new address and the current password and mails a one-hour confirmation link to the new address; AD is unchanged until that link is confirmed. Confirming writes `mail`, then mails the old address a notice with a 24-hour undo link. Undoing restores the old address, ends every session of the user, tells the user to change their password and notifies the Admin. Both links open a page that names the change; only a POST with a CSRF token applies it. Pending changes are stored in a new table owned by `profile`, with tokens only as hashes.
|
||||
|
||||
## Boundaries & Constraints
|
||||
|
||||
**Always:** AD-2, AD-3, AD-4, AD-7, AD-8, AD-9, AD-11, AD-17, AD-19 apply. The request form accepts no username; the acting user comes from the session. The order on the request is fixed: validate the form locally, `throttle.check` before any AD or mail call, `verify_password`, then the write of the pending row and the queued mail in one transaction. A wrong current password counts on the same counter as the password page (`password-change`), so both pages together stay below AD's lockout threshold. Queueing a request is also limited to 5 per hour per user. Tokens are `secrets.token_urlsafe(32)`, stored as SHA-256 hashes, single-use, valid 1 hour (confirmation) and 24 hours (undo). A GET never changes anything and never consumes a token. Confirming and undoing each run the AD write, the token use, the audit entry and the queued mail in one transaction; a failed AD call rolls back and writes a failure entry after the rollback. A new request replaces an earlier pending one of the same user. Undo applies only while AD's `mail` still equals the address that was confirmed. Passwords, tokens and addresses are never logged and never audited; the audit holds only what happened. Link pages work whether or not the visitor is signed in. A user with no valid address in AD may still change it: no undo notice is queued and the `email.changed` entry records `notice: skipped`. The Admin notice goes to the optional environment variable `ADMIN_NOTIFY_EMAIL`; a value that is set but not an address stops startup with a clear message (AD-13); when it is empty no mail is queued, an error-level log line is written with the user key only, and the undo still succeeds. Every new string has German formal text, informal where it differs, in `portal/locale`. Mails are German formal, carry a plain-text part and never contain a password.
|
||||
|
||||
**Never:** No change to `mail` before the confirmation POST. No change to another user, no username in a URL or form. No check that the address is unused by another account. No password stored. No JavaScript requirement: every page works as a plain POST.
|
||||
|
||||
## I/O & Edge-Case Matrix
|
||||
|
||||
| Scenario | Input / State | Expected Output / Behavior | Error Handling |
|
||||
|----------|--------------|---------------------------|----------------|
|
||||
| Open the page | Signed-in user | Current address shown, fields for the new address and the current password; reached from the profile page | Directory down: "not possible now" (503) |
|
||||
| Request | Valid new address, correct current password | Pending row with hashed token; confirmation mail to the new address; page says a link was sent; AD unchanged; `email.change_requested` audited | n/a |
|
||||
| Wrong current password | Password does not match | "The current password is not correct." Nothing sent; counts toward the shared limit; failure audited | No AD change, no mail |
|
||||
| Limit reached | Failed checks at the effective limit, or 5 requests within an hour | 429 "Too many attempts. Please wait a few minutes."; no AD call, no mail; throttled attempt audited | Recovers after the pause |
|
||||
| Bad new address | Empty, not an address, over 254 characters, or equal to the current one (any letter case) | Error under the field; nothing sent; not counted | n/a |
|
||||
| Newer request | A second request while one is pending | The first link stops working; only the newest confirms | n/a |
|
||||
| Confirmation page | GET with a valid, unused, unexpired token | Page names old and new address and shows a button; nothing changes | n/a |
|
||||
| Confirm | POST with CSRF on that page | AD `mail` is the new address; token used; undo token created; notice with undo link mailed to the old address; `email.changed` audited; page confirms | Directory down: 503, token stays unused, failure audited |
|
||||
| Dead link | Token unknown, used, expired or replaced | "This link is not valid any more." (410); nothing changes; the page does not say which case | n/a |
|
||||
| Undo page | GET with a valid undo token | Page names the change and shows a button; nothing changes | n/a |
|
||||
| Undo | POST with CSRF | AD `mail` is the old address again; every session of the user ended; token used; Admin notified; `email.undone` audited; page says to change the password | Directory down: 503, token stays unused |
|
||||
| No current address | The user's `mail` in AD is empty or invalid | Request and confirmation work; no undo notice is queued; `email.changed` audited with `notice: skipped`; the page does not mention a notice | n/a |
|
||||
| No Admin address | `ADMIN_NOTIFY_EMAIL` empty | Undo succeeds; no Admin mail; an error-level log line names the user key | n/a |
|
||||
| Undo after another change | AD `mail` no longer equals the confirmed address | Page says the address was changed again since; nothing changes | n/a |
|
||||
| Not signed in, wrong method, no CSRF token | `/email` without session; GET to a POST-only action; POST without token | Redirect to sign-in; 405; 403; nothing changes | n/a |
|
||||
|
||||
</frozen-after-approval>
|
||||
|
||||
## Code Map
|
||||
|
||||
- `portal/profile/models.py`, `portal/profile/migrations/0001_initial.py` -- new (the app has no tables yet): `PendingEmailChange` (user key, kind `confirm` or `undo`, token hash unique, old and new address, created, expires, used). Owned by `profile` (AD-19).
|
||||
- `portal/profile/services.py` -- add `request_email_change`, `confirm_email_change`, `undo_email_change`, token helpers; reuse `ENDPOINT = "password-change"` and `failure_limit` / `pause_seconds` for the shared counter, `_record_failure` style for failure entries, `end_sessions`, `mailer.enqueue`, `mailer.is_valid_address`, `audit.record`, `audit.record_failure`, `directory.update_attributes(sam, {"mail": ...})`.
|
||||
- `portal/profile/forms.py`, `views.py`, `urls.py` -- `EmailChangeForm`; views `email` (login required), `email_confirm`, `email_undo` (token in the path, no login, GET shows, POST applies); routes `/email`, `/email/confirm/<token>`, `/email/undo/<token>`.
|
||||
- `portal/templates/profile/email*.html`, `portal/templates/mail/email_change_confirm/`, `email_changed_notice/`, `email_undone_admin/` (subject and body) -- pages and mails.
|
||||
- `portal/templates/profile/details.html` -- the profile page shows the address read-only today; add the link "Change e-mail address".
|
||||
- `portal/config/env.py`, `settings.py`, `.env.example` -- the optional `ADMIN_NOTIFY_EMAIL` (read in `env.py` with the other settings, checked with `mailer.is_valid_address` rules, exposed in `settings`); `portal/tests/test_config.py` gets a case for an invalid value. Link base: `settings.OIDC.public_base_url`.
|
||||
- `portal/config/settings.py` -- `SECURE_REFERRER_POLICY = "same-origin"` already keeps the token out of cross-site requests; keep it.
|
||||
- `portal/locale/de`, `de_DU`, `portal/tests/test_translations.py` -- German text and its coverage test.
|
||||
- Do not change: `accounts.throttle`, the directory adapter interface, the outbox.
|
||||
|
||||
## Tasks & Acceptance
|
||||
|
||||
**Execution:**
|
||||
- [ ] `portal/profile/models.py`, migration -- `PendingEmailChange` -- AD-9, AD-19
|
||||
- [ ] `portal/profile/services.py` -- request, confirm, undo with the order and transactions above -- AD-4, AD-7, AD-8, AD-9
|
||||
- [ ] forms, views, urls, templates, mail templates, profile page link -- AD-17
|
||||
- [ ] `ADMIN_NOTIFY_EMAIL` in config, `.env.example` and its test; German text for every new string
|
||||
- [ ] `portal/tests/test_email_change.py` -- every matrix row with the fake directory and fake mailer, plus: only hashes stored, a GET changes nothing and consumes nothing, a replaced or used or expired token is dead, undo ends only that user's sessions, the shared counter with the password page, no password, token or address in logs or audit, `makemigrations --check` clean
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Given a signed-in user, when they request a change with the correct password, then AD `mail` is unchanged and one confirmation mail with a link exists for the new address.
|
||||
- Given that link, when it is opened with GET, then the page names the change and nothing changes; when confirmed with POST, then AD holds the new address and the old address is mailed an undo link.
|
||||
- Given the undo link, when confirmed with POST within 24 hours, then the old address is restored, every session of the user ends, and the Admin notification is queued.
|
||||
- Given any token, then the database holds only its hash, and it works once.
|
||||
- Given a failure after the AD call inside the transaction, then the error is logged at error level with the change details (AD-4), without addresses or tokens, and the user is told the outcome is unclear.
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
- Built by a subagent from this plan, checked against the diff. `pytest portal/tests` 472 passed, 6 skipped; `ruff check` clean; `makemigrations --check` clean. The done checkpoint against the test OU and mailbox is not run.
|
||||
- Beyond the plan, kept after reading the diff:
|
||||
- `RedactLinkTokens` log filter and `redact()` in the request log path: the request log and Django's 500 log printed the URL, token included.
|
||||
- Undo also deletes the user's unused confirmation request, so a hijacker cannot request a change, wait for the undo and confirm it later.
|
||||
- A correct password on the request resets the `password-change` counter, as AD forgets failures after a good bind.
|
||||
- Undo restores the address AD held at confirmation time; the 400 and 429 pages on `/email` do not read AD (they omit the current-address box); extra pages: "address changed again" (409) and the unclear outcome (500).
|
||||
- `is_valid_address` moved to `portal/mailer/address.py` because `env.py` needs it before Django loads; `mailer.is_valid_address` still works. `PendingEmailChange` has a `sam` column for the exact AD name.
|
||||
- Not covered: expired and used rows stay in the table until a token-expiry job exists (AD-12).
|
||||
|
||||
## Plan Change Log
|
||||
|
||||
## Review Triage Log
|
||||
|
||||
Pass 1 (quick lens): 0 high, 1 medium patched, 8 rejected (7 low, 2 false counted below).
|
||||
|
||||
| Finding | Verdict | Route | Evidence |
|
||||
|---|---|---|---|
|
||||
| A POST to a link without a CSRF token logs the token on `django.security.csrf`; the redaction filter is only on `django.request` | medium | patch | Django's `CsrfViewMiddleware._reject` logs `request.path`; that logger has no filter and propagates to the root stdout handler. Put the filter on the handler so every logger is covered; test it through the configured handler. |
|
||||
| The undo compares AD `mail` before the transaction, not inside it | low | rejected | AD is not transactional, so a re-check inside only narrows the window to a few milliseconds; a concurrent edit by the same user in that window is not met in use. |
|
||||
| An earlier undo link works again after a later change is undone (A to B, B to C, undo C) | false | rejected | The earlier link still undoes the A to B change, the address is B again, the link is the user's own, single-use and expires in 24 hours; that is its meaning. |
|
||||
| The confirmation page names the address at request time while the notice goes to the address read at confirm time | low | rejected | Differs only if someone edits `mail` between request and confirm; the page still names the change correctly. |
|
||||
| The same-address check runs after the password check, so it costs a bind | false | rejected | It cannot be known locally, the plan's order puts the password check before any other directory read, and it is not counted. |
|
||||
| The 5-per-hour counter is hit after commit; parallel requests can pass the check first | low | rejected | The same check-then-hit design of the throttle is already in deferred-work.md (AD-7); the user needs a session and their password to send each mail. |
|
||||
| `throttle.reset` after a correct password weakens the shared counter | false | rejected | The password page resets on success too and AD forgets failures after a good bind, so the counter follows AD; it is listed in the Implementation Notes. |
|
||||
| HEAD on the link pages returns 405 | low | rejected | A scanner's HEAD changes nothing; 405 is harmless. |
|
||||
| A sign-in between two `end_sessions` calls in undo is not covered | low | rejected | Needs the password change and a sign-in inside milliseconds; the next undo or the password change ends it. |
|
||||
|
||||
## Design Notes
|
||||
|
||||
**Why the shared counter.** The password page limits wrong current passwords to AD's lockout threshold minus one. A second page with its own counter would double the wrong binds AD sees. Both pages use the `password-change` counter; the five-per-hour request limit is separate, under its own endpoint name.
|
||||
|
||||
**Confirming does not need a sign-in.** The link proves the mailbox, as the undo link does; the user may read the mail on a phone. A mail scanner opening the link only does a GET, which changes nothing.
|
||||
|
||||
**Undo order.** Undo restores the address first, then ends the sessions, so a rolled-back write ends nothing. The Admin notice carries the username and time, never a password or token.
|
||||
|
||||
## Verification
|
||||
|
||||
**Commands:**
|
||||
- `.venv/Scripts/python -m pytest portal/tests -q` -- expected: all tests pass, including the new ones
|
||||
- `.venv/Scripts/python -m ruff check portal manage.py docker` -- expected: no findings
|
||||
- `.venv/Scripts/python manage.py makemigrations --check --dry-run` -- expected: no changes detected
|
||||
|
||||
**Manual checks (done checkpoint, on the Linux host against the test OU and the test mailbox):**
|
||||
- Request a change as `ptest-user1`, confirm that `mail` in AD is unchanged, open the link and see that it changes nothing, confirm, and see `mail` change and the notice arrive at the old address.
|
||||
- Open the undo link, undo, and see `mail` restored, a second browser signed out, and the Admin notice arrive.
|
||||
Reference in New Issue
Block a user