user-reauth: the user form asks for your account password (audit A2)

Security audit 2026-10-17, finding A2 (high), stage 0; the mockup is
2c27275. A stolen global session could set its own password on the user
form - no current password asked, the session kept - and then pass the
backup's reauth with it, or add a second global user with a known
password. Now the user form calls reauth (the backup's check: bcrypt
against the signed-in user's own hash, under auth.AllowPasswordChange) when
it adds a user and when a save sets a password, changes the e-mail (reset
links go there) or changes the role, the own account included; deleting a
user asks too, refused on its own confirmation page (renderUserDelete, a
Flash on UserDelete). Name and reach alone are not asked. Missing or wrong:
401 on the same page, "Your account password is incorrect.", nothing saved.

Tests: TestUserFormAsksForTheAccountPassword (create, each of password,
e-mail, role, own password, reach alone, delete); the user-form tests send
the field (withAccountPassword gives root a real hash and h an auth
module). go vet, go test ./internal/web/... on Windows.

Docs: security.md § Backup and domain export, guide § Users, help word for
word with the mockup; CHANGELOG [Unreleased] Security; plan line ticked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-17 21:28:18 +03:00
co-authored by Claude Opus 5.5
parent 2c27275970
commit a37aba6594
17 changed files with 246 additions and 24 deletions
+9
View File
@@ -5,6 +5,15 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
## [Unreleased] ## [Unreleased]
### Security
- **A stolen `global` session no longer takes a full backup** (security
audit 2026-10-17, A2, high): the user form set the signed-in user's own
password without the current one, which then passed the backup's password
check. Adding a user, setting a password, changing an e-mail or a role on
the user form, and deleting a user now ask for your own account password,
as a backup does, under the same limit on wrong guesses.
## [2.14.3] - 2026-10-17 ## [2.14.3] - 2026-10-17
A tag beside a label. No migration; schema 13. A tag beside a label. No migration; schema 13.
+6
View File
@@ -719,6 +719,12 @@ The panel refuses to remove or demote the **last** global user, so it can never
end up with none. A user cannot delete the account they are signed in with. end up with none. A user cannot delete the account they are signed in with.
The user form also sets the user's e-mail, which the users list shows. The user form also sets the user's e-mail, which the users list shows.
Adding a user, setting a user's password, changing their e-mail or role -
your own account's included - and deleting a user ask for **your account
password**, the one you sign in with, as a backup does and under the same
limit on wrong guesses: a session left open is not enough to hand out an
account. Saving only the name or the domain assignments is not asked.
### Sessions ### Sessions
A login survives a container restart: sessions live in SQLite, not in A login survives a container restart: sessions live in SQLite, not in
+4 -2
View File
@@ -685,12 +685,14 @@ the argument.
### Stage 0 — the high and the cheap guards (~5 h) → patch release ### Stage 0 — the high and the cheap guards (~5 h) → patch release
- [ ] **A2.** `reauth` on user create, on a user update that carries - [x] **A2.** `reauth` on user create, on a user update that carries
`password` or changes `role`, and on user delete; the user form gets the `password` or changes `role`, and on user delete; the user form gets the
`account_password` field the Backup page has; security.md § Backup and `account_password` field the Backup page has; security.md § Backup and
domain export says the user form is covered. Done when a test with a domain export says the user form is covered. Done when a test with a
session and no password gets 401 on all three and the existing user-form session and no password gets 401 on all three and the existing user-form
tests pass with the field. — **Opus**, 2 h. tests pass with the field. — **Opus**, 2 h. *Done 2026-10-17:* also on a
change of e-mail (where reset links go); a save of the name and the
reach alone is not asked; TestUserFormAsksForTheAccountPassword.
- [ ] **I2.** The three DMARC maps in `drop_links`; `[ -L ]` beside `[ -e ]` - [ ] **I2.** The three DMARC maps in `drop_links`; `[ -L ]` beside `[ -e ]`
in postfix-config.sh:538,545; an e2e case plants a link and expects the in postfix-config.sh:538,545; an e2e case plants a link and expects the
log line. — **Opus**, 0.5 h. log line. — **Opus**, 0.5 h.
+10
View File
@@ -471,6 +471,16 @@ operator can act on it.
file's own protection, not this one: whoever downloads it chooses that file's own protection, not this one: whoever downloads it chooses that
password. The import and the `selfpost-backup` CLI, which runs inside the password. The import and the `selfpost-backup` CLI, which runs inside the
container, are not asked. container, are not asked.
- **Nor is it enough to make an account that would pass that check**
(security audit 2026-10-17, A2). Until `2.14.4` a `global` session could
set its own password on the user form - no current password asked, the
session kept - and then pass the check above with the password it chose,
or add a second global user with a password it knew. The user form now
asks for the same account password, under the same limiter, when it adds a
user and when a save sets a password, changes the e-mail (where reset
links go) or changes the role - the signed-in user's own account included -
and deleting a user asks for it too. A save of the name and the domain
assignments alone is not asked: neither reaches an account's secret.
- Both files are secrets: a full backup carries DKIM keys, `sasldb2`, the - Both files are secrets: a full backup carries DKIM keys, `sasldb2`, the
panel users' password hashes, `docker-compose.yml`, `.env`, and the TLS panel users' password hashes, `docker-compose.yml`, `.env`, and the TLS
private key from `certs/` when present; a domain export carries the DKIM key private key from `certs/` when present; a domain export carries the DKIM key
@@ -353,11 +353,12 @@ func TestUserFormAssignsBothDirections(t *testing.T) {
out, _ := st.AddDomain("out.example.com", "mail") out, _ := st.AddDomain("out.example.com", "mail")
in, _ := st.AddInboundDomain("in.example.com") in, _ := st.AddInboundDomain("in.example.com")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
const password = "a-long-enough-password-1" const password = "a-long-enough-password-1"
create := func(username string, extra url.Values) (*httptest.ResponseRecorder, store.User) { create := func(username string, extra url.Values) (*httptest.ResponseRecorder, store.User) {
t.Helper() t.Helper()
form := url.Values{"username": {username}, "password": {password}, "role": {"domain"}} form := url.Values{"username": {username}, "password": {password}, "role": {"domain"}, "account_password": {testAccountPassword}}
for k, v := range extra { for k, v := range extra {
form[k] = v form[k] = v
} }
@@ -385,6 +386,7 @@ func TestUserFormAssignsBothDirections(t *testing.T) {
update := func(u store.User, form url.Values) store.User { update := func(u store.User, form url.Values) store.User {
t.Helper() t.Helper()
form.Set("username", u.Username) form.Set("username", u.Username)
form.Set("account_password", testAccountPassword)
rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, form) rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, form)
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("update %s = %d:\n%s", u.Username, rec.Code, rec.Body.String()) t.Fatalf("update %s = %d:\n%s", u.Username, rec.Code, rec.Body.String())
+1 -1
View File
@@ -286,7 +286,7 @@ func TestDeletingAUserLeavesDomainAddressesAndNamesNobody(t *testing.T) {
lacks(t, name+" for the co-administrator", body, `boss-private@example.org`, `admin&#39;s`, `s default`, `Another user`) lacks(t, name+" for the co-administrator", body, `boss-private@example.org`, `admin&#39;s`, `s default`, `Another user`)
} }
rec := send(s.h.HandleUserDelete, &globalPrincipal, "POST", "/server/users/"+idStr(ops.ID)+"/delete", map[string]string{"uid": idStr(ops.ID)}, url.Values{}) rec := send(s.h.HandleUserDelete, &globalPrincipal, "POST", "/server/users/"+idStr(ops.ID)+"/delete", map[string]string{"uid": idStr(ops.ID)}, url.Values{"account_password": {"correct-password-here!"}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("deleting a user = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("deleting a user = %d:\n%s", rec.Code, rec.Body.String())
} }
+3 -2
View File
@@ -18,10 +18,11 @@ func TestUserFormSendsANewAddressItsLink(t *testing.T) {
ml := withMail(t, h) ml := withMail(t, h)
out1, _ := st.AddDomain("one.example.com", "mail") out1, _ := st.AddDomain("one.example.com", "mail")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ 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"}, "username": {"ops"}, "email": {"ops@acme.io"}, "password": {"a-long-enough-password-1"}, "role": {"domain"},
"domain_ids": {idStr(out1.ID)}, "domain_ids": {idStr(out1.ID)}, "account_password": {testAccountPassword},
}) })
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String())
@@ -42,7 +43,7 @@ func TestUserFormSendsANewAddressItsLink(t *testing.T) {
} }
// The same address saved again keeps its mark and sends nothing; a new // The same address saved again keeps its mark and sends nothing; a new
// one loses it and is sent its link. // one loses it and is sent its link.
form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "email": {"ops@acme.io"}} form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "email": {"ops@acme.io"}, "account_password": {testAccountPassword}}
if rec := send(h.HandleUserEdit, &root, "POST", path, pv, form); rec.Code != http.StatusSeeOther || len(sentBy(h, ml)) != 1 { 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))) t.Fatalf("the same address: %d, sent %d", rec.Code, len(sentBy(h, ml)))
} }
@@ -16,10 +16,11 @@ func TestUserFormSetsTheAccountEmail(t *testing.T) {
h, st := inboundHandlers(t) h, st := inboundHandlers(t)
out1, _ := st.AddDomain("one.example.com", "mail") out1, _ := st.AddDomain("one.example.com", "mail")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ 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"}, "username": {"ops"}, "email": {" ops@acme.io "}, "password": {"a-long-enough-password-1"}, "role": {"domain"},
"domain_ids": {idStr(out1.ID)}, "domain_ids": {idStr(out1.ID)}, "account_password": {testAccountPassword},
}) })
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("create with an e-mail = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("create with an e-mail = %d:\n%s", rec.Code, rec.Body.String())
@@ -32,7 +33,7 @@ func TestUserFormSetsTheAccountEmail(t *testing.T) {
path, pv := "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)} path, pv := "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}
post := func(extra url.Values) store.User { post := func(extra url.Values) store.User {
t.Helper() t.Helper()
form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}} form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "account_password": {testAccountPassword}}
for k, v := range extra { for k, v := range extra {
form[k] = v form[k] = v
} }
@@ -54,7 +55,7 @@ func TestUserFormSetsTheAccountEmail(t *testing.T) {
// Creating without one is fine. // Creating without one is fine.
rec = send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ rec = send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{
"username": {"plain"}, "password": {"a-long-enough-password-1"}, "role": {"global"}}) "username": {"plain"}, "password": {"a-long-enough-password-1"}, "role": {"global"}, "account_password": {testAccountPassword}})
if plain, _ := st.GetUserByUsername("plain"); rec.Code != http.StatusSeeOther || plain.Email != "" { if plain, _ := st.GetUserByUsername("plain"); rec.Code != http.StatusSeeOther || plain.Email != "" {
t.Errorf("create without an e-mail = %d, %q", rec.Code, plain.Email) t.Errorf("create without an e-mail = %d, %q", rec.Code, plain.Email)
} }
@@ -286,6 +286,7 @@ func TestUserDeletePageNamesTheAssignmentsAndRefusalsStayOnTheForm(t *testing.T)
out1, _ := st.AddDomain("out.example.com", "mail") out1, _ := st.AddDomain("out.example.com", "mail")
in1, _ := st.AddInboundDomain("in.example.com") in1, _ := st.AddInboundDomain("in.example.com")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}, InboundDomainIDs: []int64{in1.ID}}) u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}, InboundDomainIDs: []int64{in1.ID}})
path := "/server/users/" + idStr(u.ID) + "/delete" path := "/server/users/" + idStr(u.ID) + "/delete"
@@ -294,6 +295,7 @@ func TestUserDeletePageNamesTheAssignmentsAndRefusalsStayOnTheForm(t *testing.T)
`<h1 class="title is-3">Delete shop</h1>`, `<div class="box sp-danger-zone">`, `<h1 class="title is-3">Delete shop</h1>`, `<div class="box sp-danger-zone">`,
`remove the assignments to <strong>out.example.com</strong> (outbound) and to <strong>in.example.com</strong> (inbound).`, `remove the assignments to <strong>out.example.com</strong> (outbound) and to <strong>in.example.com</strong> (inbound).`,
`<form method="post" action="` + path + `">`, `href="/server/users/` + idStr(u.ID) + `">Keep it`, `<form method="post" action="` + path + `">`, `href="/server/users/` + idStr(u.ID) + `">Keep it`,
`name="account_password" type="password" autocomplete="current-password" required`,
} { } {
if !strings.Contains(body, want) { if !strings.Contains(body, want) {
t.Errorf("the delete page is missing %q", want) t.Errorf("the delete page is missing %q", want)
@@ -309,7 +311,7 @@ func TestUserDeletePageNamesTheAssignmentsAndRefusalsStayOnTheForm(t *testing.T)
if _, err := st.GetUser(root.ID); err != nil { if _, err := st.GetUser(root.ID); err != nil {
t.Errorf("the refused delete removed the user: %v", err) t.Errorf("the refused delete removed the user: %v", err)
} }
if rec := send(h.HandleUserDelete, &root, "POST", path, map[string]string{"uid": idStr(u.ID)}, url.Values{}); rec.Code != http.StatusSeeOther { if rec := send(h.HandleUserDelete, &root, "POST", path, map[string]string{"uid": idStr(u.ID)}, url.Values{"account_password": {testAccountPassword}}); rec.Code != http.StatusSeeOther {
t.Errorf("deleting another user = %d", rec.Code) t.Errorf("deleting another user = %d", rec.Code)
} }
if _, err := st.GetUser(u.ID); err == nil { if _, err := st.GetUser(u.ID); err == nil {
@@ -102,7 +102,7 @@ func TestAccountPasswordSignsOutOnlyTheUsersOtherSessions(t *testing.T) {
// An administrator who sets another user's password on the user form takes // An administrator who sets another user's password on the user form takes
// away every login that user had under the old one. // away every login that user had under the old one.
func TestUserFormPasswordSignsOutThatUsersSessions(t *testing.T) { func TestUserFormPasswordSignsOutThatUsersSessions(t *testing.T) {
h, _ := settingsServer(t) h, password := settingsServer(t)
admin := globalPrincipal admin := globalPrincipal
bob := addUser(t, h, "bob", "bobs-long-password-1") bob := addUser(t, h, "bob", "bobs-long-password-1")
adminCurrent := signedIn(t, h, "admin", "admin-current") adminCurrent := signedIn(t, h, "admin", "admin-current")
@@ -110,7 +110,7 @@ func TestUserFormPasswordSignsOutThatUsersSessions(t *testing.T) {
bob2 := signedIn(t, h, "bob", "bob-2") bob2 := signedIn(t, h, "bob", "bob-2")
rec := postWithSession(h.HandleUserEdit, admin, adminCurrent, "/server/users/"+idStr(bob.ID), map[string]string{"uid": idStr(bob.ID)}, rec := postWithSession(h.HandleUserEdit, admin, adminCurrent, "/server/users/"+idStr(bob.ID), map[string]string{"uid": idStr(bob.ID)},
url.Values{"username": {"bob"}, "role": {"global"}, "password": {"a-brand-new-long-password"}}) url.Values{"username": {"bob"}, "role": {"global"}, "password": {"a-brand-new-long-password"}, "account_password": {password}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String())
} }
@@ -125,12 +125,12 @@ func TestUserFormPasswordSignsOutThatUsersSessions(t *testing.T) {
// Saving the form without a new password does not sign anyone out. // Saving the form without a new password does not sign anyone out.
func TestUserFormWithoutAPasswordKeepsTheSessions(t *testing.T) { func TestUserFormWithoutAPasswordKeepsTheSessions(t *testing.T) {
h, _ := settingsServer(t) h, password := settingsServer(t)
bob := addUser(t, h, "bob", "bobs-long-password-1") bob := addUser(t, h, "bob", "bobs-long-password-1")
bob1 := signedIn(t, h, "bob", "bob-1") bob1 := signedIn(t, h, "bob", "bob-1")
rec := postWithSession(h.HandleUserEdit, globalPrincipal, "no-session", "/server/users/"+idStr(bob.ID), map[string]string{"uid": idStr(bob.ID)}, rec := postWithSession(h.HandleUserEdit, globalPrincipal, "no-session", "/server/users/"+idStr(bob.ID), map[string]string{"uid": idStr(bob.ID)},
url.Values{"username": {"bob"}, "role": {"global"}, "email": {"bob@example.org"}}) url.Values{"username": {"bob"}, "role": {"global"}, "email": {"bob@example.org"}, "account_password": {password}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String())
} }
@@ -142,12 +142,12 @@ func TestUserFormWithoutAPasswordKeepsTheSessions(t *testing.T) {
// An administrator who sets their own password through the user form is signed // An administrator who sets their own password through the user form is signed
// out everywhere except in the browser they are using. // out everywhere except in the browser they are using.
func TestUserFormOwnPasswordKeepsTheCurrentSession(t *testing.T) { func TestUserFormOwnPasswordKeepsTheCurrentSession(t *testing.T) {
h, _ := settingsServer(t) h, password := settingsServer(t)
current := signedIn(t, h, "admin", "admin-current") current := signedIn(t, h, "admin", "admin-current")
other := signedIn(t, h, "admin", "admin-other") other := signedIn(t, h, "admin", "admin-other")
rec := postWithSession(h.HandleUserEdit, globalPrincipal, current, "/server/users/1", map[string]string{"uid": "1"}, rec := postWithSession(h.HandleUserEdit, globalPrincipal, current, "/server/users/1", map[string]string{"uid": "1"},
url.Values{"username": {"admin"}, "role": {"global"}, "password": {"a-brand-new-long-password"}}) url.Values{"username": {"admin"}, "role": {"global"}, "password": {"a-brand-new-long-password"}, "account_password": {password}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("POST user form = %d:\n%s", rec.Code, rec.Body.String())
} }
+41 -2
View File
@@ -203,6 +203,12 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) {
h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: "Assign at least one outbound or inbound domain to a domain administrator.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: "Assign at least one outbound or inbound domain to a domain administrator.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
// A new user is an account whose password the creator knows: a session
// alone must not be enough to make one (audit 2026-10-17, A2).
if status, msg := h.reauth(r, "create user"); status != 0 {
h.renderUserForm(w, r, status, 0, userFormView{FormErr: msg, FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return
}
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost) hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
if err != nil { if err != nil {
@@ -293,12 +299,25 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st
} }
} }
hash := "" // empty keeps the stored hash (SaveUser)
if password != "" { if password != "" {
if err := validate.AdminPassword(password); err != nil { if err := validate.AdminPassword(password); err != nil {
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
}
// A password, an e-mail (where reset links go) or a role set here is a way
// to an account, the signed-in one's own included: it asks for the
// signed-in user's password, as a backup does (audit 2026-10-17, A2).
// Name and reach alone do not.
if needsReauth(u, password, email, role) {
if status, msg := h.reauth(r, "user "+strconv.FormatInt(u.ID, 10)); status != 0 {
h.renderUserForm(w, r, status, u.ID, userFormView{FormErr: msg, FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return
}
}
hash := "" // empty keeps the stored hash (SaveUser)
if password != "" {
newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost) newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
if err != nil { if err != nil {
logf("panel: update user hash: %v", err) logf("panel: update user hash: %v", err)
@@ -336,6 +355,12 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st
http.Redirect(w, r, "/server/users?done=updated", http.StatusSeeOther) http.Redirect(w, r, "/server/users?done=updated", http.StatusSeeOther)
} }
// needsReauth is whether a user-form save changes what reaches the account:
// its password, its e-mail (reset links go there) or its role.
func needsReauth(u store.User, password, email string, role store.Role) bool {
return password != "" || email != u.Email || role != u.Role
}
// endSessionsAfterPasswordSet signs out the logins a user had under the old // endSessionsAfterPasswordSet signs out the logins a user had under the old
// password when an administrator sets a new one on the user form. An // password when an administrator sets a new one on the user form. An
// administrator who sets their own password keeps the browser they are using. // administrator who sets their own password keeps the browser they are using.
@@ -378,6 +403,13 @@ func (h *Handlers) HandleUserDeleteConfirm(w http.ResponseWriter, r *http.Reques
http.Error(w, "internal error", http.StatusInternalServerError) http.Error(w, "internal error", http.StatusInternalServerError)
return return
} }
h.renderUserDelete(w, r, http.StatusOK, u, "")
}
// renderUserDelete draws the confirmation, with a refusal above it when
// errMsg is set.
func (h *Handlers) renderUserDelete(w http.ResponseWriter, r *http.Request, status int, u store.User, errMsg string) {
var err error
in := view.UserDeleteInput{ in := view.UserDeleteInput{
ID: u.ID, Username: u.Username, Global: u.Role == store.RoleGlobal, ID: u.ID, Username: u.Username, Global: u.Role == store.RoleGlobal,
AllOutbound: u.AllDomains, AllInbound: u.AllInboundDomains, AllOutbound: u.AllDomains, AllInbound: u.AllInboundDomains,
@@ -389,7 +421,8 @@ func (h *Handlers) HandleUserDeleteConfirm(w http.ResponseWriter, r *http.Reques
return return
} }
} }
h.view.Render(w, http.StatusOK, "user-delete", view.NewUserDelete(h.shellMeta(r), in)) in.Error = errMsg
h.view.Render(w, status, "user-delete", view.NewUserDelete(h.shellMeta(r), in))
} }
// assignedNames names the domains a domain user is assigned one by one, outbound // assignedNames names the domains a domain user is assigned one by one, outbound
@@ -456,6 +489,12 @@ func (h *Handlers) submitUserDelete(w http.ResponseWriter, r *http.Request, u st
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "You cannot delete your own account while signed in.", FormUsername: u.Username, FormEmail: u.Email, FormRole: string(u.Role)}) h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "You cannot delete your own account while signed in.", FormUsername: u.Username, FormEmail: u.Email, FormRole: string(u.Role)})
return return
} }
// Removing an administrator asks for the signed-in user's password: a
// session alone must not lock the others out (audit 2026-10-17, A2).
if status, msg := h.reauth(r, "delete user "+strconv.FormatInt(u.ID, 10)); status != 0 {
h.renderUserDelete(w, r, status, u, msg)
return
}
if err := h.store.DeleteUser(u.ID); err != nil { if err := h.store.DeleteUser(u.ID); err != nil {
if errors.Is(err, store.ErrLastGlobal) { if errors.Is(err, store.ErrLastGlobal) {
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "Cannot delete the last global administrator.", FormUsername: u.Username, FormEmail: u.Email, FormRole: string(u.Role)}) h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "Cannot delete the last global administrator.", FormUsername: u.Username, FormEmail: u.Email, FormRole: string(u.Role)})
@@ -0,0 +1,136 @@
package handlers
import (
"net/http"
"net/url"
"path/filepath"
"strings"
"testing"
"github.com/mixeme/selfpost/internal/store"
"github.com/mixeme/selfpost/internal/web/auth"
"golang.org/x/crypto/bcrypt"
)
// testAccountPassword is the signed-in user's own password in the user-form
// tests: what reauth compares account_password with.
const testAccountPassword = "the-signed-in-users-password"
// withAccountPassword gives p's stored user testAccountPassword and h the
// auth module whose limiter reauth uses (the user form asks for the
// signed-in user's password, audit 2026-10-17 A2).
func withAccountPassword(t *testing.T, h *Handlers, st *store.Store, p auth.Principal) {
t.Helper()
hash, err := bcrypt.GenerateFromPassword([]byte(testAccountPassword), bcrypt.MinCost)
if err != nil {
t.Fatal(err)
}
u, err := st.GetUser(p.ID)
if err != nil {
t.Fatal(err)
}
if err := st.UpdateUser(u.ID, u.Username, string(hash), u.Email); err != nil {
t.Fatal(err)
}
if h.auth == nil {
h.auth = auth.New(st, auth.Config{}, h.view, filepath.Join(t.TempDir(), "setup-token"))
}
}
// A session alone is not enough to hand out an account (audit 2026-10-17,
// A2): adding a user, and a save that sets a password, an e-mail or a role -
// the signed-in user's own included - ask for the signed-in user's password;
// so does deleting a user. Missing or wrong, it is 401 on the same page and
// nothing changes; a save of the name and the reach alone is not asked.
func TestUserFormAsksForTheAccountPassword(t *testing.T) {
h, st := inboundHandlers(t)
out, _ := st.AddDomain("out.example.com", "mail")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
const newPassword = "a-long-enough-password-1"
refused := func(what string, rec interface {
Result() *http.Response
}, body string) {
t.Helper()
if code := rec.Result().StatusCode; code != http.StatusUnauthorized || !strings.Contains(body, "Your account password is incorrect.") {
t.Errorf("%s = %d, want 401 with the refusal:\n%.800s", what, code, body)
}
}
// Create: refused without and with a wrong password, the form kept.
form := url.Values{"username": {"ops"}, "password": {newPassword}, "role": {"domain"}, "domain_ids": {idStr(out.ID)}}
for _, pw := range []string{"", "wrong-password-entirely"} {
form.Set("account_password", pw)
rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, form)
refused("create with account password "+pw, rec, rec.Body.String())
if !strings.Contains(rec.Body.String(), `value="ops"`) || !strings.Contains(rec.Body.String(), `name="account_password" type="password" autocomplete="current-password" required`) {
t.Errorf("the refused create form lost the name or the field:\n%.800s", rec.Body.String())
}
if u, _ := st.GetUserByUsername("ops"); u.ID != 0 {
t.Fatalf("a user was created with account password %q", pw)
}
}
form.Set("account_password", testAccountPassword)
if rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, form); rec.Code != http.StatusSeeOther {
t.Fatalf("create with the right password = %d:\n%s", rec.Code, rec.Body.String())
}
ops, _ := st.GetUserByUsername("ops")
// Update: each of password, e-mail, role is asked; the own password too.
edit := func(u store.User, f url.Values) (int, string) {
rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, f)
return rec.Code, rec.Body.String()
}
base := func() url.Values {
return url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out.ID)}, "email": {""}}
}
for name, change := range map[string]func(url.Values){
"password": func(f url.Values) { f.Set("password", "another-long-password-2") },
"e-mail": func(f url.Values) { f.Set("email", "thief@example.net") },
"role": func(f url.Values) { f.Set("role", "global") },
} {
f := base()
change(f)
code, body := edit(ops, f)
if code != http.StatusUnauthorized || !strings.Contains(body, "Your account password is incorrect.") {
t.Errorf("a %s change without the account password = %d:\n%.600s", name, code, body)
}
if got, _ := st.GetUser(ops.ID); got.PasswordHash != ops.PasswordHash || got.Email != ops.Email || got.Role != ops.Role {
t.Errorf("a refused %s change changed the user: %+v", name, got)
}
}
ownForm := url.Values{"username": {"root"}, "role": {"global"}, "password": {"the-thiefs-own-password"}}
rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(root.ID), map[string]string{"uid": idStr(root.ID)}, ownForm)
if rec.Code != http.StatusUnauthorized {
t.Errorf("setting one's own password without the current one = %d", rec.Code)
}
if got, _ := st.GetUser(root.ID); bcrypt.CompareHashAndPassword([]byte(got.PasswordHash), []byte(testAccountPassword)) != nil {
t.Error("a refused own-password change replaced the password")
}
// Name and reach alone: not asked.
if code, body := edit(ops, base()); code != http.StatusSeeOther {
t.Errorf("a save of the reach alone = %d:\n%.600s", code, body)
}
// With the password: the change goes through.
f := base()
f.Set("password", "another-long-password-2")
f.Set("account_password", testAccountPassword)
if code, body := edit(ops, f); code != http.StatusSeeOther {
t.Errorf("a password change with the account password = %d:\n%.600s", code, body)
}
// Delete: refused on its own page, then done with the password.
path, pv := "/server/users/"+idStr(ops.ID)+"/delete", map[string]string{"uid": idStr(ops.ID)}
rec = send(h.HandleUserDelete, &root, "POST", path, pv, url.Values{"account_password": {"wrong-password-entirely"}})
refused("delete with a wrong password", rec, rec.Body.String())
if !strings.Contains(rec.Body.String(), "Delete ops") || !strings.Contains(rec.Body.String(), `name="account_password"`) {
t.Errorf("the refused delete is not the confirmation page:\n%.800s", rec.Body.String())
}
if u, _ := st.GetUser(ops.ID); u.ID == 0 {
t.Fatal("a refused delete removed the user")
}
if rec := send(h.HandleUserDelete, &root, "POST", path, pv, url.Values{"account_password": {testAccountPassword}}); rec.Code != http.StatusSeeOther {
t.Fatalf("delete with the right password = %d:\n%s", rec.Code, rec.Body.String())
}
}
+5 -3
View File
@@ -20,9 +20,10 @@ func TestUserFormSavesTheWholeChange(t *testing.T) {
h, st := inboundHandlers(t) h, st := inboundHandlers(t)
out1, _ := st.AddDomain("out.example.com", "mail") out1, _ := st.AddDomain("out.example.com", "mail")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}}) u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}})
rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)},
url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"global"}}) url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"global"}, "account_password": {testAccountPassword}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String())
} }
@@ -36,7 +37,7 @@ func TestUserFormSavesTheWholeChange(t *testing.T) {
// And back to a domain user, with an assignment. // And back to a domain user, with an assignment.
rec = send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, rec = send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)},
url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "account_password": {testAccountPassword}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String())
} }
@@ -85,9 +86,10 @@ func TestUserFormCreatesWithEmailAndReach(t *testing.T) {
h, st := inboundHandlers(t) h, st := inboundHandlers(t)
out1, _ := st.AddDomain("out.example.com", "mail") out1, _ := st.AddDomain("out.example.com", "mail")
root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) root := userWith(t, st, "root", store.RoleGlobal, store.Reach{})
withAccountPassword(t, h, st, root)
rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil,
url.Values{"username": {"fresh"}, "password": {"a-long-enough-password"}, "email": {"fresh@example.com"}, url.Values{"username": {"fresh"}, "password": {"a-long-enough-password"}, "email": {"fresh@example.com"},
"role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) "role": {"domain"}, "domain_ids": {idStr(out1.ID)}, "account_password": {testAccountPassword}})
if rec.Code != http.StatusSeeOther { if rec.Code != http.StatusSeeOther {
t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String()) t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String())
} }
+1 -1
View File
@@ -284,7 +284,7 @@ var serverTopics = []HelpTopic{
{ID: HelpUsers, Title: "Users and roles", Body: []Text{ {ID: HelpUsers, Title: "Users and roles", Body: []Text{
Rich("The two roles are a reach, not a rank: every user is an administrator and signs in with a username and password. Global reaches the whole server — Overview, Server, the queue and every domain — and is the only role that adds or deletes domains and users. Domain reaches only the domains assigned to it, outbound and inbound separately (the same name on both lists is two assignments); ", Rich("The two roles are a reach, not a rank: every user is an administrator and signs in with a username and password. Global reaches the whole server — Overview, Server, the queue and every domain — and is the only role that adds or deletes domains and users. Domain reaches only the domains assigned to it, outbound and inbound separately (the same name on both lists is two assignments); ",
Em("All"), " includes domains added later, and a domain user needs at least one domain or ", Em("All"), Em("All"), " includes domains added later, and a domain user needs at least one domain or ", Em("All"),
" on either list. The only global user cannot be demoted or deleted, and you cannot delete the account you are signed in with. A new password set here signs that user out everywhere; set for yourself, it keeps the session you set it from."), " on either list. The only global user cannot be demoted or deleted, and you cannot delete the account you are signed in with. A new password set here signs that user out everywhere; set for yourself, it keeps the session you set it from. Adding a user, changing a user's password, e-mail or role, and deleting a user ask for your own account password, as a backup does, with the same limit on wrong guesses."),
}}, }},
{ID: HelpSettings, Title: "Settings", Body: []Text{ {ID: HelpSettings, Title: "Settings", Body: []Text{
Rich("Settings holds what is true of the whole instance. The time zone is the container's, ", Code("TZ"), " in ", Code(".env"), Rich("Settings holds what is true of the whole instance. The time zone is the container's, ", Code("TZ"), " in ", Code(".env"),
+6
View File
@@ -342,6 +342,7 @@ func NewUserForm(m Meta, in UserFormInput) *UserForm {
type UserDelete struct { type UserDelete struct {
Meta Meta
Head Head Head Head
Flash *Flash
Danger Box Danger Box
Intro Text Intro Text
@@ -362,6 +363,8 @@ type UserDeleteInput struct {
AllInbound bool AllInbound bool
Outbound []string Outbound []string
Inbound []string Inbound []string
// Error is a refusal of the confirmation (a wrong account password).
Error string
} }
// NewUserDelete builds the page. // NewUserDelete builds the page.
@@ -384,6 +387,9 @@ func NewUserDelete(m Meta, in UserDeleteInput) *UserDelete {
Confirm: "Delete " + in.Username, Confirm: "Delete " + in.Username,
KeepHref: href, KeepHref: href,
} }
if in.Error != "" {
p.Flash = &Flash{Error: true, Text: Plain(in.Error)}
}
if in.Global { if in.Global {
p.Consequences = append(p.Consequences, Plain("remove their global access to the server and to every domain.")) p.Consequences = append(p.Consequences, Plain("remove their global access to the server and to every domain."))
return p return p
+4 -1
View File
@@ -4,7 +4,8 @@
the lists while the role is Global (data-global-role on the select, the lists while the role is Global (data-global-role on the select,
data-domain-pick on the boxes); without scripts they stay in view and the data-domain-pick on the boxes); without scripts they stay in view and the
server ignores them for the global role. A blank password keeps the current server ignores them for the global role. A blank password keeps the current
one. */}} one. Your account password is asked for a new user and for a change of
password, e-mail or role (audit 2026-10-17, A2); the server decides. */}}
{{define "content"}} {{define "content"}}
{{template "page_head" .Head}} {{template "page_head" .Head}}
{{if .Flash}}{{template "flash" .Flash}}{{end}} {{if .Flash}}{{template "flash" .Flash}}{{end}}
@@ -25,6 +26,8 @@
<option value="{{.Value}}"{{if eq .Value $sel}} selected{{end}}>{{.Label}}</option>{{end}} <option value="{{.Value}}"{{if eq .Value $sel}} selected{{end}}>{{.Label}}</option>{{end}}
</select></div></div> </select></div></div>
<p class="help">Everyone here is an administrator; the role is the reach. <em>Global</em> also sees Server and is the only one who adds or deletes domains. The only global user cannot be changed to <em>domain</em> or deleted.</p></div> <p class="help">Everyone here is an administrator; the role is the reach. <em>Global</em> also sees Server and is the only one who adds or deletes domains. The only global user cannot be changed to <em>domain</em> or deleted.</p></div>
<div class="field"><label class="label" for="acpw">Your account password</label><div class="control"><input class="input" id="acpw" name="account_password" type="password" autocomplete="current-password"{{if .NewUser}} required{{end}}></div>
<p class="help">Yours, the one you sign in with, not this user's. Asked when you add a user, or change a password, an e-mail or a role here: a session left open must not be enough to hand out an account.</p></div>
{{template "body_close"}} {{template "body_close"}}
{{template "box_close"}} {{template "box_close"}}
</div> </div>
@@ -2,6 +2,7 @@
one button that does it, and the way back to the user's form. */}} one button that does it, and the way back to the user's form. */}}
{{define "content"}} {{define "content"}}
{{template "page_head" .Head}} {{template "page_head" .Head}}
{{if .Flash}}{{template "flash" .Flash}}{{end}}
<div class="columns"> <div class="columns">
<div class="column is-7"> <div class="column is-7">
@@ -11,6 +12,8 @@
{{template "confirm_list" .Consequences}} {{template "confirm_list" .Consequences}}
<p class="sp-muted mb-4">{{template "inline" .Note}}</p> <p class="sp-muted mb-4">{{template "inline" .Note}}</p>
<form method="post" action="{{.Action}}"> <form method="post" action="{{.Action}}">
<div class="field"><label class="label" for="acpw">Your account password</label><div class="control"><input class="input" id="acpw" name="account_password" type="password" autocomplete="current-password" required></div>
<p class="help">The one you sign in with: a session left open must not be enough to remove an administrator.</p></div>
<div class="buttons"> <div class="buttons">
<button type="submit" class="button is-danger"><i class="ti ti-trash"></i>{{.Confirm}}</button> <button type="submit" class="button is-danger"><i class="ti ti-trash"></i>{{.Confirm}}</button>
<a class="button is-text" href="{{.KeepHref}}">Keep it</a> <a class="button is-text" href="{{.KeepHref}}">Keep it</a>