From b8e156c9f49ca40911ae2b70c5dd8539623a64f8 Mon Sep 17 00:00:00 2001 From: Mix Date: Sat, 17 Oct 2026 12:03:27 +0300 Subject: [PATCH] password-reset: a confirmed e-mail, then a link to reset the password MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agreed by the owner on 2026-10-13 ("the e-mail must be confirmed"); notifications to an unconfirmed address settled on 2026-10-17: "Без подтверждения слать". Mockups 2f5f5b2, route guard b7bbb2c, panel guard 0774da2. Migration 0013: users.email_confirmed_at ('' for every address saved before it) and user_links - the SHA-256 of a link's token, its user, purpose (confirm, reset), address, created and expiry. A changed address loses its mark (keepConfirmed, in UpdateUser and SaveUser). internal/web/auth/links.go. A link is 256 bits from crypto/rand, mailed as https://SELFPOST_HOSTNAME/... - never the request's Host. Confirmation: 24 h, sent when an address is saved (Account, or the global role's user form) and by Account's Send the link again; its page /confirm/ has a button (a GET changes nothing) and confirms only for a session of its own account. Reset: /login/reset asks for a username and answers the same for every name (the sign-in page, "If that account has a confirmed e-mail, the link is on its way"), mailing after the answer; only a confirmed address gets one, none within ten minutes of the last, five links an hour per account, five requests in 15 minutes per client address; /login/reset/ (30 min) sets the password with the link taken in the same transaction that checks the address again (store.TakeResetLink), then ends every session. Changing the e-mail on Account asks for the current password. A password set on Account or the user form removes the reset links; a restore removes every link; the stop waits for links being mailed (Server.Drain); a rejected request's log line hides a link's token (loggedPath). Reviewed for security before the commit (2026-10-17): no high; two medium and five low, all fixed here - confirming needs the account's session and the mail does not name the account (a mistyped address was a takeover); a reset request neither replaces the links already sent nor spends the account's quota inside ten minutes; take-and-set atomic; the confirmation mail sent after the answer; the stop waits for mail; a restore deletes the links; the token kept out of the log. Tests: store TestUserLinks, TestUserLinkTakenOnce, TestEmailConfirmation, TestTakeResetLink; auth TestResetSendsALinkOnlyToAConfirmedAddress, TestResetLinkSetsThePasswordOnce, TestResetLinkDiesWithTheAddress, TestResetRequestsAreLimited, TestResetRequestsAreSpaced, TestConfirmLink, TestLinkPathsWithoutAToken; handlers TestAccountEmailIsConfirmedByItsLink, TestUserFormSendsANewAddressItsLink; web TestLoggedPathHidesTokens; the view's pages and fixtures; e2e password_reset_by_e_mail runs it through the sink (and checks a link from before the restore is dead). go vet + go test (26 packages) and make e2e on the test server for this tree. Docs: guide § Account, security.md (the bullet and what ends sessions), architecture (routes), schema-migrations (row 13, users, user_links), help #account (word for word with the mockup), roadmap row done, CHANGELOG [Unreleased]. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 16 + cmd/panel/httpserver.go | 10 + cmd/panel/main.go | 7 + docs/architecture.md | 6 +- docs/guide.md | 28 +- docs/roadmap.md | 18 +- docs/schema-migrations.md | 6 +- docs/security.md | 33 +- .../store/migrations/0013_email_links.sql | 21 + internal/store/schema_test.go | 13 +- internal/store/userlinks.go | 204 +++++++++ internal/store/userlinks_test.go | 191 +++++++++ internal/store/users.go | 30 +- internal/web/auth/auth.go | 22 +- internal/web/auth/handlers.go | 2 +- internal/web/auth/links.go | 402 ++++++++++++++++++ internal/web/auth/links_test.go | 301 +++++++++++++ internal/web/handlers/handlers_account.go | 78 ++++ .../web/handlers/handlers_account_test.go | 101 ++++- internal/web/handlers/handlers_links_test.go | 56 +++ .../handlers/handlers_notifications_test.go | 4 +- internal/web/handlers/handlers_users.go | 28 +- internal/web/loggedpath_test.go | 19 + internal/web/security.go | 15 +- internal/web/view/account.go | 31 ++ internal/web/view/fixtures_test.go | 25 +- internal/web/view/help.go | 3 +- internal/web/view/pages_test.go | 10 +- internal/web/view/server.go | 22 +- internal/web/view/server_test.go | 7 +- internal/web/view/signin.go | 91 ++++ internal/web/view/templates/account.html | 10 +- .../web/view/templates/confirm_email.html | 28 ++ internal/web/view/templates/login.html | 3 +- internal/web/view/templates/login_reset.html | 17 + .../web/view/templates/login_reset_set.html | 19 + internal/web/view/templates/user.html | 4 +- internal/web/view/templates_test.go | 2 +- internal/web/view/view.go | 3 + internal/web/web.go | 15 + test/e2e/main_test.go | 4 + test/e2e/notifications_test.go | 2 + test/e2e/password_reset_test.go | 120 ++++++ 43 files changed, 1966 insertions(+), 61 deletions(-) create mode 100644 internal/store/migrations/0013_email_links.sql create mode 100644 internal/store/userlinks.go create mode 100644 internal/store/userlinks_test.go create mode 100644 internal/web/auth/links.go create mode 100644 internal/web/auth/links_test.go create mode 100644 internal/web/handlers/handlers_links_test.go create mode 100644 internal/web/loggedpath_test.go create mode 100644 internal/web/view/templates/confirm_email.html create mode 100644 internal/web/view/templates/login_reset.html create mode 100644 internal/web/view/templates/login_reset_set.html create mode 100644 test/e2e/password_reset_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 032c56d..72cdee9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Added + +- **Password reset by e-mail** (password-reset): *Forgot your password?* on + the sign-in page sends a one-time link, for 30 minutes, to the account's + e-mail - once that address has been confirmed by a link sent to it. A new + address is sent its confirmation link when it is saved (Account, or the + user form); Account shows whether it is confirmed and can send the link + again. Notifications go to the address confirmed or not. Setting a new + password from a link signs every session of the account out. Migration + 0013; schema 13. + +### Changed + +- **Changing your e-mail on Account asks for your password**, as changing + the password does: the address is where a reset link goes. + ## [2.13.1] - 2026-10-17 The documents and the panel's words checked against the code. No migration; diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index c1d66b0..b1a88cc 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -169,7 +169,9 @@ func serveHTTP(ctx context.Context, cfg config, st *store.Store) error { srv := newHTTPServer(cfg.httpAddr, srvApp.Handler()) // Shut the server down cleanly when the process is asked to stop. + shutDown := make(chan struct{}) go func() { + defer close(shutDown) <-ctx.Done() shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -180,5 +182,13 @@ func serveHTTP(ctx context.Context, cfg config, st *store.Store) error { if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { return err } + // ListenAndServe returns at the stop, before its requests have + // (net/http, Server.Shutdown); Shutdown returns once they have. The links + // they mail after their answers go on after that, and end before the + // database closes. + <-shutDown + drainCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + srvApp.Drain(drainCtx) return nil } diff --git a/cmd/panel/main.go b/cmd/panel/main.go index d41a0aa..c58f57c 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -318,6 +318,13 @@ func openData(cfg config, resync func(*store.Store) error) (st *store.Store, res } if restored { log.Printf("restore manifest accepted; regenerating mail-path maps from SQLite") + // A link taken after the backup was made would work again from it + // (security review, 2026-10-17); a link sent before a restore is + // asked for again. + if err := st.DeleteAllUserLinks(); err != nil { + _ = st.Close() + return nil, false, err + } if err := resync(st); err != nil { _ = st.Close() return nil, false, err diff --git a/docs/architecture.md b/docs/architecture.md index ab4220d..5da1378 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -821,7 +821,9 @@ building them only takes method values, so the route guard test can run it on a bare `Server` and ask which pattern answers a path: - **public** — `/healthz` (liveness), `/license` (the embedded `LICENSE`), - `/static/`, `/setup/`, `/login`, `/logout`; + `/static/`, `/setup/`, `/login`, `/logout`, and the links of + password-reset, opened from a mail signed in or not: `/login/reset`, + `/login/reset/`, `/confirm/` (`internal/web/auth/links.go`); - **authenticated** — everything else, behind `RequireAuth`: a request without a live session is redirected to `/login`. @@ -845,7 +847,7 @@ only reads (the delete confirmation at `GET …/delete` renders a form). | Inbound › Domains | `GET, POST /inbound/domains` · `/inbound/domains/{id}` · `POST …/dns-recheck`, `…/upstream`, `…/recipients` · `GET, POST …/delete` — registered only with `INBOUND_RELAY_ENABLE=true` | | Inbound › Log, Filter lists | `GET /inbound/log` · `GET, POST /inbound/filter-lists` · `POST /inbound/filter-lists/{id}/delete` — registered only with `INBOUND_RELAY_ENABLE=true` | | Server | `GET /server/health` (+ `/fragment`) · `POST /server/health/recheck`, `/server/health/reload`, `/server/health/test-message` · `GET /server/log` (+ `/fragment`) · `GET, POST /server/backup` · `POST /server/backup/import` · `/server/users`, `…/new`, `…/{uid}`, `…/{uid}/delete` · `GET, POST /server/settings` · `POST /server/settings/dns-recheck` · `GET /server/components` | -| Account | `GET /account` · `POST /account/profile`, `/account/password` | +| Account | `GET /account` · `POST /account/profile`, `/account/password`, `/account/email/confirm` | | User menu | `GET /help` · `POST /logout` (public mux) | An application is addressed under its domain, and an application id that does diff --git a/docs/guide.md b/docs/guide.md index b838730..3dc7acc 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -558,14 +558,36 @@ readings, numbered in the order of what stops mail first: ### Account **Account** (`/account`, every role) is the signed-in user's own: **Profile** -(username and e-mail) and **Password**. Saving the profile does not ask for the -password; changing the password asks for the current one and is rate-limited -like a sign-in. The e-mail is the user's own address: it is where the panel +(username and e-mail) and **Password**. Changing the username or the +notifications switch does not ask for the password; changing the e-mail does, +and so does changing the password, both on the limit of a sign-in's guesses +(ten in 15 minutes per user). The e-mail is the user's own address: it is where the panel sends [notifications](#notifications) while *Send me notifications* is ticked beside it, and what a domain's *Use my e-mail* button fills in (see [Domain-level DNS](#domain-level-dns-spf-dkim-dmarc)). Application SASL logins are separate and are not changed here. +**A confirmed e-mail and a forgotten password.** A new address - saved under +Account, or typed for a user by the global role on the user form - is sent a +link that confirms it (`https:///confirm/…`), which works +once, for 24 hours, opened in a browser signed in as that account; its page +has a button, and only the button confirms. The mail does not name the +account, so a mistyped address cannot be confirmed by whoever reads it. +Account tags the address *confirmed* or *not confirmed* and, while it is not, +offers *Send the link again* (at most five links an hour to one address). +Notifications go to the address either way. Only a confirmed address can be +sent a reset link: *Forgot your password?* on the sign-in page asks for the +username and always answers the same - whether the name exists, and whether +its address is confirmed, is not said; another link is not sent within ten +minutes of the last - and the link +(`https:///login/reset/…`) works once, for 30 minutes, and +only while the account's address is still the one it was sent to. Setting the +new password signs every session of the account out. The links name the +server by `SELFPOST_HOSTNAME`, so the panel must be served at that name. An +account without a confirmed address recovers as before: the global role sets +a new password on the user form, and the only global user from the container +(see [Users](#users)). + ### Settings **Server › Settings** (`/server/settings`, global role) holds what is true for diff --git a/docs/roadmap.md b/docs/roadmap.md index ff84834..14af5f1 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -37,7 +37,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | review-2026-08-followups | Remaining findings of the 2026-08-19 code review | closed 2026-10-14 (rows placed in [audit-2026-10-12](audit-2026-10-12.md)) | — | — | | panel-notifications | E-mail notifications about important events | shipped `2.3.0` | 10/10 | [plans/panel-notifications.md](plans/panel-notifications.md) | | delivery-log-storage | Keep a message's `mail.log` lines with its send-log row | shipped `2.2.0` | 8/8 | [plans/delivery-log-storage.md](plans/delivery-log-storage.md) | -| password-reset | Password reset by e-mail | agreed (2026-10-13) | — | — | +| password-reset | Password reset by e-mail | done 2026-10-17, unreleased | — | — | | local-time | The panel shows time, and counts its days, in a time zone other than UTC | shipped `2.13.0` | — | — | | notification-mail-recheck | The Notification mail records on Server › Settings can be checked again from where they are shown | shipped `2.9.0` | — | — | | settings-save-row | Server › Settings: the Save button no longer stands alone under a long right column | shipped `2.9.0` | — | — | @@ -459,6 +459,22 @@ to come need it - they are not the owner and should not have to ask him for a new password). Mockup-first; changing the account e-mail asks for the current password in the same cut. +**Notifications to an unconfirmed address (owner, 2026-10-17):** "Без +подтверждения слать" - they go to it confirmed or not; only a reset link +needs it confirmed. Every address saved before the release is unconfirmed, +and cutting them off would have stopped every warning at once. + +**Done (2026-10-17):** design 2f5f5b2, route guard b7bbb2c, panel guard +0774da2. Migration 0013 (`users.email_confirmed_at`, `user_links`). Links +are 256 bits, kept as SHA-256, confirmation 24 h and reset 30 min, taken +once (`DELETE … RETURNING`), built on `SELFPOST_HOSTNAME` (never `Host`); +the request answers the same for every name and mails after answering; +limits per client address and per account; a link's GET page has a button +(mail scanners open links). A reset ends every session. Changing the +e-mail on Account asks for the password. The global role's user form sends +a typed address its link. e2e `password_reset_by_e_mail` runs it through +the sink. + --- ## local-time diff --git a/docs/schema-migrations.md b/docs/schema-migrations.md index 5609475..d17213d 100644 --- a/docs/schema-migrations.md +++ b/docs/schema-migrations.md @@ -17,7 +17,7 @@ When you add or review a migration, update this file in the same change. | Field | Value | |---|---| -| Chain head | `user_version = 12` | +| Chain head | `user_version = 13` | | Files | `0001_init.sql` (the 2.0 baseline), `0002_inbound_spam_log.sql`, `0003_send_log_lines.sql`, `0004_notifications.sql`, `0005_send_log_rejected_index.sql`, `0006_sender_check.sql`, `0007_delivery_log_days.sql`, `0008_tenant_boundary.sql`, `0009_refusal_line.sql`, `0010_send_log_indexes.sql`, `0011_notify_addressees.sql`, `0012_inbound_upstream.sql` | | File mark | `PRAGMA application_id = 0x53503230` ("SP20") | | Database file | `/data/selfpost.db` (bind mount) | @@ -90,6 +90,7 @@ audit (`2.5.0`), `0009_refusal_line.sql` (`2.5.1`) and | 10 | `0010_send_log_indexes.sql` | 2.5.3 | DDL | Four indexes of `send_log`, each for a question that walked the table or sorted a domain's log (audit 2026-10-12, M2, stage 3): `idx_send_log_open`, partial, the queued and deferred rows by time (the tailer's sweep every 5 min); `idx_send_log_domain_id` and `idx_send_log_app_login`, a page of one domain's or one application's log in id order; `idx_send_log_refusal_line`, partial, whether a refusal line is already on a row. `TestSendLogQueriesUseTheirIndexes` checks the plans of these and of the other by-time and by-scope statements the store runs | | 11 | `0011_notify_addressees.sql` | 2.5.4 | DDL | `notify_addressees (key, address, told_at)`: who a subject's notification is owed to and who was handed it, so a copy that failed or was held back by the hourly ceiling goes out at a later check and a subject's end goes to those told (audit 2026-10-12, stage 4); deleted with its `notify_state` row | | 12 | `0012_inbound_upstream.sql` | 2.10.0 | DDL | `inbound_spam_log` gains `queue_id`, `upstream_status` (`''`, `queued`, `held`, `sent`, `deferred`, `bounced`; CHECK), `upstream_relay`, `upstream_reply` and `upstream_at`, and the partial index `idx_inbound_spam_log_queue_id` on rows that have a queue id: what the upstream answered about mail accepted on port 25, put on the row by the log tailer (inbound-upstream-result). Rows from before have none | +| 13 | `0013_email_links.sql` | 2.14.0 | DDL | `users.email_confirmed_at` (`''` for every address saved before it: all unconfirmed) and `user_links`, the one-time links that confirm an address and reset a password, with `idx_user_links_user` (password-reset; owner, 2026-10-13 and 2026-10-17) | **Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct for operators upgrading from older 2.x images. @@ -101,7 +102,8 @@ for operators upgrading from older 2.x images. | Table | Role | |-------|------| | `settings` | Key/value settings of the instance — what is true whoever is signed in (send-log retention). Nothing that belongs to a user | -| `users` | Panel logins. `role` is `global` or `domain`; `email` is the user's own address; `notify` is the user's own switch for notification e-mails (v4); `all_domains` / `all_inbound_domains` widen a `domain` user to every domain of that direction | +| `users` | Panel logins. `role` is `global` or `domain`; `email` is the user's own address; `notify` is the user's own switch for notification e-mails (v4); `all_domains` / `all_inbound_domains` widen a `domain` user to every domain of that direction; `email_confirmed_at` is when `email` was confirmed, `''` while it is not, cleared when it changes (v13) | +| `user_links` | The one-time links of password-reset (v13): the SHA-256 of the token, the user, `purpose` (`confirm`, `reset`; CHECK), the address it was sent to, when, and until when; one live link per user and purpose | | `sessions` | Panel login sessions (token hash, owning user by id and deleted with them, sliding idle expiry) | | `domains` | Sending (outbound) domains. `dmarc_rua` is where aggregate reports go (`''` = no reports) | | `user_domains` | Outbound domains assigned to a `domain` user | diff --git a/docs/security.md b/docs/security.md index c2f0249..cfe31bc 100644 --- a/docs/security.md +++ b/docs/security.md @@ -138,7 +138,8 @@ The panel is exposed to the internet — the items below are **not optional**. after it was found, so the cookie of a deleted user is dead and cannot become the session of whoever is created under the same name later; renaming a user leaves their sessions alone. -- **What ends a user's sessions:** deleting the user (all); a password change on +- **What ends a user's sessions:** deleting the user (all); a password set + from a reset link (all, `2.14.0`); a password change on Account (that user's *other* sessions — never another user's); a password set for them by a global user on the user form (all of theirs, except the current one when they set their own); signing out (the current one only); signing @@ -148,6 +149,36 @@ The panel is exposed to the internet — the items below are **not optional**. held. - Polling by the monitoring screens (`HX-Request` `GET`s) does not renew a session. +- **Password reset by e-mail (`2.14.0`).** A link goes only to an address + its account has confirmed, by a link sent to that address; notifications + go to an unconfirmed one (owner, 2026-10-17) - they carry no secret. A link + is 256 random bits (`crypto/rand`); `user_links` keeps its SHA-256 only, + so the database or a backup opens nothing. A confirmation link lives 24 + hours, a reset link 30 minutes; each is taken once, in the transaction + that deletes it (`DELETE … RETURNING`), and a newer link of the same kind + replaces the older. A reset link holds only while the account's address is + still the confirmed one it was sent to; a confirmation link only while the + address is the one it confirms. The link's host is `SELFPOST_HOSTNAME`, + never the request's `Host`, which an attacker would choose to receive the + token. The request form answers the same for every name and sends the mail + after answering, so neither the words nor the time tell whether a name + exists or is confirmed; requests are limited to five in 15 minutes per + client address, mails to five an hour per account, and a link's page to 30 + requests in 15 minutes per client address. A link's page changes nothing + on `GET` (a mail scanner opens links): the button does. Setting a password + from a link ends every session of the account and every other reset link. + Changing the account e-mail asks for the current password, as the password + does: the address is where a reset link goes. From the security review + (2026-10-17): confirming an address also takes being signed in as its + account, and the confirmation mail does not name the account - a mistyped + address is a stranger's, who could otherwise confirm it and reset the + password; a reset request does not replace the links already sent, and + none is sent within ten minutes of the last, so a stranger repeating a + username neither kills the link in the mailbox nor spends the account's + links; the link is taken and the password set in one transaction that + checks the address again; a restore deletes every link; a rejected + request's log line hides a link's token; a stop waits for the links still + being mailed. ### Roles diff --git a/internal/store/migrations/0013_email_links.sql b/internal/store/migrations/0013_email_links.sql new file mode 100644 index 0000000..8af7fe4 --- /dev/null +++ b/internal/store/migrations/0013_email_links.sql @@ -0,0 +1,21 @@ +-- A confirmed account e-mail, and the one-time links that confirm one and +-- reset a forgotten password (password-reset, 2.14.0; owner, 2026-10-13: "the +-- e-mail must be confirmed"; 2026-10-17: notifications go to it confirmed or +-- not). email_confirmed_at is when the address in email was confirmed, '' +-- while it is not; changing the address clears it. Every address saved before +-- this file is unconfirmed. +ALTER TABLE users ADD COLUMN email_confirmed_at TEXT NOT NULL DEFAULT ''; + +-- A link's token is in a mail and nowhere else: only its SHA-256 is kept, as a +-- session's. email is the address a confirmation link confirms - it fails +-- once the account's address is another - and the one a reset link was sent +-- to. A link is taken once; a new one of the same purpose replaces the old. +CREATE TABLE user_links ( + token_hash TEXT PRIMARY KEY, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + purpose TEXT NOT NULL CHECK (purpose IN ('confirm', 'reset')), + email TEXT NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL +); +CREATE INDEX idx_user_links_user ON user_links(user_id, purpose); diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 63c9df2..89736f9 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -42,7 +42,7 @@ func TestBaselineSchema(t *testing.T) { st := openTestStore(t) want := map[string]string{ "settings": "key value", - "users": "id username password_hash role email all_domains all_inbound_domains created_at notify", + "users": "id username password_hash role email all_domains all_inbound_domains created_at notify email_confirmed_at", "sessions": "token_hash user_id created_at expires_at", "domains": "id name dkim_selector dmarc_rua created_at", "user_domains": "user_id domain_id", @@ -64,6 +64,7 @@ func TestBaselineSchema(t *testing.T) { "notify_addressees": "key address told_at", "sender_check_days": "day down_seconds deferred messages estimated", "delivery_log_days": "day reconcile_closed reconcile_missed reconcile_contradicted tailer_blind_seconds queued_over_lifetime", + "user_links": "token_hash user_id purpose email created_at expires_at", } rows, err := st.db.Query("SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name") if err != nil { @@ -111,7 +112,7 @@ func TestBaselineSchema(t *testing.T) { "idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at", "idx_inbound_spam_log_domain_id_created_at", "idx_inbound_spam_log_queue_id", "idx_send_log_app_login", "idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_domain_id", "idx_send_log_domain_id_created_at", "idx_send_log_lines_queue_id", "idx_send_log_open", "idx_send_log_queue_id", "idx_send_log_refusal_line", "idx_send_log_rejected", - "idx_sessions_expires_at", + "idx_sessions_expires_at", "idx_user_links_user", } sort.Strings(wantIdx) if strings.Join(indexes, " ") != strings.Join(wantIdx, " ") { @@ -119,8 +120,8 @@ func TestBaselineSchema(t *testing.T) { } var version, appID int64 - if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 12 { - t.Errorf("user_version = %d, %v; want 12", version, err) + if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 13 { + t.Errorf("user_version = %d, %v; want 13", version, err) } if err := st.db.QueryRow("PRAGMA application_id").Scan(&appID); err != nil || appID != applicationID { t.Errorf("application_id = %#x, %v; want %#x", appID, err, applicationID) @@ -196,8 +197,8 @@ func TestOpenMigratesA20Database(t *testing.T) { } defer st.Close() var version int - if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 12 { - t.Errorf("user_version = %d, %v; want 12", version, err) + if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 13 { + t.Errorf("user_version = %d, %v; want 13", version, err) } if ok, err := st.UserExists(); err != nil || !ok { t.Errorf("the user of the 2.0 database: exists = %v, %v", ok, err) diff --git a/internal/store/userlinks.go b/internal/store/userlinks.go new file mode 100644 index 0000000..8eead38 --- /dev/null +++ b/internal/store/userlinks.go @@ -0,0 +1,204 @@ +package store + +import ( + "database/sql" + "errors" + "fmt" + "time" +) + +// The purposes of a user link (migration 0013). +const ( + LinkConfirm = "confirm" // confirms the account's e-mail + LinkReset = "reset" // sets a new password +) + +// ErrNoLink is a link that does not exist, has been taken or has expired: +// the three are one answer. +var ErrNoLink = errors.New("no such link") + +// UserLink is a one-time link of a user, as stored: the token itself is in +// the mail alone. +type UserLink struct { + UserID int64 + Purpose string + Email string + CreatedAt time.Time + ExpiresAt time.Time +} + +// CreateUserLink stores a link of the user for purpose, sent to email, valid +// until expires, and removes every expired link. A confirmation link replaces +// the user's earlier one - it confirms the address saved last. A reset link +// does not: anyone who knows a username can ask for one, and a request of +// theirs must not kill the link the user is about to open (security review, +// 2026-10-17); setting a password removes them all. +func (s *Store) CreateUserLink(tokenHash string, userID int64, purpose, email string, now, expires time.Time) error { + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("create user link: %w", err) + } + defer tx.Rollback() + // The first statement is a write (see UpdateStatus). + if _, err := tx.Exec(`DELETE FROM user_links WHERE (user_id = ? AND purpose = ? AND purpose = ?) OR expires_at <= ?`, + userID, purpose, LinkConfirm, now.UTC().Format(time.RFC3339)); err != nil { + return fmt.Errorf("create user link: %w", err) + } + if _, err := tx.Exec(`INSERT INTO user_links (token_hash, user_id, purpose, email, created_at, expires_at) + VALUES (?, ?, ?, ?, ?, ?)`, tokenHash, userID, purpose, email, + now.UTC().Format(time.RFC3339), expires.UTC().Format(time.RFC3339)); err != nil { + return fmt.Errorf("create user link: %w", err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("create user link: %w", err) + } + return nil +} + +func scanLink(r scanRow) (UserLink, error) { + var l UserLink + var created, expires string + if err := r.Scan(&l.UserID, &l.Purpose, &l.Email, &created, &expires); err != nil { + return UserLink{}, err + } + l.CreatedAt, _ = time.Parse(time.RFC3339, created) + l.ExpiresAt, _ = time.Parse(time.RFC3339, expires) + return l, nil +} + +const linkColumns = "user_id, purpose, email, created_at, expires_at" + +// PeekUserLink returns the link of purpose with tokenHash if it is still +// valid at now, without taking it: the page a link opens shows its form. +func (s *Store) PeekUserLink(tokenHash, purpose string, now time.Time) (UserLink, error) { + l, err := scanLink(s.db.QueryRow(`SELECT `+linkColumns+` FROM user_links + WHERE token_hash = ? AND purpose = ? AND expires_at > ?`, + tokenHash, purpose, now.UTC().Format(time.RFC3339))) + if errors.Is(err, sql.ErrNoRows) { + return UserLink{}, ErrNoLink + } + if err != nil { + return UserLink{}, fmt.Errorf("peek user link: %w", err) + } + return l, nil +} + +// TakeUserLink returns the link of purpose with tokenHash and deletes it, in +// one transaction: of two requests with the same link one gets it. +func (s *Store) TakeUserLink(tokenHash, purpose string, now time.Time) (UserLink, error) { + tx, err := s.db.Begin() + if err != nil { + return UserLink{}, fmt.Errorf("take user link: %w", err) + } + defer tx.Rollback() + // The delete first: it is the write that takes the lock (see + // UpdateStatus), and RETURNING gives the row it removed (SQLite 3.35+, + // https://sqlite.org/lang_returning.html). + l, err := scanLink(tx.QueryRow(`DELETE FROM user_links + WHERE token_hash = ? AND purpose = ? AND expires_at > ? + RETURNING `+linkColumns, tokenHash, purpose, now.UTC().Format(time.RFC3339))) + if errors.Is(err, sql.ErrNoRows) { + return UserLink{}, ErrNoLink + } + if err != nil { + return UserLink{}, fmt.Errorf("take user link: %w", err) + } + if err := tx.Commit(); err != nil { + return UserLink{}, fmt.Errorf("take user link: %w", err) + } + return l, nil +} + +// LatestUserLink is the user's live link of purpose, ErrNoLink when there is +// none: Account says when the confirmation link went. +func (s *Store) LatestUserLink(userID int64, purpose string, now time.Time) (UserLink, error) { + l, err := scanLink(s.db.QueryRow(`SELECT `+linkColumns+` FROM user_links + WHERE user_id = ? AND purpose = ? AND expires_at > ? ORDER BY created_at DESC LIMIT 1`, + userID, purpose, now.UTC().Format(time.RFC3339))) + if errors.Is(err, sql.ErrNoRows) { + return UserLink{}, ErrNoLink + } + if err != nil { + return UserLink{}, fmt.Errorf("latest user link: %w", err) + } + return l, nil +} + +// DeleteUserLinks removes every link of the user of purpose: a password set +// another way leaves no reset link behind it. +func (s *Store) DeleteUserLinks(userID int64, purpose string) error { + if _, err := s.db.Exec(`DELETE FROM user_links WHERE user_id = ? AND purpose = ?`, userID, purpose); err != nil { + return fmt.Errorf("delete user links: %w", err) + } + return nil +} + +// ConfirmEmail marks the user's e-mail confirmed at now if it is still email: +// false when the address has changed since the link was sent. +func (s *Store) ConfirmEmail(userID int64, email string, now time.Time) (bool, error) { + res, err := s.db.Exec(`UPDATE users SET email_confirmed_at = ? WHERE id = ? AND email = ? AND email <> ''`, + now.UTC().Format(time.RFC3339), userID, email) + if err != nil { + return false, fmt.Errorf("confirm e-mail: %w", err) + } + n, _ := res.RowsAffected() + return n == 1, nil +} + +// TakeResetLink sets the password of the reset link with tokenHash, in one +// transaction: the link is taken, the password set only while the account's +// address is still the confirmed one the link was sent to, and the user's +// other reset links go. ErrNoLink, and nothing changed, for a link that does +// not hold - taken, expired, or of an address the account no longer has. +func (s *Store) TakeResetLink(tokenHash, passwordHash string, now time.Time) (int64, error) { + tx, err := s.db.Begin() + if err != nil { + return 0, fmt.Errorf("take reset link: %w", err) + } + defer tx.Rollback() + l, err := scanLink(tx.QueryRow(`DELETE FROM user_links + WHERE token_hash = ? AND purpose = ? AND expires_at > ? + RETURNING `+linkColumns, tokenHash, LinkReset, now.UTC().Format(time.RFC3339))) + if errors.Is(err, sql.ErrNoRows) { + return 0, ErrNoLink + } + if err != nil { + return 0, fmt.Errorf("take reset link: %w", err) + } + res, err := tx.Exec(`UPDATE users SET password_hash = ? + WHERE id = ? AND email = ? AND email <> '' AND email_confirmed_at <> ''`, passwordHash, l.UserID, l.Email) + if err != nil { + return 0, fmt.Errorf("take reset link: %w", err) + } + if n, _ := res.RowsAffected(); n != 1 { + return 0, ErrNoLink + } + if _, err := tx.Exec(`DELETE FROM user_links WHERE user_id = ? AND purpose = ?`, l.UserID, LinkReset); err != nil { + return 0, fmt.Errorf("take reset link: %w", err) + } + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("take reset link: %w", err) + } + return l.UserID, nil +} + +// DeleteAllUserLinks removes every link: a restored backup must not bring +// back one that was taken after it was made (security review, 2026-10-17). +func (s *Store) DeleteAllUserLinks() error { + if _, err := s.db.Exec(`DELETE FROM user_links`); err != nil { + return fmt.Errorf("delete user links: %w", err) + } + return nil +} + +// SetUserPassword replaces the user's password hash alone. +func (s *Store) SetUserPassword(userID int64, passwordHash string) error { + res, err := s.db.Exec(`UPDATE users SET password_hash = ? WHERE id = ?`, passwordHash, userID) + if err != nil { + return fmt.Errorf("set password: %w", err) + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrUserNotFound + } + return nil +} diff --git a/internal/store/userlinks_test.go b/internal/store/userlinks_test.go new file mode 100644 index 0000000..7d96686 --- /dev/null +++ b/internal/store/userlinks_test.go @@ -0,0 +1,191 @@ +package store + +import ( + "errors" + "sync" + "testing" + "time" +) + +func linkTestUser(t *testing.T, st *Store, email string) User { + t.Helper() + if err := st.CreateGlobalUser("admin", "hash"); err != nil { + t.Fatal(err) + } + u, err := st.GetUserByUsername("admin") + if err != nil { + t.Fatal(err) + } + if err := st.UpdateUser(u.ID, u.Username, u.PasswordHash, email); err != nil { + t.Fatal(err) + } + u, _ = st.GetUser(u.ID) + return u +} + +// A link is taken once, not after it expires, and only for its purpose; a +// newer one of the same purpose replaces it. +func TestUserLinks(t *testing.T) { + st := openTestStore(t) + u := linkTestUser(t, st, "admin@example.org") + now := time.Date(2026, 10, 17, 10, 0, 0, 0, time.UTC) + + if err := st.CreateUserLink("h1", u.ID, LinkReset, u.Email, now, now.Add(30*time.Minute)); err != nil { + t.Fatal(err) + } + if _, err := st.PeekUserLink("h1", LinkConfirm, now); !errors.Is(err, ErrNoLink) { + t.Errorf("a reset link taken as a confirmation one: %v", err) + } + if l, err := st.PeekUserLink("h1", LinkReset, now); err != nil || l.UserID != u.ID || l.Email != u.Email { + t.Fatalf("peek = %+v, %v", l, err) + } + if _, err := st.PeekUserLink("h1", LinkReset, now.Add(30*time.Minute)); !errors.Is(err, ErrNoLink) { + t.Errorf("an expired link was valid: %v", err) + } + // A second reset link leaves the first working: a request of anyone's + // must not kill the link in the user's mailbox. + if err := st.CreateUserLink("h2", u.ID, LinkReset, u.Email, now.Add(time.Minute), now.Add(31*time.Minute)); err != nil { + t.Fatal(err) + } + if l, err := st.LatestUserLink(u.ID, LinkReset, now.Add(2*time.Minute)); err != nil || !l.CreatedAt.Equal(now.Add(time.Minute)) { + t.Errorf("latest = %+v, %v", l, err) + } + if _, err := st.TakeUserLink("h1", LinkReset, now.Add(2*time.Minute)); err != nil { + t.Errorf("the first reset link died with the second: %v", err) + } + if _, err := st.TakeUserLink("h2", LinkReset, now.Add(2*time.Minute)); err != nil { + t.Fatalf("take: %v", err) + } + if _, err := st.TakeUserLink("h2", LinkReset, now.Add(3*time.Minute)); !errors.Is(err, ErrNoLink) { + t.Errorf("a link was taken twice: %v", err) + } + if _, err := st.LatestUserLink(u.ID, LinkReset, now.Add(3*time.Minute)); !errors.Is(err, ErrNoLink) { + t.Errorf("a taken link is still the latest: %v", err) + } + // A second confirmation link replaces the first: it confirms the + // address saved last. + if err := st.CreateUserLink("c1", u.ID, LinkConfirm, u.Email, now, now.Add(time.Hour)); err != nil { + t.Fatal(err) + } + if err := st.CreateUserLink("c2", u.ID, LinkConfirm, u.Email, now.Add(time.Minute), now.Add(time.Hour)); err != nil { + t.Fatal(err) + } + if _, err := st.TakeUserLink("c1", LinkConfirm, now.Add(2*time.Minute)); !errors.Is(err, ErrNoLink) { + t.Errorf("the replaced confirmation link was taken: %v", err) + } +} + +// A reset link sets the password only while the account's address is still +// the confirmed one it was sent to; then it and the user's other reset links +// are gone. A link that does not hold changes nothing and is not used up. +func TestTakeResetLink(t *testing.T) { + st := openTestStore(t) + u := linkTestUser(t, st, "admin@example.org") + now := time.Date(2026, 10, 17, 10, 0, 0, 0, time.UTC) + for _, h := range []string{"r1", "r2"} { + if err := st.CreateUserLink(h, u.ID, LinkReset, u.Email, now, now.Add(30*time.Minute)); err != nil { + t.Fatal(err) + } + } + // Not confirmed yet: refused, and the link is still there. + if _, err := st.TakeResetLink("r1", "new-hash", now); !errors.Is(err, ErrNoLink) { + t.Fatalf("an unconfirmed address: %v", err) + } + if _, err := st.PeekUserLink("r1", LinkReset, now); err != nil { + t.Fatalf("a refused take used the link up: %v", err) + } + if ok, _ := st.ConfirmEmail(u.ID, u.Email, now); !ok { + t.Fatal("confirm") + } + id, err := st.TakeResetLink("r1", "new-hash", now) + if err != nil || id != u.ID { + t.Fatalf("take = %d, %v", id, err) + } + if got, _ := st.GetUser(u.ID); got.PasswordHash != "new-hash" { + t.Errorf("hash = %q", got.PasswordHash) + } + if _, err := st.PeekUserLink("r2", LinkReset, now); !errors.Is(err, ErrNoLink) { + t.Errorf("another reset link survived the reset: %v", err) + } + // A link of an address the account no longer has does not hold. + if err := st.CreateUserLink("r3", u.ID, LinkReset, u.Email, now, now.Add(30*time.Minute)); err != nil { + t.Fatal(err) + } + if err := st.UpdateUser(u.ID, u.Username, "new-hash", "other@example.org"); err != nil { + t.Fatal(err) + } + if _, err := st.TakeResetLink("r3", "third-hash", now); !errors.Is(err, ErrNoLink) { + t.Errorf("a link of the old address set the password: %v", err) + } + if err := st.DeleteAllUserLinks(); err != nil { + t.Fatal(err) + } + if _, err := st.PeekUserLink("r3", LinkReset, now); !errors.Is(err, ErrNoLink) { + t.Errorf("a link survived DeleteAllUserLinks: %v", err) + } +} + +// Of two requests with one link, one takes it. +func TestUserLinkTakenOnce(t *testing.T) { + st := openTestStore(t) + u := linkTestUser(t, st, "admin@example.org") + now := time.Now() + if err := st.CreateUserLink("h", u.ID, LinkReset, u.Email, now, now.Add(time.Hour)); err != nil { + t.Fatal(err) + } + var wg sync.WaitGroup + var mu sync.Mutex + took := 0 + for range 8 { + wg.Add(1) + go func() { + defer wg.Done() + if _, err := st.TakeUserLink("h", LinkReset, now); err == nil { + mu.Lock() + took++ + mu.Unlock() + } + }() + } + wg.Wait() + if took != 1 { + t.Errorf("the link was taken %d times", took) + } +} + +// The mark follows the address: confirmed only while it is the address the +// link was sent to, cleared when the address changes, kept when it does not. +func TestEmailConfirmation(t *testing.T) { + st := openTestStore(t) + u := linkTestUser(t, st, "admin@example.org") + if !u.EmailConfirmed.IsZero() { + t.Fatal("a new address is confirmed") + } + now := time.Date(2026, 10, 17, 10, 0, 0, 0, time.UTC) + if ok, err := st.ConfirmEmail(u.ID, "old@example.org", now); err != nil || ok { + t.Errorf("another address confirmed: %v %v", ok, err) + } + if ok, err := st.ConfirmEmail(u.ID, "admin@example.org", now); err != nil || !ok { + t.Fatalf("confirm: %v %v", ok, err) + } + u, _ = st.GetUser(u.ID) + if !u.EmailConfirmed.Equal(now) { + t.Fatalf("confirmed at %v", u.EmailConfirmed) + } + // Saving the same address keeps the mark; another clears it. + if err := st.UpdateUser(u.ID, "admin2", u.PasswordHash, u.Email); err != nil { + t.Fatal(err) + } + if u, _ = st.GetUser(u.ID); u.EmailConfirmed.IsZero() { + t.Error("the same address lost its mark") + } + if err := st.SaveUser(UserChange{ID: u.ID, Username: u.Username, Email: "new@example.org", Role: RoleGlobal}); err != nil { + t.Fatal(err) + } + if u, _ = st.GetUser(u.ID); !u.EmailConfirmed.IsZero() { + t.Error("a new address kept the old one's mark") + } + if ok, _ := st.ConfirmEmail(u.ID, "admin@example.org", now); ok { + t.Error("a link of the old address confirmed the new one") + } +} diff --git a/internal/store/users.go b/internal/store/users.go index dc0af17..f1d1411 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -42,6 +42,9 @@ type User struct { // Email is the user's own address — what the panel writes to, and what the // button beside a domain's typed report address fills it with. Email string + // EmailConfirmed is when Email was confirmed by its link, zero while it + // is not: only a confirmed address is sent a reset link (migration 0013). + EmailConfirmed time.Time // AllDomains / AllInboundDomains widen a domain user's list to every // domain of that direction, including those added later; the id lists // are then ignored. @@ -55,19 +58,20 @@ type User struct { InboundDomainIDs []int64 } -const userColumns = "id, username, password_hash, role, email, all_domains, all_inbound_domains, notify, created_at" +const userColumns = "id, username, password_hash, role, email, all_domains, all_inbound_domains, notify, created_at, email_confirmed_at" func scanUser(r scanRow) (User, error) { var ( - u User - createdAt string + u User + createdAt, confirmed string ) err := r.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.Role, &u.Email, - &u.AllDomains, &u.AllInboundDomains, &u.Notify, &createdAt) + &u.AllDomains, &u.AllInboundDomains, &u.Notify, &createdAt, &confirmed) if err != nil { return User{}, err } u.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) + u.EmailConfirmed, _ = time.Parse(time.RFC3339, confirmed) return u, nil } @@ -316,8 +320,8 @@ func (s *Store) SaveUser(c UserChange) error { // An empty hash keeps the stored one: the form read it when it was // opened, and the user may have changed it on Account since. res, err := tx.Exec( - "UPDATE users SET username = ?, password_hash = COALESCE(NULLIF(?, ''), password_hash), email = ?, role = ? WHERE id = ?", - c.Username, c.PasswordHash, c.Email, c.Role, c.ID, + "UPDATE users SET username = ?, password_hash = COALESCE(NULLIF(?, ''), password_hash), email = ?, role = ?, "+keepConfirmed+" WHERE id = ?", + c.Username, c.PasswordHash, c.Email, c.Role, c.Email, c.ID, ) if err != nil { if isUniqueViolation(err) { @@ -366,11 +370,19 @@ func requireAGlobal(tx *sql.Tx) error { return nil } -// UpdateUser replaces a user's username, password hash and account e-mail. +// keepConfirmed keeps the confirmation of an address that stays the same and +// clears it for another; its placeholder takes the new address. SQLite +// 3.53.4, https://sqlite.org/lang_update.html: every expression on the right +// of SET sees the row as it was before the UPDATE, so email here is the old +// address. +const keepConfirmed = "email_confirmed_at = CASE WHEN email = ? THEN email_confirmed_at ELSE '' END" + +// UpdateUser replaces a user's username, password hash and account e-mail; a +// new e-mail is unconfirmed. func (s *Store) UpdateUser(id int64, username, passwordHash, email string) error { res, err := s.db.Exec( - "UPDATE users SET username = ?, password_hash = ?, email = ? WHERE id = ?", - username, passwordHash, email, id, + "UPDATE users SET username = ?, password_hash = ?, email = ?, "+keepConfirmed+" WHERE id = ?", + username, passwordHash, email, email, id, ) if err != nil { if isUniqueViolation(err) { diff --git a/internal/web/auth/auth.go b/internal/web/auth/auth.go index 765ddfe..962b471 100644 --- a/internal/web/auth/auth.go +++ b/internal/web/auth/auth.go @@ -8,6 +8,7 @@ import ( "net/http" "strconv" "strings" + "sync" "time" "github.com/mixeme/selfpost/internal/store" @@ -33,6 +34,16 @@ type Module struct { loginLimiter *rateLimiter setupLimiter *rateLimiter passwordLimiter *rateLimiter + // resetLimiter counts the requests for a reset link per client address, + // linkLimiter the requests a link's page gets per client address, and + // mailLimiter the links mailed to one user's address (links.go). + resetLimiter *rateLimiter + linkLimiter *rateLimiter + mailLimiter *rateLimiter + mailer Mailer + // mailing counts the links being mailed after their answer, so a stop + // waits for them (Wait) before the database closes. + mailing sync.WaitGroup trustedProxies []*net.IPNet } @@ -56,11 +67,20 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo // out of signing in (audit 2026-10-12, L15), and the one guessing // is whoever holds that user's session, wherever from. passwordLimiter: newRateLimiter(10, 15*time.Minute), - trustedProxies: cfg.TrustedProxyCIDRs, + resetLimiter: newRateLimiter(5, 15*time.Minute), + linkLimiter: newRateLimiter(30, 15*time.Minute), + // Five links an hour to one address, of either kind: three resets + // asked for and a couple of confirmations is ordinary use, a mailbox + // full of them is not. + mailLimiter: newRateLimiter(5, time.Hour), + trustedProxies: cfg.TrustedProxyCIDRs, } m.setupLimiter.startSweeper() m.loginLimiter.startSweeper() m.passwordLimiter.startSweeper() + m.resetLimiter.startSweeper() + m.linkLimiter.startSweeper() + m.mailLimiter.startSweeper() // Made now rather than at the first unknown name, which would otherwise // pay for two bcrypt runs (review of audit stage 4). go noSuchUserHash() diff --git a/internal/web/auth/handlers.go b/internal/web/auth/handlers.go index 3e6fe52..59a1649 100644 --- a/internal/web/auth/handlers.go +++ b/internal/web/auth/handlers.go @@ -88,7 +88,7 @@ func (m *Module) HandleLogin(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: - m.renderLogin(w, http.StatusOK, "") + m.view.Render(w, http.StatusOK, "login", view.NewLogin(m.cfg.Hostname, "").WithInfo(loginInfo(r))) case http.MethodPost: m.submitLogin(w, r) default: diff --git a/internal/web/auth/links.go b/internal/web/auth/links.go new file mode 100644 index 0000000..2e2594e --- /dev/null +++ b/internal/web/auth/links.go @@ -0,0 +1,402 @@ +package auth + +import ( + "context" + "errors" + "net/http" + "strconv" + "strings" + "time" + + "github.com/mixeme/selfpost/internal/store" + "github.com/mixeme/selfpost/internal/web/validate" + "github.com/mixeme/selfpost/internal/web/view" + "golang.org/x/crypto/bcrypt" +) + +// The one-time links of password-reset (owner, 2026-10-13: a reset link goes +// only to a confirmed address; 2026-10-17: notifications go to one confirmed +// or not). A link is 256 random bits in a mail; the database keeps its +// SHA-256 only, as a session's (store.CreateUserLink), so a copy of the +// database or a backup opens nothing. Its address is the server's own name: +// the Host a request names is the requester's to choose, and a reset link +// built from it would carry the token to whichever host they chose. +const ( + confirmLinkLife = 24 * time.Hour + resetLinkLife = 30 * time.Minute + // resetSpacing is how long after a reset link another is not sent: the + // one in the mailbox works, and a stranger repeating a username cannot + // spend the account's links an hour in a minute (security review, + // 2026-10-17). + resetSpacing = 10 * time.Minute +) + +// later runs f after the answer, counted for Wait. +func (m *Module) later(f func()) { + m.mailing.Add(1) + go func() { + defer m.mailing.Done() + f() + }() +} + +// Wait waits, until ctx ends, for the links still being mailed: the panel's +// stop calls it before the database closes. +func (m *Module) Wait(ctx context.Context) { + done := make(chan struct{}) + go func() { + m.mailing.Wait() + close(done) + }() + select { + case <-done: + case <-ctx.Done(): + logf("panel: stopped with a link still being mailed") + } +} + +// Mailer sends the panel's own mail (notify.Mailer). +type Mailer interface { + Send(to, subject, body string) error +} + +// errNoMail is a link that cannot be sent: no address, no mailer, or no name +// to put in it. +var errNoMail = errors.New("no address to send a link to") + +// SetMailer gives the module the mailer its links go out by. +func (m *Module) SetMailer(ml Mailer) { m.mailer = ml } + +// CanMail reports whether a link can be sent at all: a mailer, and the +// server's name to put in it. +func (m *Module) CanMail() bool { return m.mailer != nil && m.cfg.Hostname != "" } + +// linkURL is the address of a link: https:// the server's own name, the path +// and the token. +func (m *Module) linkURL(path, token string) string { + return "https://" + m.cfg.Hostname + path + token +} + +// AllowConfirmationLink says whether another confirmation link may be sent +// to the user's address now: five an hour, whoever asks (the user from +// Account, the global administrator from the user form) - a link is a mail +// to that address, and nobody is to fill a mailbox with them. +func (m *Module) AllowConfirmationLink(userID int64) bool { + return m.mailLimiter.Allow("confirm:" + strconv.FormatInt(userID, 10)) +} + +// SendConfirmation mails u's address a link that confirms it, replacing the +// link sent before. The caller has asked AllowConfirmationLink. +func (m *Module) SendConfirmation(u store.User) error { + if u.Email == "" || m.mailer == nil || m.cfg.Hostname == "" { + return errNoMail + } + token := randomToken(32) + now := time.Now() + if err := m.store.CreateUserLink(hashToken(token), u.ID, store.LinkConfirm, u.Email, now, now.Add(confirmLinkLife)); err != nil { + return err + } + // The account is not named: the address may be a mistyped one, a + // stranger's (security review, 2026-10-17); confirming also takes being + // signed in as the account (HandleConfirm). + body := "The address " + u.Email + " was given for an account of the SelfPost panel at " + m.cfg.Hostname + + ". Sign in to that account, then open this link to confirm it:\n\n" + m.linkURL("/confirm/", token) + + "\n\nIt works once, for 24 hours. A confirmed address can be sent a link to reset the account's password. " + + "If you did not expect this message, ignore it: nothing changes.\n" + if err := m.mailer.Send(u.Email, "[SelfPost] Confirm your e-mail", body); err != nil { + return err + } + logf("panel: user %d: a confirmation link was sent", u.ID) + return nil +} + +// SendConfirmationLater is SendConfirmation after the answer: a stuck +// Postfix does not hold the form that saved the address (sendmail's own +// time-out is 20 s). A link that cannot go is logged, and Account offers to +// send it again. +func (m *Module) SendConfirmationLater(u store.User) { + m.later(func() { + if err := m.SendConfirmation(u); err != nil { + logf("panel: user %d: confirmation link not sent: %v", u.ID, err) + } + }) +} + +// HandleReset serves /login/reset: the form that asks for a reset link (GET) +// and the request (POST). The answer to a request is the sign-in page with +// the same words whatever the name - one that exists or not, with a +// confirmed address or not - and the link, when there is one, goes out after +// the answer, so the time to answer tells nothing either. +func (m *Module) HandleReset(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + m.view.Render(w, http.StatusOK, "login-reset", view.NewReset(m.cfg.Hostname, "")) + case http.MethodPost: + if !m.resetLimiter.Allow(clientIP(r, m.trustedProxies)) { + m.view.Render(w, http.StatusTooManyRequests, "login-reset", view.NewReset(m.cfg.Hostname, "Too many attempts. Please wait and try again.")) + return + } + if err := r.ParseForm(); err != nil { + m.view.Render(w, http.StatusBadRequest, "login-reset", view.NewReset(m.cfg.Hostname, "Invalid form submission.")) + return + } + username := strings.TrimSpace(r.PostFormValue("username")) + m.later(func() { m.sendReset(username) }) + http.Redirect(w, r, "/login?reset=sent", http.StatusSeeOther) + default: + w.Header().Set("Allow", "GET, POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } +} + +// sendReset mails the account named username a reset link if its address is +// confirmed, at most three an hour. +func (m *Module) sendReset(username string) { + if username == "" || m.mailer == nil || m.cfg.Hostname == "" { + return + } + u, err := m.store.GetUserByUsername(username) + if err != nil { + if !errors.Is(err, store.ErrUserNotFound) { + logf("panel: reset: get user: %v", err) + } + return + } + if u.Email == "" || u.EmailConfirmed.IsZero() { + logf("panel: user %d asked for a reset link and has no confirmed e-mail; none was sent", u.ID) + return + } + if l, err := m.store.LatestUserLink(u.ID, store.LinkReset, time.Now()); err == nil && time.Since(l.CreatedAt) < resetSpacing { + logf("panel: user %d: a reset link went less than %s ago; none was sent", u.ID, resetSpacing) + return + } + if !m.mailLimiter.Allow("reset:" + strconv.FormatInt(u.ID, 10)) { + logf("panel: user %d: reset links asked for too often; none was sent", u.ID) + return + } + token := randomToken(32) + now := time.Now() + if err := m.store.CreateUserLink(hashToken(token), u.ID, store.LinkReset, u.Email, now, now.Add(resetLinkLife)); err != nil { + logf("panel: user %d: reset link: %v", u.ID, err) + return + } + body := "Someone asked to reset the password of the SelfPost account " + u.Username + " at " + m.cfg.Hostname + + ". Open this link to set a new one:\n\n" + m.linkURL("/login/reset/", token) + + "\n\nIt works once, for 30 minutes. If it was not you, ignore this message: the password stays as it is.\n" + if err := m.mailer.Send(u.Email, "[SelfPost] Reset your password", body); err != nil { + logf("panel: user %d: reset link not sent: %v", u.ID, err) + return + } + logf("panel: user %d: a reset link was sent", u.ID) +} + +// linkToken is the token of a link path under prefix; "" for anything else. +func linkToken(path, prefix string) string { + token := strings.TrimPrefix(path, prefix) + if token == path || token == "" || strings.Contains(token, "/") { + return "" + } + return token +} + +const deadLink = "This link has expired or was already used." + +// HandleResetLink serves /login/reset/: the new password's form (GET) +// and setting it (POST). Setting it takes the link, ends every session of the +// account and every other reset link, and leaves the person at sign-in. +func (m *Module) HandleResetLink(w http.ResponseWriter, r *http.Request) { + if !m.linkLimiter.Allow(clientIP(r, m.trustedProxies)) { + http.Error(w, "too many requests", http.StatusTooManyRequests) + return + } + token := linkToken(r.URL.Path, "/login/reset/") + if token == "" { + http.NotFound(w, r) + return + } + dead := func() { + m.view.Render(w, http.StatusNotFound, "login-reset", view.NewReset(m.cfg.Hostname, deadLink+" Ask for a new one.")) + } + switch r.Method { + case http.MethodGet: + l, err := m.store.PeekUserLink(hashToken(token), store.LinkReset, time.Now()) + if err != nil { + dead() + return + } + u, ok := m.linkUser(l) + if !ok { + dead() + return + } + m.view.Render(w, http.StatusOK, "login-reset-set", view.NewResetSet(m.cfg.Hostname, u.Username, token, "")) + case http.MethodPost: + m.setPasswordFromLink(w, r, token, dead) + default: + w.Header().Set("Allow", "GET, POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } +} + +// linkUser is the user of a reset link while the link still holds: the +// account's address is the one it was sent to, and confirmed. +func (m *Module) linkUser(l store.UserLink) (store.User, bool) { + u, err := m.store.GetUser(l.UserID) + if err != nil { + return store.User{}, false + } + return u, u.Email == l.Email && !u.EmailConfirmed.IsZero() +} + +func (m *Module) setPasswordFromLink(w http.ResponseWriter, r *http.Request, token string, dead func()) { + l, err := m.store.PeekUserLink(hashToken(token), store.LinkReset, time.Now()) + if err != nil { + dead() + return + } + u, ok := m.linkUser(l) + if !ok { + dead() + return + } + fail := func(status int, msg string) { + m.view.Render(w, status, "login-reset-set", view.NewResetSet(m.cfg.Hostname, u.Username, token, msg)) + } + if err := r.ParseForm(); err != nil { + fail(http.StatusBadRequest, "Invalid form submission.") + return + } + password := r.PostFormValue("new_password") + if password != r.PostFormValue("new_password_confirm") { + fail(http.StatusBadRequest, "Passwords do not match.") + return + } + if err := validate.AdminPassword(password); err != nil { + fail(http.StatusBadRequest, err.Error()) + return + } + hash, err := bcrypt.GenerateFromPassword([]byte(password), BcryptCost) + if err != nil { + logf("panel: reset: hashing password failed: %v", err) + fail(http.StatusInternalServerError, "Internal error. Please try again.") + return + } + // Taken only now, so a mistyped repeat does not use the link up; taken + // and the password set in one transaction, which also checks the + // address again (store.TakeResetLink): of two requests with the link one + // sets it, and a write that fails leaves the link to try again. + if _, err := m.store.TakeResetLink(hashToken(token), string(hash), time.Now()); err != nil { + if errors.Is(err, store.ErrNoLink) { + dead() + return + } + logf("panel: reset: set password of user %d: %v", u.ID, err) + fail(http.StatusInternalServerError, "Internal error. Please try again.") + return + } + m.sessions.DestroyAll(u.ID) + logf("panel: user %d set a new password from a reset link; every session was ended", u.ID) + http.Redirect(w, r, "/login?reset=done", http.StatusSeeOther) +} + +// HandleConfirm serves /confirm/: the address and a button (GET), and +// confirming it (POST). The token proves its holder reads the mailbox; being +// signed in as the account proves the mailbox is the account's - without it +// a mistyped address would hand the account to whoever reads it, by a reset +// link (security review, 2026-10-17). +func (m *Module) HandleConfirm(w http.ResponseWriter, r *http.Request) { + if !m.linkLimiter.Allow(clientIP(r, m.trustedProxies)) { + http.Error(w, "too many requests", http.StatusTooManyRequests) + return + } + token := linkToken(r.URL.Path, "/confirm/") + if token == "" { + http.NotFound(w, r) + return + } + dead := func(msg string) { + p := view.NewConfirm(m.cfg.Hostname, "", "", "") + p.Error = msg + m.view.Render(w, http.StatusNotFound, "confirm-email", p) + } + signIn := func(w http.ResponseWriter) { + p := view.NewConfirm(m.cfg.Hostname, "", "", "") + p.Error = "Sign in to the account in this browser, then open the link again." + p.SignIn = true + m.view.Render(w, http.StatusForbidden, "confirm-email", p) + } + switch r.Method { + case http.MethodGet: + l, err := m.store.PeekUserLink(hashToken(token), store.LinkConfirm, time.Now()) + if err != nil { + dead(deadLink + " Send a new one from Account.") + return + } + u, err := m.store.GetUser(l.UserID) + if err != nil || u.Email != l.Email { + dead("The account's e-mail has changed since this link was sent.") + return + } + if !m.signedInAs(r, l.UserID) { + signIn(w) + return + } + m.view.Render(w, http.StatusOK, "confirm-email", view.NewConfirm(m.cfg.Hostname, u.Username, u.Email, token)) + case http.MethodPost: + // Looked at first and taken only for its own account's session, so + // a stranger's press does not use the link up. + l, err := m.store.PeekUserLink(hashToken(token), store.LinkConfirm, time.Now()) + if err != nil { + dead(deadLink + " Send a new one from Account.") + return + } + if !m.signedInAs(r, l.UserID) { + signIn(w) + return + } + if l, err = m.store.TakeUserLink(hashToken(token), store.LinkConfirm, time.Now()); err != nil { + dead(deadLink + " Send a new one from Account.") + return + } + ok, err := m.store.ConfirmEmail(l.UserID, l.Email, time.Now()) + if err != nil { + logf("panel: confirm: %v", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + if !ok { + dead("The account's e-mail has changed since this link was sent.") + return + } + u, _ := m.store.GetUser(l.UserID) + logf("panel: user %d confirmed their e-mail", l.UserID) + p := view.NewConfirm(m.cfg.Hostname, u.Username, l.Email, "") + p.Done = true + m.view.Render(w, http.StatusOK, "confirm-email", p) + default: + w.Header().Set("Allow", "GET, POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } +} + +// signedInAs reports whether the request carries a live session of userID. +func (m *Module) signedInAs(r *http.Request, userID int64) bool { + token, ok := m.sessionToken(r) + if !ok { + return false + } + id, ok := m.sessions.Lookup(token) + return ok && id == userID +} + +// loginInfo is the sign-in page's success flash for what brought the person +// back to it. +func loginInfo(r *http.Request) string { + switch r.URL.Query().Get("reset") { + case "sent": + return "If that account has a confirmed e-mail, the link is on its way. It works once, for 30 minutes." + case "done": + return "Password set. Sign in with it." + } + return "" +} diff --git a/internal/web/auth/links_test.go b/internal/web/auth/links_test.go new file mode 100644 index 0000000..1519e36 --- /dev/null +++ b/internal/web/auth/links_test.go @@ -0,0 +1,301 @@ +package auth + +import ( + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + "time" + + "golang.org/x/crypto/bcrypt" +) + +// sentMail is one message a recordingMailer was given. +type sentMail struct{ to, subject, body string } + +// recordingMailer keeps what would have been sent. +type recordingMailer struct{ sent chan sentMail } + +func (r *recordingMailer) Send(to, subject, body string) error { + r.sent <- sentMail{to, subject, body} + return nil +} + +// next is the next message, or none within a moment: a reset link goes out +// after the answer. +func (r *recordingMailer) next(t *testing.T) (sentMail, bool) { + t.Helper() + select { + case m := <-r.sent: + return m, true + case <-time.After(500 * time.Millisecond): + return sentMail{}, false + } +} + +// linkModule is a panel named mail.example.org with the administrator of +// moduleWithAdmin, its address admin@example.org, confirmed when confirmed. +func linkModule(t *testing.T, confirmed bool) (*Module, *recordingMailer) { + t.Helper() + m := moduleWithAdmin(t) + m.cfg.Hostname = "mail.example.org" + ml := &recordingMailer{sent: make(chan sentMail, 8)} + m.SetMailer(ml) + u, err := m.store.GetUserByUsername("admin") + if err != nil { + t.Fatal(err) + } + if err := m.store.UpdateUser(u.ID, u.Username, u.PasswordHash, "admin@example.org"); err != nil { + t.Fatal(err) + } + if confirmed { + if ok, err := m.store.ConfirmEmail(u.ID, "admin@example.org", time.Now()); err != nil || !ok { + t.Fatalf("confirm: %v %v", ok, err) + } + } + return m, ml +} + +func postForm(h http.HandlerFunc, target, remoteAddr string, form url.Values) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodPost, target, strings.NewReader(form.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + r.RemoteAddr = remoteAddr + rec := httptest.NewRecorder() + h(rec, r) + return rec +} + +func get(h http.HandlerFunc, target string) *httptest.ResponseRecorder { + return getAs(h, target, "") +} + +// getAs is get with the session cookie of session, none for "". +func getAs(h http.HandlerFunc, target, session string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, target, nil) + r.RemoteAddr = "192.0.2.1:1234" + if session != "" { + r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: session}) + } + rec := httptest.NewRecorder() + h(rec, r) + return rec +} + +// postAs is a POST of no fields with the session cookie of session. +func postAs(h http.HandlerFunc, target, session string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodPost, target, nil) + r.RemoteAddr = "192.0.2.1:1234" + if session != "" { + r.AddCookie(&http.Cookie{Name: "selfpost_session", Value: session}) + } + rec := httptest.NewRecorder() + h(rec, r) + return rec +} + +var resetLinkRe = regexp.MustCompile(`https://mail\.example\.org/login/reset/([A-Za-z0-9_-]{43})\n`) + +// The answer is the same for every name; only an account with a confirmed +// address is sent a link, and the link names the server, whatever Host the +// request named. +func TestResetSendsALinkOnlyToAConfirmedAddress(t *testing.T) { + for _, c := range []struct { + name string + confirmed bool + username string + mail bool + }{ + {"unconfirmed", false, "admin", false}, + {"unknown name", true, "nobody", false}, + {"confirmed", true, "admin", true}, + } { + t.Run(c.name, func(t *testing.T) { + m, ml := linkModule(t, c.confirmed) + r := httptest.NewRequest(http.MethodPost, "http://evil.example.net/login/reset", + strings.NewReader(url.Values{"username": {c.username}}.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + r.RemoteAddr = "192.0.2.1:1234" + rec := httptest.NewRecorder() + m.HandleReset(rec, r) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/login?reset=sent" { + t.Fatalf("answer %d %q", rec.Code, rec.Header().Get("Location")) + } + got, sent := ml.next(t) + if sent != c.mail { + t.Fatalf("mail sent = %v, want %v", sent, c.mail) + } + if !sent { + return + } + if got.to != "admin@example.org" || got.subject != "[SelfPost] Reset your password" || + !resetLinkRe.MatchString(got.body) || strings.Contains(got.body, "evil") { + t.Errorf("mail = %+v", got) + } + }) + } + // The sign-in page it lands on says the same whatever happened. + m, _ := linkModule(t, false) + if body := get(m.HandleLogin, "/login?reset=sent").Body.String(); !strings.Contains(body, + "If that account has a confirmed e-mail, the link is on its way. It works once, for 30 minutes.") { + t.Errorf("sign-in after a request:\n%s", body) + } +} + +func askReset(t *testing.T, m *Module, ml *recordingMailer) string { + t.Helper() + postForm(m.HandleReset, "/login/reset", "192.0.2.1:1234", url.Values{"username": {"admin"}}) + got, ok := ml.next(t) + if !ok { + t.Fatal("no reset link was sent") + } + return resetLinkRe.FindStringSubmatch(got.body)[1] +} + +// A link opens the form; a mistyped repeat does not use it up; the password +// it sets is the one that signs in, every session ends, and the link works +// once. +func TestResetLinkSetsThePasswordOnce(t *testing.T) { + m, ml := linkModule(t, true) + session := mustCreate(t, m.sessions, "admin") + token := askReset(t, m, ml) + path := "/login/reset/" + token + + page := get(m.HandleResetLink, path) + if page.Code != http.StatusOK || !strings.Contains(page.Body.String(), "mail.example.org · admin") || + !strings.Contains(page.Body.String(), `action="`+path+`"`) { + t.Fatalf("the link's page: %d\n%s", page.Code, page.Body.String()) + } + const fresh = "a-new-password-of-mine" + if rec := postForm(m.HandleResetLink, path, "192.0.2.1:1234", url.Values{"new_password": {fresh}, "new_password_confirm": {"other"}}); rec.Code != http.StatusBadRequest { + t.Fatalf("a mistyped repeat: %d", rec.Code) + } + if rec := postForm(m.HandleResetLink, path, "192.0.2.1:1234", url.Values{"new_password": {"short"}, "new_password_confirm": {"short"}}); rec.Code != http.StatusBadRequest { + t.Fatalf("a short password: %d", rec.Code) + } + rec := postForm(m.HandleResetLink, path, "192.0.2.1:1234", url.Values{"new_password": {fresh}, "new_password_confirm": {fresh}}) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/login?reset=done" { + t.Fatalf("set: %d %q\n%s", rec.Code, rec.Header().Get("Location"), rec.Body.String()) + } + u, _ := m.store.GetUserByUsername("admin") + if bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(fresh)) != nil { + t.Error("the new password does not sign in") + } + if _, found, _ := m.store.LookupSession(hashToken(session)); found { + t.Error("a session survived the reset") + } + if again := get(m.HandleResetLink, path); again.Code != http.StatusNotFound || !strings.Contains(again.Body.String(), deadLink) { + t.Errorf("the link worked twice: %d", again.Code) + } + if again := postForm(m.HandleResetLink, path, "192.0.2.1:1234", url.Values{"new_password": {fresh + "x"}, "new_password_confirm": {fresh + "x"}}); again.Code != http.StatusNotFound { + t.Errorf("the link set a password twice: %d", again.Code) + } +} + +// A link sent to an address the account no longer has does not work. +func TestResetLinkDiesWithTheAddress(t *testing.T) { + m, ml := linkModule(t, true) + token := askReset(t, m, ml) + u, _ := m.store.GetUserByUsername("admin") + if err := m.store.UpdateUser(u.ID, u.Username, u.PasswordHash, "other@example.org"); err != nil { + t.Fatal(err) + } + if rec := get(m.HandleResetLink, "/login/reset/"+token); rec.Code != http.StatusNotFound { + t.Errorf("a link of the old address opened: %d", rec.Code) + } +} + +// Requests for a link are counted per address: the sixth in 15 minutes is +// refused, and it says so without saying anything of the name. +func TestResetRequestsAreLimited(t *testing.T) { + m, _ := linkModule(t, false) + for i := range 5 { + if rec := postForm(m.HandleReset, "/login/reset", "192.0.2.9:1", url.Values{"username": {"x"}}); rec.Code != http.StatusSeeOther { + t.Fatalf("request %d: %d", i+1, rec.Code) + } + } + if rec := postForm(m.HandleReset, "/login/reset", "192.0.2.9:1", url.Values{"username": {"x"}}); rec.Code != http.StatusTooManyRequests { + t.Errorf("the sixth request: %d", rec.Code) + } +} + +var confirmLinkRe = regexp.MustCompile(`https://mail\.example\.org/confirm/([A-Za-z0-9_-]{43})\n`) + +// The confirmation link's page changes nothing; its button confirms the +// address, once - and only for its own account's session: a mistyped address +// is a stranger's, and the mail does not name the account. +func TestConfirmLink(t *testing.T) { + m, ml := linkModule(t, false) + u, _ := m.store.GetUserByUsername("admin") + if err := m.SendConfirmation(u); err != nil { + t.Fatal(err) + } + got, ok := ml.next(t) + if !ok || got.to != "admin@example.org" || got.subject != "[SelfPost] Confirm your e-mail" || !confirmLinkRe.MatchString(got.body) { + t.Fatalf("mail = %+v %v", got, ok) + } + if strings.Contains(got.body, "admin ") || strings.Contains(got.body, "account admin") { + t.Errorf("the confirmation mail names the account:\n%s", got.body) + } + path := "/confirm/" + confirmLinkRe.FindStringSubmatch(got.body)[1] + // Without a session, or with another account's, the link asks to sign in + // and is not used up. + mustCreate(t, m.sessions, "other") + other := mustCreate(t, m.sessions, "other") + for name, session := range map[string]string{"no session": "", "another account": other} { + if rec := getAs(m.HandleConfirm, path, session); rec.Code != http.StatusForbidden || !strings.Contains(rec.Body.String(), `href="/login"`) { + t.Errorf("%s: GET %d", name, rec.Code) + } + if rec := postAs(m.HandleConfirm, path, session); rec.Code != http.StatusForbidden { + t.Errorf("%s: POST %d", name, rec.Code) + } + } + session := mustCreate(t, m.sessions, "admin") + page := getAs(m.HandleConfirm, path, session) + if page.Code != http.StatusOK || !strings.Contains(page.Body.String(), `action="`+path+`"`) || + !strings.Contains(page.Body.String(), "admin@example.org") { + t.Fatalf("the link's page: %d\n%s", page.Code, page.Body.String()) + } + if u, _ = m.store.GetUser(u.ID); !u.EmailConfirmed.IsZero() { + t.Fatal("opening the link confirmed the address") + } + if rec := postAs(m.HandleConfirm, path, session); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "E-mail confirmed") { + t.Fatalf("confirm: %d\n%s", rec.Code, rec.Body.String()) + } + if u, _ = m.store.GetUser(u.ID); u.EmailConfirmed.IsZero() { + t.Fatal("the button did not confirm the address") + } + if rec := postAs(m.HandleConfirm, path, session); rec.Code != http.StatusNotFound { + t.Errorf("the link worked twice: %d", rec.Code) + } +} + +// A second request within resetSpacing sends nothing and leaves the first +// link working: a stranger repeating a username neither kills the link in +// the mailbox nor spends the account's links. +func TestResetRequestsAreSpaced(t *testing.T) { + m, ml := linkModule(t, true) + first := askReset(t, m, ml) + postForm(m.HandleReset, "/login/reset", "192.0.2.2:1234", url.Values{"username": {"admin"}}) + if got, sent := ml.next(t); sent { + t.Fatalf("a second link went within %s: %+v", resetSpacing, got) + } + if rec := get(m.HandleResetLink, "/login/reset/"+first); rec.Code != http.StatusOK { + t.Errorf("the first link died: %d", rec.Code) + } +} + +// A link path that is not one is not found. +func TestLinkPathsWithoutAToken(t *testing.T) { + m, _ := linkModule(t, true) + for _, p := range []string{"/login/reset/", "/login/reset/a/b", "/confirm/"} { + h := m.HandleResetLink + if strings.HasPrefix(p, "/confirm/") { + h = m.HandleConfirm + } + if rec := get(h, p); rec.Code != http.StatusNotFound { + t.Errorf("%s: %d", p, rec.Code) + } + } +} diff --git a/internal/web/handlers/handlers_account.go b/internal/web/handlers/handlers_account.go index 736a23b..964de1d 100644 --- a/internal/web/handlers/handlers_account.go +++ b/internal/web/handlers/handlers_account.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" "strings" + "time" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" @@ -60,6 +61,11 @@ func (h *Handlers) HandleAccount(w http.ResponseWriter, r *http.Request) { func (h *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, status int, u store.User, f accountForm) { page := view.NewAccount(h.shellMeta(r), string(u.Role)) page.Username, page.Email, page.Notify = f.Username, f.Email, f.Notify + var sent time.Time + if l, err := h.store.LatestUserLink(u.ID, store.LinkConfirm, time.Now()); err == nil && l.Email == u.Email { + sent = l.CreatedAt + } + page.WithEmailState(u.Email, !u.EmailConfirmed.IsZero(), sent) page.WithResult(accountFlash(r), f.Err) h.view.Render(w, status, "account", page) } @@ -68,6 +74,10 @@ func accountFlash(r *http.Request) string { switch r.URL.Query().Get("done") { case "profile": return "Profile saved." + case "profile-link": + return "Profile saved. A link to confirm the new address is on its way." + case "link": + return "A new link is on its way." case "password": return "Password changed. Any other signed-in sessions were signed out." } @@ -108,6 +118,25 @@ func (h *Handlers) HandleAccountProfile(w http.ResponseWriter, r *http.Request) fail(http.StatusBadRequest, err.Error()) return } + // The e-mail is where a reset link goes once it is confirmed, so + // changing it asks for the password, as changing the password does + // (roadmap password-reset; 2.0 security review), on the same limit. + emailChanged := f.Email != u.Email + if emailChanged { + if !h.auth.AllowPasswordChange(u.ID) { + fail(http.StatusTooManyRequests, "Too many attempts. Please wait and try again.") + return + } + current := r.PostFormValue("current_password") + if current == "" { + fail(http.StatusUnauthorized, "Your current password is needed to change the e-mail.") + return + } + if err := bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(current)); err != nil { + fail(http.StatusUnauthorized, "Current password is incorrect.") + return + } + } if err := h.store.UpdateUser(u.ID, f.Username, u.PasswordHash, f.Email); err != nil { if errors.Is(err, store.ErrUserExists) { fail(http.StatusConflict, "That username is already in use.") @@ -125,9 +154,54 @@ func (h *Handlers) HandleAccountProfile(w http.ResponseWriter, r *http.Request) } } logf("panel: user %d profile updated (username: %t, e-mail: %t, notifications: %t)", u.ID, f.Username != u.Username, f.Email != u.Email, f.Notify) + if emailChanged && f.Email != "" && h.sendConfirmation(u.ID) { + http.Redirect(w, r, "/account?done=profile-link", http.StatusSeeOther) + return + } http.Redirect(w, r, "/account?done=profile", http.StatusSeeOther) } +// HandleAccountConfirmLink sends the account's unconfirmed address its link +// again (Account, "Send the link again"). +func (h *Handlers) HandleAccountConfirmLink(w http.ResponseWriter, r *http.Request) { + u, ok := h.accountUser(w, r) + if !ok { + return + } + if u.Email == "" || !u.EmailConfirmed.IsZero() { + http.Redirect(w, r, "/account", http.StatusSeeOther) + return + } + f := formOf(u) + if !h.auth.AllowConfirmationLink(u.ID) { + f.Err = "Too many links asked for. Please wait and try again." + h.renderAccount(w, r, http.StatusTooManyRequests, u, f) + return + } + h.auth.SendConfirmationLater(u) + http.Redirect(w, r, "/account?done=link", http.StatusSeeOther) +} + +// sendConfirmation mails the user's unconfirmed address its link after the +// answer, within the limit of links to one address; whether one was sent off. +// A link that cannot go is logged: the address is saved all the same, and +// Account offers to send it again. +func (h *Handlers) sendConfirmation(userID int64) bool { + if h.auth == nil || !h.auth.CanMail() { + return false + } + u, err := h.store.GetUser(userID) + if err != nil || u.Email == "" || !u.EmailConfirmed.IsZero() { + return false + } + if !h.auth.AllowConfirmationLink(u.ID) { + logf("panel: user %d: confirmation links asked for too often; none was sent", u.ID) + return false + } + h.auth.SendConfirmationLater(u) + return true +} + // HandleAccountPassword changes the password. It asks for the current one and // is rate-limited like a sign-in: the form is a place to guess a password from // an unattended session. @@ -178,6 +252,10 @@ func (h *Handlers) HandleAccountPassword(w http.ResponseWriter, r *http.Request) if token, ok := h.auth.SessionToken(r); ok { h.auth.DestroyOtherSessions(u.ID, token) } + // A password set another way leaves no reset link behind it. + if err := h.store.DeleteUserLinks(u.ID, store.LinkReset); err != nil { + logf("panel: account: %v", err) + } logf("panel: user %d changed their password", u.ID) http.Redirect(w, r, "/account?done=password", http.StatusSeeOther) } diff --git a/internal/web/handlers/handlers_account_test.go b/internal/web/handlers/handlers_account_test.go index 98bc80b..bb6b23e 100644 --- a/internal/web/handlers/handlers_account_test.go +++ b/internal/web/handlers/handlers_account_test.go @@ -1,13 +1,17 @@ package handlers import ( + "context" "net/http" "net/http/httptest" "net/url" + "path/filepath" "strings" "testing" + "time" "github.com/mixeme/selfpost/internal/store" + "github.com/mixeme/selfpost/internal/web/auth" "golang.org/x/crypto/bcrypt" ) @@ -32,10 +36,11 @@ func TestAccountPageShowsTwoForms(t *testing.T) { } } -// The profile form saves the name and the e-mail without asking for the -// password and without changing the password or any domain's report address. +// The profile form saves the name and the e-mail - a new e-mail with the +// current password - without changing the password or any domain's report +// address. func TestAccountProfile(t *testing.T) { - h, _ := settingsServer(t) + h, password := settingsServer(t) before, _ := h.store.GetUser(globalPrincipal.ID) d, _ := h.store.AddDomain("example.com", "mail") if err := h.store.SetDomainDMARCAddress(d.ID, "reports@hub.example"); err != nil { @@ -43,7 +48,7 @@ func TestAccountProfile(t *testing.T) { } rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, - url.Values{"username": {"operator"}, "email": {"mix@example.org"}}) + url.Values{"username": {"operator"}, "email": {"mix@example.org"}, "current_password": {password}}) if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/account?done=profile" { t.Fatalf("POST /account/profile = %d to %q:\n%s", rec.Code, rec.Header().Get("Location"), rec.Body.String()) } @@ -70,12 +75,91 @@ func TestAccountProfile(t *testing.T) { if _, err := h.store.CreateUser("taken", "h", store.RoleGlobal, store.Reach{}); err != nil { t.Fatal(err) } - rec = postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, url.Values{"username": {"taken"}, "email": {"other@example.org"}}) + rec = postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, url.Values{"username": {"taken"}, "email": {"other@example.org"}, "current_password": {password}}) if u, _ = h.store.GetUser(globalPrincipal.ID); rec.Code != http.StatusConflict || u.Username != "operator" || u.Email != "mix@example.org" { t.Errorf("a taken username = %d, profile now %q / %q", rec.Code, u.Username, u.Email) } } +// linkMailer keeps the links the panel would have mailed. +type linkMailer struct{ sent []string } + +func (r *linkMailer) Send(to, subject, body string) error { + r.sent = append(r.sent, to+" | "+subject+" | "+body) + return nil +} + +// sentBy is what ml has been given once h's links sent after their answers +// have gone. +func sentBy(h *Handlers, ml *linkMailer) []string { + h.auth.Wait(context.Background()) + return ml.sent +} + +// withMail gives h an auth module that mails, as mail.example.org, to a +// linkMailer. +func withMail(t *testing.T, h *Handlers) *linkMailer { + t.Helper() + h.auth = auth.New(h.store, auth.Config{Hostname: "mail.example.org"}, h.view, filepath.Join(t.TempDir(), "setup-token")) + ml := &linkMailer{} + h.auth.SetMailer(ml) + return ml +} + +// The e-mail is where a reset link goes once confirmed, so changing it asks +// for the password; the new address is sent its link and shows as not +// confirmed, with Send the link again; a confirmed address shows as such and +// keeps its mark when only the name changes. +func TestAccountEmailIsConfirmedByItsLink(t *testing.T) { + h, password := settingsServer(t) + ml := withMail(t, h) + change := url.Values{"username": {"admin"}, "email": {"mix@example.org"}} + for name, pw := range map[string]string{"no password": "", "wrong password": "nope"} { + form := url.Values{"username": change["username"], "email": change["email"], "current_password": {pw}} + if rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, form); rec.Code != http.StatusUnauthorized { + t.Errorf("%s: %d", name, rec.Code) + } + } + if u, _ := h.store.GetUser(globalPrincipal.ID); u.Email != "" || len(sentBy(h, ml)) != 0 { + t.Fatalf("a refused change saved %q, sent %d", u.Email, len(sentBy(h, ml))) + } + change.Set("current_password", password) + rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, change) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/account?done=profile-link" { + t.Fatalf("change: %d %q\n%s", rec.Code, rec.Header().Get("Location"), rec.Body.String()) + } + if len(sentBy(h, ml)) != 1 || !strings.HasPrefix(sentBy(h, ml)[0], "mix@example.org | [SelfPost] Confirm your e-mail | ") || + !strings.Contains(sentBy(h, ml)[0], "https://mail.example.org/confirm/") { + t.Fatalf("sent = %q", sentBy(h, ml)) + } + out := getBody(t, h.HandleAccount, "/account") + for _, want := range []string{`not confirmed`, `action="/account/email/confirm"`, `A link was sent to`} { + if !strings.Contains(out, want) { + t.Errorf("account page missing %q", want) + } + } + // Send the link again: a second link. + if rec := postFormAs(h.HandleAccountConfirmLink, globalPrincipal, "/account/email/confirm", nil, nil); rec.Code != http.StatusSeeOther || len(sentBy(h, ml)) != 2 { + t.Fatalf("send again: %d, sent %d", rec.Code, len(sentBy(h, ml))) + } + // Confirmed: the tag says so, the form is gone, and a new name keeps it + // without the password. + if ok, _ := h.store.ConfirmEmail(globalPrincipal.ID, "mix@example.org", time.Now()); !ok { + t.Fatal("confirm") + } + out = getBody(t, h.HandleAccount, "/account") + if !strings.Contains(out, `confirmed`) || strings.Contains(out, `action="/account/email/confirm"`) { + t.Errorf("a confirmed address:\n%s", out) + } + if rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, + url.Values{"username": {"operator"}, "email": {"mix@example.org"}}); rec.Code != http.StatusSeeOther { + t.Fatalf("a new name: %d", rec.Code) + } + if u, _ := h.store.GetUser(globalPrincipal.ID); u.EmailConfirmed.IsZero() || len(sentBy(h, ml)) != 2 { + t.Errorf("a new name: confirmed %v, sent %d", u.EmailConfirmed, len(sentBy(h, ml))) + } +} + func TestAccountPassword(t *testing.T) { h, password := settingsServer(t) const next = "another-long-password-2" @@ -110,7 +194,12 @@ func TestAccountIsOpenToADomainAdministrator(t *testing.T) { h, _ := settingsServer(t) d, _ := h.store.AddDomain("example.com", "mail") p := domainAdmin(t, h.store, "ops", d.ID) - rec := postFormAs(h.HandleAccountProfile, p, "/account/profile", nil, url.Values{"username": {"ops"}, "email": {"ops@example.org"}}) + // A new e-mail asks for the password: the administrator's is set here. + hash, _ := bcrypt.GenerateFromPassword([]byte("ops-password-1234"), bcrypt.MinCost) + if err := h.store.SetUserPassword(p.ID, string(hash)); err != nil { + t.Fatal(err) + } + rec := postFormAs(h.HandleAccountProfile, p, "/account/profile", nil, url.Values{"username": {"ops"}, "email": {"ops@example.org"}, "current_password": {"ops-password-1234"}}) if rec.Code != http.StatusSeeOther { t.Fatalf("POST /account/profile as a domain administrator = %d:\n%s", rec.Code, rec.Body.String()) } diff --git a/internal/web/handlers/handlers_links_test.go b/internal/web/handlers/handlers_links_test.go new file mode 100644 index 0000000..e1e246d --- /dev/null +++ b/internal/web/handlers/handlers_links_test.go @@ -0,0 +1,56 @@ +package handlers + +import ( + "net/http" + "net/url" + "strings" + "testing" + "time" + + "github.com/mixeme/selfpost/internal/store" +) + +// An address the global administrator types for a user is unconfirmed until +// that user opens its link: creating the user with one, or saving a new one, +// sends it; the form tags it either way. +func TestUserFormSendsANewAddressItsLink(t *testing.T) { + h, st := inboundHandlers(t) + ml := withMail(t, h) + out1, _ := st.AddDomain("one.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + + rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ + "username": {"ops"}, "email": {"ops@acme.io"}, "password": {"a-long-enough-password-1"}, "role": {"domain"}, + "domain_ids": {idStr(out1.ID)}, + }) + if rec.Code != http.StatusSeeOther { + t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String()) + } + if len(sentBy(h, ml)) != 1 || !strings.HasPrefix(sentBy(h, ml)[0], "ops@acme.io | [SelfPost] Confirm your e-mail | ") { + t.Fatalf("sent = %q", sentBy(h, ml)) + } + u, _ := st.GetUserByUsername("ops") + path, pv := "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)} + if out := send(h.HandleUserEdit, &root, "GET", path, pv, nil).Body.String(); !strings.Contains(out, `not confirmed`) { + t.Errorf("an unconfirmed address is not tagged:\n%s", out) + } + if ok, _ := st.ConfirmEmail(u.ID, "ops@acme.io", time.Now()); !ok { + t.Fatal("confirm") + } + if out := send(h.HandleUserEdit, &root, "GET", path, pv, nil).Body.String(); !strings.Contains(out, `confirmed`) { + t.Errorf("a confirmed address is not tagged:\n%s", out) + } + // The same address saved again keeps its mark and sends nothing; a new + // one loses it and is sent its link. + form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "email": {"ops@acme.io"}} + if rec := send(h.HandleUserEdit, &root, "POST", path, pv, form); rec.Code != http.StatusSeeOther || len(sentBy(h, ml)) != 1 { + t.Fatalf("the same address: %d, sent %d", rec.Code, len(sentBy(h, ml))) + } + form.Set("email", "new@acme.io") + if rec := send(h.HandleUserEdit, &root, "POST", path, pv, form); rec.Code != http.StatusSeeOther { + t.Fatalf("a new address: %d", rec.Code) + } + if got, _ := st.GetUser(u.ID); !got.EmailConfirmed.IsZero() || len(sentBy(h, ml)) != 2 || !strings.HasPrefix(sentBy(h, ml)[1], "new@acme.io | ") { + t.Errorf("a new address: confirmed %v, sent %q", got.EmailConfirmed, sentBy(h, ml)) + } +} diff --git a/internal/web/handlers/handlers_notifications_test.go b/internal/web/handlers/handlers_notifications_test.go index a06d52f..76b5651 100644 --- a/internal/web/handlers/handlers_notifications_test.go +++ b/internal/web/handlers/handlers_notifications_test.go @@ -26,14 +26,14 @@ const notifyHost = "mail.example.org" // stored, Save profile writes it, and an unchecked box is a form without the // field, which means off. Saving it changes nothing else of the profile. func TestAccountNotifyCheckbox(t *testing.T) { - h, _ := settingsServer(t) + h, password := settingsServer(t) notify := func() bool { u, _ := h.store.GetUser(globalPrincipal.ID) return u.Notify } save := func(extra url.Values) { t.Helper() - form := url.Values{"username": {"admin"}, "email": {"mix@example.org"}} + form := url.Values{"username": {"admin"}, "email": {"mix@example.org"}, "current_password": {password}} for k, v := range extra { form[k] = v } diff --git a/internal/web/handlers/handlers_users.go b/internal/web/handlers/handlers_users.go index 8989ef9..e75e12f 100644 --- a/internal/web/handlers/handlers_users.go +++ b/internal/web/handlers/handlers_users.go @@ -153,7 +153,13 @@ func (h *Handlers) renderUserForm(w http.ResponseWriter, r *http.Request, status in.InDomains = append(in.InDomains, view.DomainChoice{ID: d.ID, Name: d.Name, Checked: inb[d.ID]}) } } - h.view.Render(w, status, "user", view.NewUserForm(h.shellMeta(r), in)) + page := view.NewUserForm(h.shellMeta(r), in) + if userID != 0 { + if u, err := h.store.GetUser(userID); err == nil { + page.WithEmailState(u.Email, !u.EmailConfirmed.IsZero()) + } + } + h.view.Render(w, status, "user", page) } // lastGlobalLocked is whether the form's user is the only global one, whose role @@ -206,7 +212,8 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) { } // The user, the e-mail and the reach in one transaction: a failure leaves // no user rather than half of one (audit 2026-10-12, M9). - if _, err := h.store.AddUser(store.UserChange{Username: username, PasswordHash: string(hash), Email: email, Role: role, Reach: reach}); err != nil { + id, err := h.store.AddUser(store.UserChange{Username: username, PasswordHash: string(hash), Email: email, Role: role, Reach: reach}) + if err != nil { if errors.Is(err, store.ErrUserExists) { h.renderUserForm(w, r, http.StatusConflict, 0, userFormView{FormErr: "That username is already in use.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) return @@ -219,6 +226,11 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) { h.renderUserForm(w, r, http.StatusInternalServerError, 0, userFormView{FormErr: "Could not create user. Please check the logs."}) return } + // An address typed for a user is unconfirmed until that user opens its + // link (roadmap password-reset). + if email != "" { + h.sendConfirmation(id) + } http.Redirect(w, r, "/server/users?done=created", http.StatusSeeOther) } @@ -316,6 +328,10 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st } if password != "" { h.endSessionsAfterPasswordSet(r, p, u) + h.endResetLinks(u.ID) + } + if email != "" && email != u.Email { + h.sendConfirmation(u.ID) } http.Redirect(w, r, "/server/users?done=updated", http.StatusSeeOther) } @@ -333,6 +349,14 @@ func (h *Handlers) endSessionsAfterPasswordSet(r *http.Request, by auth.Principa h.auth.DestroyUserSessions(u.ID) } +// endResetLinks removes the reset links of a user whose password was just set +// on the user form: a password set another way leaves none behind it. +func (h *Handlers) endResetLinks(userID int64) { + if err := h.store.DeleteUserLinks(userID, store.LinkReset); err != nil { + logf("panel: user form: %v", err) + } +} + // HandleUserDeleteConfirm shows the cascade warning before a panel user is // removed — the same pattern as HandleDeleteConfirm for domains, so a single // mis-click on Delete cannot remove a user (P3, code-review.md). diff --git a/internal/web/loggedpath_test.go b/internal/web/loggedpath_test.go new file mode 100644 index 0000000..e86192f --- /dev/null +++ b/internal/web/loggedpath_test.go @@ -0,0 +1,19 @@ +package web + +import "testing" + +// A rejected request's path is logged without the token of a link +// (security review, 2026-10-17). +func TestLoggedPathHidesTokens(t *testing.T) { + for in, want := range map[string]string{ + "/login/reset/abcDEF123": "/login/reset/…", + "/confirm/abcDEF123": "/confirm/…", + "/login/reset": "/login/reset", + "/confirm/": "/confirm/", + "/account/profile": "/account/profile", + } { + if got := loggedPath(in); got != want { + t.Errorf("loggedPath(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/web/security.go b/internal/web/security.go index 977fa26..3176ee4 100644 --- a/internal/web/security.go +++ b/internal/web/security.go @@ -3,6 +3,7 @@ package web import ( "net/http" "net/url" + "strings" ) // contentSecurityPolicy is the panel's CSP. Everything the pages load — @@ -96,7 +97,7 @@ func (s *Server) secure(next http.Handler) http.Handler { // saving anything". logf("panel: rejected %s %s: cross-origin request (Sec-Fetch-Site=%q Origin=%q Host=%q); "+ "if this was a legitimate request, check that the reverse proxy passes the original Host header through", - r.Method, r.URL.Path, r.Header.Get("Sec-Fetch-Site"), r.Header.Get("Origin"), r.Host) + r.Method, loggedPath(r.URL.Path), r.Header.Get("Sec-Fetch-Site"), r.Header.Get("Origin"), r.Host) http.Error(w, "cross-origin request rejected", http.StatusForbidden) return } @@ -168,3 +169,15 @@ func originAllowed(r *http.Request) bool { } return u.Host == r.Host } + +// loggedPath is a request's path as the log may hold it: the token of a +// password-reset link is a secret, and the log is read by more than the +// person the link went to (security review, 2026-10-17). +func loggedPath(p string) string { + for _, prefix := range []string{"/login/reset/", "/confirm/"} { + if strings.HasPrefix(p, prefix) && len(p) > len(prefix) { + return prefix + "…" + } + } + return p +} diff --git a/internal/web/view/account.go b/internal/web/view/account.go index 272d983..fc52aac 100644 --- a/internal/web/view/account.go +++ b/internal/web/view/account.go @@ -27,10 +27,41 @@ type Account struct { // Notify is whether the panel writes to this user: the checkbox under the // e-mail. Notify bool + // EmailTag says whether the stored address is confirmed (nil without + // one); Resend is the form that sends its link again, nil once it is + // confirmed (password-reset). + EmailTag *Tag + Resend *ResendLink Password Box } +// ResendLink is Account's "Send the link again": the address and when its +// live link went ("" for none). +type ResendLink struct { + Email string + Sent string +} + +// WithEmailState sets the stored address's tag and, while it is not +// confirmed, the resend form: sentAt is when its live link went, zero for +// none. +func (a *Account) WithEmailState(email string, confirmed bool, sentAt time.Time) *Account { + if email == "" { + return a + } + if confirmed { + a.EmailTag = &Tag{Status: "ok", Label: "confirmed"} + return a + } + a.EmailTag = &Tag{Status: "warn", Label: "not confirmed"} + a.Resend = &ResendLink{Email: email} + if !sentAt.IsZero() { + a.Resend.Sent = sentAt.Local().Format("15:04") + } + return a +} + // NewAccount builds the page for a user. role is "global" or "domain" and // appears in the kicker. func NewAccount(m Meta, role string) *Account { diff --git a/internal/web/view/fixtures_test.go b/internal/web/view/fixtures_test.go index 602acbb..6cd8184 100644 --- a/internal/web/view/fixtures_test.go +++ b/internal/web/view/fixtures_test.go @@ -21,13 +21,20 @@ import ( // This file belongs to the implementation, not to the guards: the step that // restyles a page adds its fixture here in the same commit. var pageFixtures = map[string]func() any{ - "components": func() any { return KitPage() }, - "login": func() any { return NewLogin("mail.example.org", "") }, - "setup": func() any { return NewSetup("0123456789abcdef", "") }, - "overview": func() any { return overviewFixture() }, - "health": func() any { return healthFixture(false) }, - "account": func() any { return accountFixture() }, - "settings": func() any { return settingsFixture() }, + "components": func() any { return KitPage() }, + "login": func() any { return NewLogin("mail.example.org", "") }, + "setup": func() any { return NewSetup("0123456789abcdef", "") }, + "login-reset": func() any { return NewReset("mail.example.org", "") }, + "login-reset-set": func() any { + return NewResetSet("mail.example.org", "admin", "0123456789abcdef", "") + }, + "confirm-email": func() any { + return NewConfirm("mail.example.org", "admin", "mix@example.org", "0123456789abcdef") + }, + "overview": func() any { return overviewFixture() }, + "health": func() any { return healthFixture(false) }, + "account": func() any { return accountFixture() }, + "settings": func() any { return settingsFixture() }, "out-domains": func() any { return outDomainsFixture() }, "out-domain": func() any { return outDomainFixture() }, @@ -141,6 +148,8 @@ func healthFixture(refresh bool) *Health { func accountFixture() *Account { a := NewAccount(admin(), "global") a.Username, a.Email, a.Notify = "admin", "mix@example.org", true + // The mockup's address: not confirmed, its link sent at 14:02. + a.WithEmailState("mix@example.org", false, time.Date(2026, 10, 17, 14, 2, 0, 0, time.UTC)) return a } @@ -438,7 +447,7 @@ func userFixture() *UserForm { AllOut: true, OutDomains: []DomainChoice{{ID: 1, Name: "example.org"}, {ID: 2, Name: "shop.example.org"}, {ID: 3, Name: "notify.acme.io"}}, InDomains: []DomainChoice{{ID: 1, Name: "lists.example.org"}, {ID: 2, Name: "acme.io", Checked: true}}, - }) + }).WithEmailState("ops@acme.io", true) } // userDeleteFixture is the mockup's confirmation: acme-ops, who has all diff --git a/internal/web/view/help.go b/internal/web/view/help.go index baf7644..79b19ca 100644 --- a/internal/web/view/help.go +++ b/internal/web/view/help.go @@ -305,6 +305,7 @@ var serverTopics = []HelpTopic{ var accountTopics = []HelpTopic{ {ID: HelpAccount, Title: "Your account", Body: []Text{ - Plain("Account is your own, whatever your role: your username and e-mail, whether the panel sends you notifications, and your password. Changing the password signs out every other session of this account and keeps the one you are using; applications have their own logins and are not affected."), + Rich("Account is your own, whatever your role: your username and e-mail, whether the panel sends you notifications, and your password. Changing the password signs out every other session of this account and keeps the one you are using; applications have their own logins and are not affected. A new e-mail is sent a link that confirms it, which works once, for 24 hours; changing the e-mail asks for your password. Notifications go to the address whether it is confirmed or not, but only a confirmed one can get a link to reset a forgotten password: ", + Em("Forgot your password?"), " on the sign-in page sends it, for 30 minutes and once, and setting the new password signs every session of the account out. Without a confirmed e-mail the global administrator sets a new password under Users, and the only global administrator recovers from the container."), }}, } diff --git a/internal/web/view/pages_test.go b/internal/web/view/pages_test.go index c5ace53..475508b 100644 --- a/internal/web/view/pages_test.go +++ b/internal/web/view/pages_test.go @@ -290,14 +290,18 @@ func TestAccountHasTwoFormsEachItsOwnPost(t *testing.T) { `

Account

`, `Signed in as admin · global`, `action="/account/profile"`, `name="username"`, `name="email"`, `value="mix@example.org"`, `action="/account/password"`, `name="current_password"`, `name="new_password"`, `name="new_password_confirm"`, - `Your own address. A domain's report address can be filled with it from Domain settings.`, + `Your own address. Saving a new one sends it a link to confirm it; once confirmed it is where a link to reset your password can go. A domain's report address can be filled with it from Domain settings.`, + `E-mail not confirmed`, + ``, + `action="/account/email/confirm"`, `A link was sent to mix@example.org at 14:02; it works once, for 24 hours.`, ) // Account is Profile and Password; where a domain's reports go is the // domain's own setting. pageLacks(t, "Account", out, `send_log_retention_days`, `/account/dmarc`, `dmarc_default`, `

DMARC reports

`, `Report authorization`, `default report address`, `id="dmarc"`) - // No form asks for another's fields: two forms, two submit buttons. - if got := strings.Count(out, `
acme-ops`, `href="/server/users">Users`, `href="/server/users/3/delete"`, ``, - `name="username" value="acme-ops"`, ``, + `name="username" value="acme-ops"`, ``, ``, `name="password" type="password" autocomplete="new-password">`, `Leave empty to keep the current one.`, @@ -316,8 +316,9 @@ func TestEveryHelpLinkLandsOnATopic(t *testing.T) { // change it under Account, and it can receive DMARC reports if they choose. func TestUserFormEmailHelpSaysOnlyWhatIsTrue(t *testing.T) { out := renderSignedIn(t, "user", userFixture()) - pageHas(t, "User", out, `The user's own address. They can change it under Account.`) - pageLacks(t, "User", out, `notification`, `reset`) + pageHas(t, "User", out, `The user's own address. They can change it under Account. A new one is sent a link to confirm it; until it is confirmed no reset link goes to it.`, + `E-mail confirmed`) + pageLacks(t, "User", out, `notification`) } // A domain's report address belongs to the domain, not to a user: deleting a diff --git a/internal/web/view/signin.go b/internal/web/view/signin.go index 6775666..232e11d 100644 --- a/internal/web/view/signin.go +++ b/internal/web/view/signin.go @@ -16,6 +16,9 @@ type Login struct { Host string SetupHint bool Error string + // Info is the result of what brought the person back here - a reset link + // asked for, a password set from one - shown as a success flash. + Info string } // NewLogin returns the sign-in page's data. host and formErr may be empty. @@ -33,6 +36,94 @@ func NewLoginSetupHint(host string) *Login { // Error is set. func (l *Login) Notice() Flash { return Flash{Error: true, Text: Plain(l.Error)} } +// WithInfo sets the success flash. +func (l *Login) WithInfo(info string) *Login { + l.Info = info + return l +} + +// InfoNotice is Info as the flash partial's input. +func (l *Login) InfoNotice() Flash { return Flash{Text: Plain(l.Info)} } + +// Reset is the page that asks for a reset link (templates/login_reset.html, +// mockup login-reset.html): a username, and what will happen. Error is a +// refusal, or a link that no longer works. +type Reset struct { + Meta + Host string + Error string +} + +// NewReset returns the reset request page's data. +func NewReset(host, formErr string) *Reset { + return &Reset{Meta: Meta{Title: "Reset password"}, Host: host, Error: formErr} +} + +// Notice is the page's error as the flash partial's input. +func (p *Reset) Notice() Flash { return Flash{Error: true, Text: Plain(p.Error)} } + +// ResetSet is the page a reset link opens (templates/login_reset_set.html, +// mockup login-reset-set.html): the new password, for User, posted back to +// the link's own path. +type ResetSet struct { + Meta + Host string + User string + Token string + Error string +} + +// NewResetSet returns the new-password page's data. +func NewResetSet(host, user, token, formErr string) *ResetSet { + return &ResetSet{Meta: Meta{Title: "New password"}, Host: host, User: user, Token: token, Error: formErr} +} + +// Kicker is the host and the account the password is for. +func (p *ResetSet) Kicker() string { return kickerOf(p.Host, p.User) } + +// Notice is the page's error as the flash partial's input. +func (p *ResetSet) Notice() Flash { return Flash{Error: true, Text: Plain(p.Error)} } + +// Confirm is the page a confirmation link opens (templates/confirm_email.html, +// mockup confirm-email.html): the address and a button - a mail scanner +// opens a link, so the GET changes nothing. Done is the page after the +// button; Error a link that does not work, with no button. +type Confirm struct { + Meta + Host string + User string + Email string + Token string + Error string + Done bool + // SignIn is the error of a link opened without its account's session: + // the page points at sign-in. + SignIn bool +} + +// NewConfirm returns the confirmation page's data. +func NewConfirm(host, user, email, token string) *Confirm { + return &Confirm{Meta: Meta{Title: "Confirm e-mail"}, Host: host, User: user, Email: email, Token: token} +} + +// Kicker is the host and the account the address is of. +func (p *Confirm) Kicker() string { return kickerOf(p.Host, p.User) } + +// Notice is the page's error as the flash partial's input. +func (p *Confirm) Notice() Flash { return Flash{Error: true, Text: Plain(p.Error)} } + +// kickerOf joins the host and an account's name, either of which may be +// unknown. +func kickerOf(host, user string) string { + switch { + case host == "": + return user + case user == "": + return host + } + return host + " · " + user +} + // Setup is the data of the first-run page that creates the administrator // (templates/setup.html), reached by the one-time link /setup/. Token is // the path segment the form posts back to; Error is the refusal of the last diff --git a/internal/web/view/templates/account.html b/internal/web/view/templates/account.html index 7cb3176..dacc350 100644 --- a/internal/web/view/templates/account.html +++ b/internal/web/view/templates/account.html @@ -12,12 +12,18 @@ {{template "body_open"}}
-
-

Your own address. A domain's report address can be filled with it from Domain settings.

+
+

Your own address. Saving a new one sends it a link to confirm it; once confirmed it is where a link to reset your password can go. A domain's report address can be filled with it from Domain settings.

+
+

Needed only to change the e-mail.

To this address, when something needs you: a DNS record of a domain stops matching, a rate limit refuses mail, the TLS certificate is about to expire, a message has waited in the queue for three days. What is sent and when

+ {{with .Resend}}
+

{{if .Sent}}A link was sent to {{.Email}} at {{.Sent}}; it works once, for 24 hours.{{else}}No link is waiting: send one to confirm {{.Email}}.{{end}}

+ +
{{end}} {{template "body_close"}} {{template "box_close"}} diff --git a/internal/web/view/templates/confirm_email.html b/internal/web/view/templates/confirm_email.html new file mode 100644 index 0000000..bc69f9d --- /dev/null +++ b/internal/web/view/templates/confirm_email.html @@ -0,0 +1,28 @@ +{{/* The page a confirmation link opens (mockup confirm-email.html): the + address and a button, since a mail scanner opens links and a GET must + change nothing. After the button, the same column says it is done; a link + that does not work says so and has no button. */}} +{{define "content"}} +{{if .Done}} +
+ {{with .Kicker}}

{{template "kicker" .}}

{{end}} +

E-mail confirmed

+

{{.Email}} is the confirmed address of this account.

+

Account

+
+{{else if .Error}} +
+ {{with .Kicker}}

{{template "kicker" .}}

{{end}} +

Confirm e-mail

+ {{template "flash" .Notice}} + {{if .SignIn}}

Sign in

{{else}}

Account

{{end}} +
+{{else}} +
+ {{with .Kicker}}

{{template "kicker" .}}

{{end}} +

Confirm e-mail

+

{{.Email}} becomes the confirmed address of this account: where a link to reset its password can be sent.

+
+
+{{end}} +{{end}} diff --git a/internal/web/view/templates/login.html b/internal/web/view/templates/login.html index b99de1d..810ec62 100644 --- a/internal/web/view/templates/login.html +++ b/internal/web/view/templates/login.html @@ -14,7 +14,7 @@
{{if .Host}}

{{template "kicker" .Host}}

{{end}}

Sign in

- {{if .Error}}{{template "flash" .Notice}}{{end}} + {{if .Error}}{{template "flash" .Notice}}{{else if .Info}}{{template "flash" .InfoNotice}}{{end}}
@@ -24,6 +24,7 @@
+

Forgot your password?

{{end}} {{end}} diff --git a/internal/web/view/templates/login_reset.html b/internal/web/view/templates/login_reset.html new file mode 100644 index 0000000..d23efb6 --- /dev/null +++ b/internal/web/view/templates/login_reset.html @@ -0,0 +1,17 @@ +{{/* Asking for a reset link (mockup login-reset.html): the form column of the + signed-out screen. The answer to the form is the sign-in page, the same + for any name (mockup login-reset-sent.html). */}} +{{define "content"}} +
+ {{if .Host}}

{{template "kicker" .Host}}

{{end}} +

Reset password

+

A link to set a new password goes to the e-mail of the account, once that address is confirmed under Account. It works once, for 30 minutes.

+ {{if .Error}}{{template "flash" .Notice}}{{end}} +
+ +
+
+
+

Back to sign in

+
+{{end}} diff --git a/internal/web/view/templates/login_reset_set.html b/internal/web/view/templates/login_reset_set.html new file mode 100644 index 0000000..6605ec6 --- /dev/null +++ b/internal/web/view/templates/login_reset_set.html @@ -0,0 +1,19 @@ +{{/* The page a reset link opens (mockup login-reset-set.html): the new + password, posted back to the link's own path. */}} +{{define "content"}} +
+ {{with .Kicker}}

{{template "kicker" .}}

{{end}} +

New password

+

Setting it signs every session of this account out; sign in with it afterwards.

+ {{if .Error}}{{template "flash" .Notice}}{{end}} +
+ +
+
+
+ +
+
+
+
+{{end}} diff --git a/internal/web/view/templates/user.html b/internal/web/view/templates/user.html index d68f089..27bcf57 100644 --- a/internal/web/view/templates/user.html +++ b/internal/web/view/templates/user.html @@ -15,8 +15,8 @@ {{template "box_open" .Account}} {{template "body_open"}}
-
-

The user's own address. They can change it under Account.

+
+

The user's own address. They can change it under Account. A new one is sent a link to confirm it; until it is confirmed no reset link goes to it.

{{if .NewUser}}At least {{.PasswordMin}} characters.{{else}}Leave empty to keep the current one.{{end}}

diff --git a/internal/web/view/templates_test.go b/internal/web/view/templates_test.go index 1745f49..ac00679 100644 --- a/internal/web/view/templates_test.go +++ b/internal/web/view/templates_test.go @@ -40,7 +40,7 @@ func TestLayoutShowsTheVersionOnlyWhenSignedIn(t *testing.T) { "Source", "No warranty", } - signedOut := map[string]bool{"login": true, "setup": true} + signedOut := map[string]bool{"login": true, "setup": true, "login-reset": true, "login-reset-set": true, "confirm-email": true} for name, fixture := range pageFixtures { rec := httptest.NewRecorder() engine.Render(rec, http.StatusOK, name, fixture()) diff --git a/internal/web/view/view.go b/internal/web/view/view.go index 46ca293..875e8d1 100644 --- a/internal/web/view/view.go +++ b/internal/web/view/view.go @@ -35,6 +35,9 @@ type Engine struct { var pageFiles = map[string][]string{ "setup": {"templates/setup.html"}, "login": {"templates/login.html"}, + "login-reset": {"templates/login_reset.html"}, + "login-reset-set": {"templates/login_reset_set.html"}, + "confirm-email": {"templates/confirm_email.html"}, "overview": {"templates/overview.html", "templates/overview_cards.html"}, "health": {"templates/health.html", "templates/health_body.html"}, "account": {"templates/account.html"}, diff --git a/internal/web/web.go b/internal/web/web.go index 2df582a..50ec8a6 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -4,6 +4,7 @@ package web import ( + "context" "log" "net" "net/http" @@ -17,6 +18,7 @@ import ( "github.com/mixeme/selfpost/internal/health" "github.com/mixeme/selfpost/internal/inbound" "github.com/mixeme/selfpost/internal/legal" + "github.com/mixeme/selfpost/internal/notify" "github.com/mixeme/selfpost/internal/postfix" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/auth" @@ -141,6 +143,7 @@ func New(st *store.Store, domains *domain.Service, apps *app.Service, inboundSvc SessionIdleDays: cfg.SessionIdleDays, TrustedProxyCIDRs: cfg.TrustedProxyCIDRs, }, v, setupTokenPath) + a.SetMailer(notify.NewMailer(cfg.Hostname)) h := handlers.New(st, domains, apps, inboundSvc, lists, dmarcSvc, handlers.Config{ Hostname: cfg.Hostname, SubmissionEnabled: cfg.SubmissionEnabled, @@ -179,6 +182,13 @@ func (s *Server) Start() error { return s.auth.Bootstrap() } +// Drain waits, until ctx ends, for what the panel still does after its +// answers - the links of password-reset being mailed - so the database is not +// closed under them. +func (s *Server) Drain(ctx context.Context) { + s.auth.Wait(ctx) +} + // Handler returns the panel's HTTP handler (router). func (s *Server) Handler() http.Handler { mux, authed := s.muxes() @@ -200,6 +210,10 @@ func (s *Server) muxes() (public, authed *http.ServeMux) { mux.Handle("/static/", view.StaticHandler()) mux.HandleFunc("/setup/", s.auth.HandleSetup) mux.HandleFunc("/login", s.auth.HandleLogin) + // The links of password-reset, opened from a mail, signed in or not. + mux.HandleFunc("/login/reset", s.auth.HandleReset) + mux.HandleFunc("/login/reset/", s.auth.HandleResetLink) + mux.HandleFunc("/confirm/", s.auth.HandleConfirm) mux.HandleFunc("/logout", s.auth.HandleLogout) authed = http.NewServeMux() @@ -213,6 +227,7 @@ func (s *Server) muxes() (public, authed *http.ServeMux) { authed.HandleFunc("GET /account", h.HandleAccount) authed.HandleFunc("POST /account/profile", h.HandleAccountProfile) authed.HandleFunc("POST /account/password", h.HandleAccountPassword) + authed.HandleFunc("POST /account/email/confirm", h.HandleAccountConfirmLink) // Overview. Its handler checks the role itself: the page is global-only // but does not live under /server/. diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index ac9ad08..686233f 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -401,6 +401,10 @@ func TestE2E(t *testing.T) { run("inbound_bare_newline_is_one_message", func(t *testing.T) { testInboundBareNewline(t, sc) }) + // After every step that uses adminPassword: the reset sets another. + run("password_reset_by_e_mail", func(t *testing.T) { + testPasswordReset(t, sc) + }) // Last: it recreates the container in another zone. run("time_zone_is_the_containers", func(t *testing.T) { testTimeZone(t, sc) diff --git a/test/e2e/notifications_test.go b/test/e2e/notifications_test.go index d5e95a4..000642f 100644 --- a/test/e2e/notifications_test.go +++ b/test/e2e/notifications_test.go @@ -40,6 +40,8 @@ func (c *panelClient) setAccountEmail(username, email string) error { "username": {username}, "email": {email}, "notify": {"on"}, + // A new e-mail asks for the password (password-reset). + "current_password": {adminPassword}, }) if err != nil { return err diff --git a/test/e2e/password_reset_test.go b/test/e2e/password_reset_test.go new file mode 100644 index 0000000..fddfe77 --- /dev/null +++ b/test/e2e/password_reset_test.go @@ -0,0 +1,120 @@ +package e2e + +import ( + "net/url" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "time" +) + +// testPasswordReset (password-reset): the whole way, through the sink, as +// the administrator would. The address saved for notifications was sent its +// confirmation link, and the restore of an earlier step deleted it (a +// restore deletes every link), so Account sends it again; opening it changes +// nothing until its button is pressed. Then, signed out, Forgot your +// password? sends a link to that confirmed address; it sets a new password, +// which signs in, and ends every session - the scenario's own included, +// which signs in again with the new one. +func testPasswordReset(t *testing.T, sc *scenario) { + // links are the links of path in the sink's messages to the + // administrator. + links := func(path string) map[string]bool { + t.Helper() + re := regexp.MustCompile(`https://` + regexp.QuoteMeta(selfpostHostname) + `(` + regexp.QuoteMeta(path) + `[A-Za-z0-9_-]{43})\r?\n`) + dir := filepath.Join(h.stageDir, "mail-stage") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + out := map[string]bool{} + for _, e := range entries { + b, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err != nil || !strings.Contains(string(b), "To: <"+adminEmail+">") { + continue + } + for _, m := range re.FindAllStringSubmatch(string(b), -1) { + out[m[1]] = true + } + } + return out + } + // newLink waits for a link of path that before did not have. + newLink := func(path string, before map[string]bool) string { + t.Helper() + var link string + if err := waitFor("a new link to "+path+" in the sink", 60*time.Second, 500*time.Millisecond, func() (bool, error) { + for l := range links(path) { + if !before[l] { + link = l + return true, nil + } + } + return false, nil + }); err != nil { + t.Fatalf("%v\n%s", err, tailLines(h.logs("selfpost"), "link", 20)) + } + return link + } + + before := links("/confirm/") + if len(before) == 0 { + t.Fatal("saving the administrator's address sent no confirmation link") + } + if resp, body, err := sc.panel.postForm("/account/email/confirm", nil); err != nil || resp.Request.URL.Path != "/account" || !strings.Contains(body, "A new link is on its way.") { + t.Fatalf("send the link again: %v\n%.2000s", err, body) + } + confirm := newLink("/confirm/", before) + // The link sent before the restore no longer works. + for old := range before { + if _, page, _ := sc.panel.get(old); !strings.Contains(page, "This link has expired or was already used.") { + t.Errorf("a link sent before the restore still works") + } + } + _, page, err := sc.panel.get(confirm) + if err != nil || !strings.Contains(page, `action="`+confirm+`"`) { + t.Fatalf("the confirmation link's page: %v\n%.2000s", err, page) + } + if _, account, _ := sc.panel.get("/account"); !strings.Contains(account, "not confirmed") { + t.Fatalf("opening the link confirmed the address:\n%.3000s", account) + } + if _, done, err := sc.panel.postForm(confirm, nil); err != nil || !strings.Contains(done, "E-mail confirmed") { + t.Fatalf("confirm: %v\n%.2000s", err, done) + } + if _, account, _ := sc.panel.get("/account"); !strings.Contains(account, `confirmed`) { + t.Fatalf("the address is not confirmed on Account:\n%.3000s", account) + } + + stranger, err := newPanelClient() + if err != nil { + t.Fatal(err) + } + resp, body, err := stranger.postForm("/login/reset", url.Values{"username": {adminUser}}) + if err != nil || resp.Request.URL.Path != "/login" || !strings.Contains(body, "the link is on its way") { + t.Fatalf("ask for a reset link: %v, landed on %s\n%.2000s", err, resp.Request.URL.Path, body) + } + reset := newLink("/login/reset/", nil) + const fresh = "a-brand-new-e2e-password" + resp, body, err = stranger.postForm(reset, url.Values{"new_password": {fresh}, "new_password_confirm": {fresh}}) + if err != nil || resp.Request.URL.Path != "/login" || !strings.Contains(body, "Password set. Sign in with it.") { + t.Fatalf("set the new password: %v, landed on %s\n%.2000s", err, resp.Request.URL.Path, body) + } + if resp, _, _ := stranger.postForm(reset, url.Values{"new_password": {fresh + "x"}, "new_password_confirm": {fresh + "x"}}); resp == nil || resp.StatusCode != 404 { + t.Errorf("the reset link worked twice") + } + if err := stranger.login(adminUser, adminPassword); err == nil { + t.Error("the old password still signs in") + } + // The scenario's session ended with the reset. + if resp, err := sc.panel.status(); err != nil || resp.Request.URL.Path != "/login" { + t.Fatalf("the scenario's session survived the reset") + } + if err := sc.panel.login(adminUser, fresh); err != nil { + t.Fatalf("sign in with the new password: %v", err) + } + if err := stranger.login(adminUser, fresh); err != nil { + t.Fatalf("sign in with the new password elsewhere: %v", err) + } +}