diff --git a/CHANGELOG.md b/CHANGELOG.md index a913e02..7cd31fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,12 +44,24 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version still authenticate to Postfix. This matches the order domain deletion already used. +- panel (GUI): a rejected rate-limit change on a domain's page now renders on + the danger surface (`.flash.error`) instead of the success one — it was + green with red text, reading as good news. Deleting a panel user now goes + through a confirmation page, the same pattern as domain deletion, instead of + a plain submit button next to Save with no confirmation at all. + ### Changed - ci: gofmt on eight files that failed the formatting workflow check (panel config, DNS check, domain transfer export, rate-limit tests, auth principal, domain and delivery handlers, web package doc comment). +- panel (templates): the repeated Host/Type/Value DNS record markup on a + domain's page and the duplicated credentials form on Settings are now + shared partials (`host_type`, `host_type_copy`, `field_value`, + `field_values`, `credentials_fields`) instead of copy-pasted blocks. No + behaviour or visible change. + - docs: full-tree review plan ([docs/plans/code-review.md](docs/plans/code-review.md)) — architecture, quality, GUI, tests, licence; P0 is domain-admin send-log authorization. Roadmap queues that plan ahead of inbound-relay and records diff --git a/docs/plans/code-review.md b/docs/plans/code-review.md index 31a454e..0a1fb23 100644 --- a/docs/plans/code-review.md +++ b/docs/plans/code-review.md @@ -545,11 +545,12 @@ roles, and security.md no longer calls the panel single-user. **Model: Sonnet.** -- [ ] `.flash.error` (or stop using `.flash` for `RateLimitErr`) — danger +- [x] `.flash.error` (or stop using `.flash` for `RateLimitErr`) — danger surface, not success. -- [ ] User delete: `data-confirm` at minimum; prefer a confirm page like - domain delete. -- [ ] DNS field partial; settings credentials partial. +- [x] User delete: `data-confirm` at minimum; prefer a confirm page like + domain delete. Done as a confirm page (`GET/POST /users/{uid}/delete`), + matching `domain_delete.html`. +- [x] DNS field partial; settings credentials partial. - [ ] Optional: `urlquery` on deliveries fragment params; `aria-live` on polled regions; confirm-without-JS note next to the CSRF accepted risks. diff --git a/internal/web/handlers/handlers_users.go b/internal/web/handlers/handlers_users.go index ce80617..05330b6 100644 --- a/internal/web/handlers/handlers_users.go +++ b/internal/web/handlers/handlers_users.go @@ -102,10 +102,6 @@ func (h *Handlers) HandleUserEdit(w http.ResponseWriter, r *http.Request) { h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "Invalid form submission.", FormUsername: u.Username, FormRole: string(u.Role)}) return } - if r.PostFormValue("action") == "delete" { - h.submitUserDelete(w, r, u) - return - } h.submitUserUpdate(w, r, u) default: w.Header().Set("Allow", "GET, POST") @@ -291,6 +287,58 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st http.Redirect(w, r, "/users?done=updated", http.StatusSeeOther) } +// 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). +func (h *Handlers) HandleUserDeleteConfirm(w http.ResponseWriter, r *http.Request) { + if _, ok := h.requireGlobal(w, r); !ok { + return + } + uid, ok := parseUserID(w, r) + if !ok { + return + } + u, err := h.store.GetUser(uid) + if err != nil { + if errors.Is(err, store.ErrUserNotFound) { + http.NotFound(w, r) + return + } + logf("panel: get user %d: %v", uid, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + data := h.pageBase(r) + data["Title"] = "SelfPost — delete " + u.Username + data["Active"] = "users" + data["TargetID"] = u.ID + data["TargetUsername"] = u.Username + h.view.Render(w, http.StatusOK, "user_delete", data) +} + +// HandleUserDelete performs the deletion confirmed on HandleUserDeleteConfirm +// and returns to the user list. +func (h *Handlers) HandleUserDelete(w http.ResponseWriter, r *http.Request) { + if _, ok := h.requireGlobal(w, r); !ok { + return + } + uid, ok := parseUserID(w, r) + if !ok { + return + } + u, err := h.store.GetUser(uid) + if err != nil { + if errors.Is(err, store.ErrUserNotFound) { + http.NotFound(w, r) + return + } + logf("panel: get user %d: %v", uid, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + h.submitUserDelete(w, r, u) +} + func (h *Handlers) submitUserDelete(w http.ResponseWriter, r *http.Request, u store.User) { p, ok := h.principal(r) if !ok { diff --git a/internal/web/view/static/panel.css b/internal/web/view/static/panel.css index 68f107e..30bab78 100644 --- a/internal/web/view/static/panel.css +++ b/internal/web/view/static/panel.css @@ -245,6 +245,11 @@ form.inline { display: inline; margin: 0; } main.page-login, main.page-setup { max-width: 24rem; } .card + .card { margin-top: 1.2rem; } .flash { background: var(--flash-bg); border: 1px solid var(--flash-border); color: var(--flash-fg); padding: 0.7rem 1rem; border-radius: 6px; margin-bottom: 1.2rem; } +/* RateLimitErr renders as .flash.error: a validation failure, not a success + notice. Without this rule it inherited the success surface (green) and only + .error's text colour, so a rejected rate limit read as good news in red + text. Same box, danger palette. */ +.flash.error { background: var(--danger-bg); border-color: var(--danger-border); color: var(--danger-fg); } table { width: 100%; border-collapse: collapse; } /* A table column is at least as wide as the longest unbreakable run inside it, and the panel's tables are full of runs with nothing to break on: email diff --git a/internal/web/view/templates/domain_detail.html b/internal/web/view/templates/domain_detail.html index bd072e6..df45f15 100644 --- a/internal/web/view/templates/domain_detail.html +++ b/internal/web/view/templates/domain_detail.html @@ -2,6 +2,61 @@ measure (same pattern as Status). */}} {{define "wide"}}wide{{end}} +{{/* The Host/name ‖ Type field-pair repeats for every DNS record this page + shows (DKIM, SPF, DMARC, report authorization) in both the status card + and the publishable-record cards below it — only the host and whether it + carries a Copy button change. Two variants rather than one templated + Copy flag: the DNS status card never offers Copy (its host is derived, + not something to paste), the record cards always do. */}} +{{define "host_type"}} +
{{.DNS.DKIM.Detail}}
{{end}} @@ -59,21 +101,8 @@{{.DNS.SPF.Detail}}
{{end}} @@ -83,21 +112,8 @@{{.DNS.DMARC.Detail}}
{{else}} @@ -108,21 +124,8 @@{{.DNS.DMARCReportAuth.Detail}}
{{else}} @@ -141,49 +144,15 @@DKIM
-Not a secret. Signed with selector {{.Domain.DKIMSelector}}.
SPF
-Merge into an existing SPF if the domain already has one — do not publish a second record.
@@ -192,19 +161,7 @@Report authorization
-p=none does not affect delivery. Tighten to
diff --git a/internal/web/view/templates/settings.html b/internal/web/view/templates/settings.html
index 27630ec..4d5a37f 100644
--- a/internal/web/view/templates/settings.html
+++ b/internal/web/view/templates/settings.html
@@ -2,6 +2,25 @@
in panel.css, as on a delivery's page). */}}
{{define "wide"}}wide{{end}}
+{{/* The username/password fields are identical for a global administrator
+ (split card, DMARC alongside) and a domain administrator (narrow card,
+ no DMARC card) — only the surrounding form and card differ. */}}
+{{define "credentials_fields"}}
+
+
+
+
+
+
+
+
+
+
+
+{{end}}
+
{{define "content"}}
These are the credentials for this control panel only. Applications keep their own logins and passwords, which are not affected.
- - - - - - - - - - - + {{template "credentials_fields" .}}These are the credentials for this control panel only. Applications keep their own logins and passwords, which are not affected.
diff --git a/internal/web/view/templates/user_delete.html b/internal/web/view/templates/user_delete.html new file mode 100644 index 0000000..87d007a --- /dev/null +++ b/internal/web/view/templates/user_delete.html @@ -0,0 +1,16 @@ +{{define "content"}} +You are about to delete the panel user {{.TargetUsername}}. + This cannot be undone; a signed-in session for this user stops working + immediately.
+ + +The only global administrator cannot be deleted.
{{end}} - {{end}} + {{if .IsEdit}} + {{if .LastGlobalLocked}} +The only global administrator cannot be deleted.
+ {{else}} + Delete user + {{end}} + {{end}}