diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e5b70f..580cc3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version whether inbound mail is filtered and what happens while the filter is down. The filter's address is shown to the global role only. `inbound`, `inbound_domain` and `inbound_delete` are off the ratchet. +- panel-redesign (stage 2): the **Server** pages - System log, Backup, + Users, the user form and its delete confirmation - and **Help** are in the + new design. Help is a page of topics that the `?` in a box's head opens; + the help drawer is gone. The user form sets the user's e-mail. The backup + download is encrypted unless the box is unticked. Only the queue page is + left on the ratchet. ### Changed @@ -142,6 +148,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version application forms (the script applied only the first matching rule). - The DMARC page of a domain no longer answers 500 once the domain has a reported source. +- The confirmation before deleting a user names the domains that follow + that user's default report address and will be left without one; it used + to say the domains were not affected. ### Removed diff --git a/docs/assets/panel-redesign/evidence/backup.png b/docs/assets/panel-redesign/evidence/backup.png new file mode 100644 index 0000000..da7770f Binary files /dev/null and b/docs/assets/panel-redesign/evidence/backup.png differ diff --git a/docs/assets/panel-redesign/evidence/help.png b/docs/assets/panel-redesign/evidence/help.png new file mode 100644 index 0000000..8c62f92 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/help.png differ diff --git a/docs/assets/panel-redesign/evidence/system-log.png b/docs/assets/panel-redesign/evidence/system-log.png new file mode 100644 index 0000000..d0b45f4 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/system-log.png differ diff --git a/docs/assets/panel-redesign/evidence/user-delete.png b/docs/assets/panel-redesign/evidence/user-delete.png new file mode 100644 index 0000000..f62b626 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/user-delete.png differ diff --git a/docs/assets/panel-redesign/evidence/user.png b/docs/assets/panel-redesign/evidence/user.png new file mode 100644 index 0000000..6d31958 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/user.png differ diff --git a/docs/assets/panel-redesign/evidence/users.png b/docs/assets/panel-redesign/evidence/users.png new file mode 100644 index 0000000..6a080b6 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/users.png differ diff --git a/docs/plans/panel-redesign.md b/docs/plans/panel-redesign.md index 62e54a3..3118c8c 100644 --- a/docs/plans/panel-redesign.md +++ b/docs/plans/panel-redesign.md @@ -518,7 +518,7 @@ only when both are done. Every stage-2 step ends with the evidence of - [x] Outbound: domains, domain, domain settings, application form, shown-once password, delete — **Sonnet** - [x] Outbound: log, message, DMARC hub / domain / report — **Sonnet** - [x] Inbound: domains, domain with the *Spam filter* box, delete — **Sonnet** -- [ ] Server: system log, backup, users, user form, delete; Help as `help_topic` partials, the drawer removed — **Sonnet** +- [x] Server: system log, backup, users, user form, delete; Help as `help_topic` partials, the drawer removed — **Sonnet** - [ ] Queues: old content inside the new shell (outbound), Inbound › Queue hidden — **Sonnet** - [ ] Remove the old `panel.css`, the drawer code in `panel.js` and every class nothing references; `legacyPages` empty, vocabulary test green with no allow-list — **Sonnet** diff --git a/docs/roadmap.md b/docs/roadmap.md index b378292..02ea499 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -39,7 +39,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | panel-notifications | E-mail notifications about important events | candidate | — | — | | delivery-log-storage | Keep a message's `mail.log` lines with its send-log row | candidate | — | — | | password-reset | Password reset by e-mail | candidate | — | — | -| panel-redesign | Panel redesign (accepted mockups, design contract) | agreed | 20/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | +| panel-redesign | Panel redesign (accepted mockups, design contract) | agreed | 21/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows diff --git a/internal/web/handlers/handlers_account.go b/internal/web/handlers/handlers_account.go index 82f71cd..22d8795 100644 --- a/internal/web/handlers/handlers_account.go +++ b/internal/web/handlers/handlers_account.go @@ -162,9 +162,7 @@ func (h *Handlers) HandleAccountProfile(w http.ResponseWriter, r *http.Request) } // The e-mail is the report address of every domain that follows this // user's "my account e-mail" default. - if f.Email != u.Email && u.DMARCDefaultMode == store.DMARCDefaultAccount { - h.resyncDMARC("account e-mail") - } + h.resyncAfterEmailChange(u, f.Email) logf("panel: user %d profile updated (username: %t, e-mail: %t)", u.ID, f.Username != u.Username, f.Email != u.Email) http.Redirect(w, r, "/account?done=profile", http.StatusSeeOther) } @@ -276,6 +274,16 @@ func (h *Handlers) HandleAccountDMARC(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/account?done=dmarc", http.StatusSeeOther) } +// resyncAfterEmailChange is what changing a user's account e-mail does for +// DMARC: the address is the report address of every domain that follows a user +// whose default is "my account e-mail", so the ingest allow-list is rebuilt. It +// is the one path both the Account page and the user form take. +func (h *Handlers) resyncAfterEmailChange(u store.User, newEmail string) { + if newEmail != u.Email && u.DMARCDefaultMode == store.DMARCDefaultAccount { + h.resyncDMARC("account e-mail") + } +} + // resyncDMARC rebuilds the ingest allow-list after something that changes // where a domain's reports go. A failure is logged, not shown: the setting is // saved, and the next resync picks it up. diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index 3909c61..b706361 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -15,6 +15,7 @@ import ( "github.com/mixeme/selfpost/internal/secretfile" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" + "github.com/mixeme/selfpost/internal/web/view" ) // maxImportBytes caps a domain-import upload. A domain export is a small JSON @@ -45,13 +46,11 @@ func (h *Handlers) renderBackupPage(w http.ResponseWriter, r *http.Request, stat // password), importErr to the import card, so neither message appears under the // wrong form. func (h *Handlers) renderBackupPageWith(w http.ResponseWriter, r *http.Request, status int, importErr, backupErr string) { - data := h.pageBase(r) - data["Title"] = "SelfPost — backup & migration" - data["Active"] = "backup" - data["ImportErr"] = importErr - data["BackupErr"] = backupErr - data["MinPwLen"] = validate.MinSecretFilePasswordLen - h.view.Render(w, status, "backup", data) + refusal := importErr + if refusal == "" { + refusal = backupErr + } + h.view.Render(w, status, "backup", view.NewBackup(h.shellMeta(r), validate.MinSecretFilePasswordLen, refusal)) } // HandleBackup streams a full-server backup as a download (architecture.md § @@ -327,7 +326,7 @@ func decryptErrorMessage(err error) string { // deployBackupErr phrases a pre-flight backup failure for the operator. func deployBackupErr(err error) string { if strings.Contains(err.Error(), "DeployRoot") || strings.Contains(err.Error(), "deploy root") { - return "Full backup needs the project directory mounted read-only at /selfpost-deploy — add .:/selfpost-deploy:ro to docker-compose.yml and recreate the container." + return "Full backup needs the project directory mounted read-only at /selfpost-deploy — add .:/selfpost-deploy:ro to the volumes in docker-compose.yml and recreate the container." } return "Could not create the backup: " + err.Error() } diff --git a/internal/web/handlers/handlers_help.go b/internal/web/handlers/handlers_help.go index d8c8dbf..ec1b987 100644 --- a/internal/web/handlers/handlers_help.go +++ b/internal/web/handlers/handlers_help.go @@ -3,20 +3,16 @@ package handlers import ( "net/http" - "github.com/mixeme/selfpost/internal/web/auth" + "github.com/mixeme/selfpost/internal/web/view" ) +// HandleHelp renders the Help page. It is for both roles; the topics about the +// checks of Overview and Health are left out for a viewer who cannot see them. func (h *Handlers) HandleHelp(w http.ResponseWriter, r *http.Request) { p, ok := h.principal(r) if !ok { http.Error(w, "internal error", http.StatusInternalServerError) return } - data := map[string]any{ - "Title": "SelfPost — help", - "User": auth.CurrentUser(r), - "Active": "help", - "IsGlobal": p.IsGlobal(), - } - h.view.Render(w, http.StatusOK, "help", data) + h.view.Render(w, http.StatusOK, "help", view.NewHelp(h.shellMeta(r), p.IsGlobal())) } diff --git a/internal/web/handlers/handlers_monitor.go b/internal/web/handlers/handlers_monitor.go index 29ad3a1..6deebfc 100644 --- a/internal/web/handlers/handlers_monitor.go +++ b/internal/web/handlers/handlers_monitor.go @@ -385,15 +385,7 @@ func (h *Handlers) HandleSystemLog(w http.ResponseWriter, r *http.Request) { if _, ok := h.requireGlobal(w, r); !ok { return } - lines, errText := h.readLogTail() - h.view.Render(w, http.StatusOK, "system_log", map[string]any{ - "Title": "SelfPost — system log", - "User": auth.CurrentUser(r), - "Active": "system_log", - "IsGlobal": true, - "Lines": lines, - "Error": errText, - }) + h.view.Render(w, http.StatusOK, "system-log", h.systemLogPage(r)) } // HandleSystemLogBody serves the HTMX polling fragment for the log-tail view. @@ -401,11 +393,21 @@ func (h *Handlers) HandleSystemLogBody(w http.ResponseWriter, r *http.Request) { if _, ok := h.requireGlobal(w, r); !ok { return } - lines, errText := h.readLogTail() - h.view.RenderFragment(w, http.StatusOK, "system_log_body", map[string]any{ - "Lines": lines, - "Error": errText, - }) + h.view.RenderFragment(w, http.StatusOK, "system_log_body", h.systemLogPage(r)) +} + +// systemLogPage reads the tail of mail.log and splits each line into the time +// and the text the page shows it in, as the delivery log of one message does. A +// line whose head is not a timestamp the log format recognises keeps its whole +// text and no time: nothing is dropped from what the log says. +func (h *Handlers) systemLogPage(r *http.Request) *view.SystemLog { + raw, errText := h.readLogTail() + lines := make([]view.LogLine, len(raw)) + for i, line := range raw { + stamp, rest := logtail.SplitTimestamp(line) + lines[i] = view.LogLine{Time: stamp, Text: rest, Level: view.LogLineLevel(rest)} + } + return view.NewSystemLog(h.shellMeta(r), lines, errText) } func (h *Handlers) readLogTail() ([]string, string) { diff --git a/internal/web/handlers/handlers_server_email_test.go b/internal/web/handlers/handlers_server_email_test.go new file mode 100644 index 0000000..d946fca --- /dev/null +++ b/internal/web/handlers/handlers_server_email_test.go @@ -0,0 +1,191 @@ +package handlers + +import ( + "net/http" + "net/url" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/dmarc" + "github.com/mixeme/selfpost/internal/postfix" + "github.com/mixeme/selfpost/internal/store" + "github.com/mixeme/selfpost/internal/web/auth" +) + +// The user form sets the account e-mail: create stores it, an edit changes it, +// clearing the field clears it, and a post that does not mention the field at +// all leaves it alone. +func TestUserFormSetsTheAccountEmail(t *testing.T) { + h, st := inboundHandlers(t) + 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 with an e-mail = %d:\n%s", rec.Code, rec.Body.String()) + } + u, _ := st.GetUserByUsername("ops") + if u.Email != "ops@acme.io" || len(u.DomainIDs) != 1 { + t.Fatalf("created user = %+v", u) + } + + path, pv := "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)} + post := func(extra url.Values) store.User { + t.Helper() + form := url.Values{"username": {"ops"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}} + for k, v := range extra { + form[k] = v + } + if rec := send(h.HandleUserEdit, &root, "POST", path, pv, form); rec.Code != http.StatusSeeOther { + t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String()) + } + got, _ := st.GetUser(u.ID) + return got + } + if got := post(url.Values{"email": {"new@acme.io"}}); got.Email != "new@acme.io" { + t.Errorf("changed e-mail = %q", got.Email) + } + if got := post(nil); got.Email != "new@acme.io" { + t.Errorf("a post without the field changed the e-mail to %q", got.Email) + } + if got := post(url.Values{"email": {""}}); got.Email != "" { + t.Errorf("clearing the field left %q", got.Email) + } + + // Creating without one is fine. + rec = send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ + "username": {"plain"}, "password": {"a-long-enough-password-1"}, "role": {"global"}}) + if plain, _ := st.GetUserByUsername("plain"); rec.Code != http.StatusSeeOther || plain.Email != "" { + t.Errorf("create without an e-mail = %d, %q", rec.Code, plain.Email) + } +} + +// A bad address is refused on both forms: nothing is saved, and the form comes +// back with what was typed, the refusal as the flash. +func TestUserFormRefusesABadEmailAndKeepsWhatWasTyped(t *testing.T) { + h, st := inboundHandlers(t) + out1, _ := st.AddDomain("one.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + u := userWith(t, st, "ops", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}}) + if err := st.UpdateUser(u.ID, "ops", "test-hash", "ops@acme.io"); err != nil { + t.Fatal(err) + } + + rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, + url.Values{"username": {"ops"}, "email": {"not-an-address"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) + body := rec.Body.String() + for _, want := range []string{`class="notification is-danger is-light"`, "enter a valid email address", + `name="email" type="email" value="not-an-address"`, `

ops

`} { + if !strings.Contains(body, want) { + t.Errorf("the refused edit form is missing %q", want) + } + } + if after, _ := st.GetUser(u.ID); rec.Code != http.StatusBadRequest || after.Email != "ops@acme.io" { + t.Errorf("a bad address on edit = %d, stored e-mail %q", rec.Code, after.Email) + } + + rec = send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, url.Values{ + "username": {"fresh"}, "email": {"nobody@"}, "password": {"a-long-enough-password-1"}, "role": {"global"}}) + body = rec.Body.String() + for _, want := range []string{"enter a valid email address", `name="email" type="email" value="nobody@"`, `name="username" value="fresh"`, + `

Create user

`} { + if !strings.Contains(body, want) { + t.Errorf("the refused create form is missing %q", want) + } + } + if fresh, _ := st.GetUserByUsername("fresh"); rec.Code != http.StatusBadRequest || fresh.ID != 0 { + t.Errorf("a bad address on create = %d, user created: %v", rec.Code, fresh.ID != 0) + } +} + +type countingMaps struct{ n int } + +func (c *countingMaps) RebuildDMARCMaps(postfix.DMARCMapsConfig) error { c.n++; return nil } + +// Changing the e-mail of a user whose default report address is "my account +// e-mail" rebuilds the ingest allow-list, exactly as the Account page does; for +// anyone else, or when the address is not changed, nothing is rebuilt. +func TestUserFormEmailChangeResyncsDMARCOnlyForTheAccountDefault(t *testing.T) { + h, st := inboundHandlers(t) + maps := &countingMaps{} + h.dmarc = dmarc.NewService(st, maps, "mail.example.org", true) + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + follower := userWith(t, st, "follower", store.RoleGlobal, store.Reach{}) + other := userWith(t, st, "other", store.RoleGlobal, store.Reach{}) + if err := st.SetDMARCDefault(follower.ID, store.DMARCDefaultAccount, ""); err != nil { + t.Fatal(err) + } + save := func(who auth.Principal, email string) { + t.Helper() + rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(who.ID), map[string]string{"uid": idStr(who.ID)}, + url.Values{"username": {who.Username}, "email": {email}, "role": {"global"}}) + if rec.Code != http.StatusSeeOther { + t.Fatalf("save %s = %d:\n%s", who.Username, rec.Code, rec.Body.String()) + } + } + + save(other, "other@example.org") + if maps.n != 0 { + t.Errorf("a user whose default is not the account e-mail rebuilt the maps %d time(s)", maps.n) + } + save(follower, "follower@example.org") + if maps.n != 1 { + t.Errorf("changing the account e-mail of a follower rebuilt the maps %d time(s), want 1", maps.n) + } + save(follower, "follower@example.org") + if maps.n != 1 { + t.Errorf("saving the same address again rebuilt the maps (%d)", maps.n) + } +} + +// Deleting a user leaves every domain that follows their default report address +// with none; the confirmation says which, and says nothing of the kind when no +// domain follows. +func TestUserDeleteConfirmationNamesTheDomainsThatLoseTheirReportAddress(t *testing.T) { + h, st := inboundHandlers(t) + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + u := userWith(t, st, "shop", store.RoleGlobal, store.Reach{}) + path, pv := "/server/users/"+idStr(u.ID)+"/delete", map[string]string{"uid": idStr(u.ID)} + page := func() string { return send(h.HandleUserDeleteConfirm, &root, "GET", path, pv, nil).Body.String() } + + if body := page(); strings.Contains(body, "DMARC report address") { + t.Errorf("no domain follows the user, yet the page speaks of DMARC:\n%s", body) + } + + var follows []store.Domain + for _, name := range []string{"a.example.com", "b.example.com", "c.example.com", "d.example.com"} { + d, _ := st.AddDomain(name, "mail") + if err := st.SetDomainDMARCUser(d.ID, u.ID); err != nil { + t.Fatal(err) + } + follows = append(follows, d) + } + elsewhere, _ := st.AddDomain("z.example.com", "mail") + if err := st.SetDomainDMARCUser(elsewhere.ID, root.ID); err != nil { + t.Fatal(err) + } + + body := page() + want := `a.example.com, b.example.com, c.example.com and 1 more will have no DMARC report address until someone sets one.` + if !strings.Contains(body, want) { + t.Errorf("the delete page is missing %q:\n%s", want, body) + } + if strings.Contains(body, "z.example.com") || strings.Contains(body, "d.example.com") { + t.Error("the page names a domain that does not follow this user, or spells out a fourth name") + } + + // What it says is what happens: the follower domains end up with no address. + if rec := send(h.HandleUserDelete, &root, "POST", path, pv, url.Values{}); rec.Code != http.StatusSeeOther { + t.Fatalf("delete = %d", rec.Code) + } + for _, d := range follows { + got, _ := st.GetDomain(d.ID) + rua, err := st.DomainDMARCRua(got, "") + if err != nil || rua != "" { + t.Errorf("%s still reports to %q (%v) after its user was deleted", d.Name, rua, err) + } + } +} diff --git a/internal/web/handlers/handlers_server_test.go b/internal/web/handlers/handlers_server_test.go new file mode 100644 index 0000000..7381777 --- /dev/null +++ b/internal/web/handlers/handlers_server_test.go @@ -0,0 +1,363 @@ +package handlers + +import ( + "html" + "net/http" + "net/url" + "regexp" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/store" + "github.com/mixeme/selfpost/internal/web/auth" +) + +// ---- System log + +// The page and the fragment polled from it show the tail of mail.log, newest +// line first, with the time split off each line. +func TestSystemLogPageAndFragmentShowTheTail(t *testing.T) { + h := &Handlers{view: mustView(t), cfg: Config{Version: "test"}} + h.cfg.MailLogPath = writeMailLog(t, + "Sep 21 13:58:09 mail postfix/smtp[2123]: 4XcB7k2Jm9z1: to=, status=deferred (451 Greylisted)", + "Sep 21 14:01:52 mail postfix/qmgr[58]: 4XcB9q0Lw2z1: removed", + "a line the log format does not stamp", + ) + + page := getBody(t, h.HandleSystemLog, "/server/log") + for _, want := range []string{ + `

System log

`, `hx-get="/server/log/fragment"`, + `Sep 21 14:01:52 mail postfix/qmgr[58]: 4XcB9q0Lw2z1: removed`, + `mail postfix/smtp[2123]`, // a deferral is amber + `a line the log format does not stamp`, + } { + if !strings.Contains(page, want) { + t.Errorf("the system log page is missing %q", want) + } + } + if strings.Index(page, "a line the log format") > strings.Index(page, "postfix/qmgr") || + strings.Index(page, "postfix/qmgr") > strings.Index(page, "postfix/smtp[2123]") { + t.Error("the log is not shown newest line first") + } + + frag := getBody(t, h.HandleSystemLogBody, "/server/log/fragment") + if !strings.HasPrefix(frag, `
root you`, `root@example.org`, + `global`, `domain`, + `out.example.comin.example.com`, + `All—`, + `User updated.`, `3 users`, `href="/server/users/new"`, `Inbound domains`, + } { + if !strings.Contains(body, want) { + t.Errorf("the users page is missing %q", want) + } + } + + h.cfg.InboundEnabled = false + if body := send(h.HandleUsers, &root, "GET", "/server/users", nil, nil).Body.String(); strings.Contains(body, "Inbound domains") { + t.Error("the users list has an inbound column while the inbound feature is off") + } +} + +// ---- The user form + +var ( + tagRe = regexp.MustCompile(`<(input|select)\b[^>]*>`) + attrRe = regexp.MustCompile(`([a-z-]+)(?:="([^"]*)")?`) + optionRe = regexp.MustCompile(`