fix(panel): GUI defects from the 1.2.x layout pass (P3)
- .flash.error now renders on the danger surface instead of the success
one; RateLimitErr previously showed as green with red text.
- User delete goes through a confirmation page (GET/POST
/users/{uid}/delete), matching the domain-delete pattern, instead of a
plain submit button next to Save with no confirmation.
- Extracted the repeated DNS Host/Type/Value markup on a domain's page and
the duplicated Settings credentials form into shared partials. No
behaviour change.
docs/plans/code-review.md P3 checked off; CHANGELOG updated.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"}}
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<span class="code">{{.}}</span>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{end}}
|
||||
|
||||
{{define "host_type_copy"}}
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{end}}
|
||||
|
||||
{{/* Value partials pair with the two host_type variants above: field_values
|
||||
is the DNS status card's raw record dump (no Copy — those values are for
|
||||
comparison, not for pasting), field_value is the single publishable
|
||||
value on the record cards below (always has Copy). Callers still guard
|
||||
the empty case, since "no records yet" and "one blank record" read
|
||||
differently. */}}
|
||||
{{define "field_values"}}
|
||||
<label>Value</label>
|
||||
<span class="code">{{range .}}{{.}}
|
||||
{{end}}</span>
|
||||
{{end}}
|
||||
|
||||
{{define "field_value"}}
|
||||
<label>Value</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
{{end}}
|
||||
|
||||
{{define "content"}}
|
||||
<h1>{{.Domain.Name}}</h1>
|
||||
|
||||
@@ -37,21 +92,8 @@
|
||||
<div class="check-cols">
|
||||
<div class="check-col">
|
||||
<label>DKIM <span class="st st-{{.DNS.DKIM.Status}}">{{.DNS.DKIM.Status}}</span></label>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<span class="code">{{.Record.Name}}</span>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{if .DNS.DKIM.Records}}
|
||||
<label>Value</label>
|
||||
<span class="code">{{range .DNS.DKIM.Records}}{{.}}
|
||||
{{end}}</span>
|
||||
{{end}}
|
||||
{{template "host_type" .Record.Name}}
|
||||
{{if .DNS.DKIM.Records}}{{template "field_values" .DNS.DKIM.Records}}{{end}}
|
||||
{{if ne .DNS.DKIM.Status "ok"}}
|
||||
<p class="{{if eq .DNS.DKIM.Status "unknown"}}muted{{else}}error{{end}}">{{.DNS.DKIM.Detail}}</p>
|
||||
{{end}}
|
||||
@@ -59,21 +101,8 @@
|
||||
|
||||
<div class="check-col">
|
||||
<label>SPF <span class="st st-{{.DNS.SPF.Status}}">{{.DNS.SPF.Status}}</span></label>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<span class="code">{{.Domain.Name}}</span>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{if .DNS.SPF.Records}}
|
||||
<label>Value</label>
|
||||
<span class="code">{{range .DNS.SPF.Records}}{{.}}
|
||||
{{end}}</span>
|
||||
{{end}}
|
||||
{{template "host_type" .Domain.Name}}
|
||||
{{if .DNS.SPF.Records}}{{template "field_values" .DNS.SPF.Records}}{{end}}
|
||||
{{if ne .DNS.SPF.Status "ok"}}
|
||||
<p class="{{if eq .DNS.SPF.Status "unknown"}}muted{{else}}error{{end}}">{{.DNS.SPF.Detail}}</p>
|
||||
{{end}}
|
||||
@@ -83,21 +112,8 @@
|
||||
|
||||
<div class="check-col">
|
||||
<label>DMARC <span class="st st-{{.DNS.DMARC.Status}}">{{.DNS.DMARC.Status}}</span></label>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<span class="code">{{.DMARCName}}</span>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{if .DNS.DMARC.Records}}
|
||||
<label>Value</label>
|
||||
<span class="code">{{range .DNS.DMARC.Records}}{{.}}
|
||||
{{end}}</span>
|
||||
{{end}}
|
||||
{{template "host_type" .DMARCName}}
|
||||
{{if .DNS.DMARC.Records}}{{template "field_values" .DNS.DMARC.Records}}{{end}}
|
||||
{{if ne .DNS.DMARC.Status "ok"}}
|
||||
<p class="{{if eq .DNS.DMARC.Status "unknown"}}muted{{else}}error{{end}}">{{.DNS.DMARC.Detail}}</p>
|
||||
{{else}}
|
||||
@@ -108,21 +124,8 @@
|
||||
<div class="check-col">
|
||||
{{if .DNS.DMARCReportAuth.Status}}
|
||||
<label>Report authorization <span class="st st-{{.DNS.DMARCReportAuth.Status}}">{{.DNS.DMARCReportAuth.Status}}</span></label>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<span class="code">{{.ReportAuthName}}</span>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{if .DNS.DMARCReportAuth.Records}}
|
||||
<label>Value</label>
|
||||
<span class="code">{{range .DNS.DMARCReportAuth.Records}}{{.}}
|
||||
{{end}}</span>
|
||||
{{end}}
|
||||
{{template "host_type" .ReportAuthName}}
|
||||
{{if .DNS.DMARCReportAuth.Records}}{{template "field_values" .DNS.DMARCReportAuth.Records}}{{end}}
|
||||
<p class="{{if eq .DNS.DMARCReportAuth.Status "ok"}}muted{{else}}error{{end}}">{{.DNS.DMARCReportAuth.Detail}}</p>
|
||||
{{else}}
|
||||
<label>Report authorization</label>
|
||||
@@ -141,49 +144,15 @@
|
||||
<h2>DKIM and SPF records</h2>
|
||||
|
||||
<p class="check-col-title">DKIM</p>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.Record.Name}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<label>Value</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.Record.Value}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
{{template "host_type_copy" .Record.Name}}
|
||||
{{template "field_value" .Record.Value}}
|
||||
|
||||
<p class="muted">Not a secret. Signed with selector
|
||||
<strong>{{.Domain.DKIMSelector}}</strong>.</p>
|
||||
|
||||
<p class="check-col-title">SPF</p>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.Domain.Name}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<label>Value</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.SPFExample}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
{{template "host_type_copy" .Domain.Name}}
|
||||
{{template "field_value" .SPFExample}}
|
||||
|
||||
<p class="muted">Merge into an existing SPF if the domain already has one —
|
||||
do not publish a second record.</p>
|
||||
@@ -192,19 +161,7 @@
|
||||
<div class="card" id="dmarc">
|
||||
<h2>DMARC record</h2>
|
||||
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.DMARCName}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
{{template "host_type_copy" .DMARCName}}
|
||||
|
||||
<label>Value{{if eq .DMARCSource "settings"}} <span class="muted">(from Settings)</span>{{else if eq .DMARCSource "custom"}} <span class="muted">(custom)</span>{{else if eq .DMARCSource "none"}} <span class="muted">(no reports)</span>{{end}}</label>
|
||||
<div class="code-row">
|
||||
@@ -220,25 +177,8 @@
|
||||
|
||||
{{if .NeedsReportAuth}}
|
||||
<p class="check-col-title">Report authorization</p>
|
||||
<div class="field-pair host-type">
|
||||
<div>
|
||||
<label>Host / name</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.ReportAuthName}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="field-type">
|
||||
<label>Type</label>
|
||||
<span class="code">TXT</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<label>Value</label>
|
||||
<div class="code-row">
|
||||
<span class="code">{{.ReportAuthValue}}</span>
|
||||
<button type="button" class="copy">Copy</button>
|
||||
</div>
|
||||
{{template "host_type_copy" .ReportAuthName}}
|
||||
{{template "field_value" .ReportAuthValue}}
|
||||
{{end}}
|
||||
|
||||
<p class="muted"><code>p=none</code> does not affect delivery. Tighten to
|
||||
|
||||
@@ -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"}}
|
||||
<label for="username">Username</label>
|
||||
<input id="username" name="username" autocomplete="username"
|
||||
autocapitalize="none" spellcheck="false" value="{{.FormUsername}}" required>
|
||||
|
||||
<label for="current_password">Current password</label>
|
||||
<input id="current_password" name="current_password" type="password"
|
||||
autocomplete="current-password" required>
|
||||
|
||||
<label for="new_password">New password</label>
|
||||
<input id="new_password" name="new_password" type="password" autocomplete="new-password">
|
||||
|
||||
<label for="new_password_confirm">Confirm new password</label>
|
||||
<input id="new_password_confirm" name="new_password_confirm" type="password" autocomplete="new-password">
|
||||
{{end}}
|
||||
|
||||
{{define "content"}}
|
||||
<h1>Settings</h1>
|
||||
|
||||
@@ -16,19 +35,7 @@
|
||||
<p class="muted">These are the credentials for this control panel only.
|
||||
Applications keep their own logins and passwords, which are not affected.</p>
|
||||
|
||||
<label for="username">Username</label>
|
||||
<input id="username" name="username" autocomplete="username"
|
||||
autocapitalize="none" spellcheck="false" value="{{.FormUsername}}" required>
|
||||
|
||||
<label for="current_password">Current password</label>
|
||||
<input id="current_password" name="current_password" type="password"
|
||||
autocomplete="current-password" required>
|
||||
|
||||
<label for="new_password">New password</label>
|
||||
<input id="new_password" name="new_password" type="password" autocomplete="new-password">
|
||||
|
||||
<label for="new_password_confirm">Confirm new password</label>
|
||||
<input id="new_password_confirm" name="new_password_confirm" type="password" autocomplete="new-password">
|
||||
{{template "credentials_fields" .}}
|
||||
</div>
|
||||
|
||||
<div class="card">
|
||||
@@ -81,19 +88,7 @@ this one stays signed in.</p>
|
||||
<p class="muted">These are the credentials for this control panel only.
|
||||
Applications keep their own logins and passwords, which are not affected.</p>
|
||||
<form method="post" action="/settings">
|
||||
<label for="username">Username</label>
|
||||
<input id="username" name="username" autocomplete="username"
|
||||
autocapitalize="none" spellcheck="false" value="{{.FormUsername}}" required>
|
||||
|
||||
<label for="current_password">Current password</label>
|
||||
<input id="current_password" name="current_password" type="password"
|
||||
autocomplete="current-password" required>
|
||||
|
||||
<label for="new_password">New password</label>
|
||||
<input id="new_password" name="new_password" type="password" autocomplete="new-password">
|
||||
|
||||
<label for="new_password_confirm">Confirm new password</label>
|
||||
<input id="new_password_confirm" name="new_password_confirm" type="password" autocomplete="new-password">
|
||||
{{template "credentials_fields" .}}
|
||||
|
||||
<button type="submit">Save changes</button>
|
||||
</form>
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
{{define "content"}}
|
||||
<h1>Delete {{.TargetUsername}}</h1>
|
||||
|
||||
{{template "back_link" (back (printf "/users/%d" .TargetID) (printf "Back to %s" .TargetUsername))}}
|
||||
|
||||
<div class="card narrow">
|
||||
<h2>Confirm deletion</h2>
|
||||
<p>You are about to delete the panel user <strong>{{.TargetUsername}}</strong>.
|
||||
This cannot be undone; a signed-in session for this user stops working
|
||||
immediately.</p>
|
||||
|
||||
<form method="post" action="/users/{{.TargetID}}/delete">
|
||||
<button type="submit" class="danger">Delete {{.TargetUsername}}</button>
|
||||
</form>
|
||||
</div>
|
||||
{{end}}
|
||||
@@ -33,10 +33,13 @@
|
||||
</fieldset>
|
||||
|
||||
<button type="submit">{{if .IsEdit}}Save{{else}}Create{{end}}</button>
|
||||
{{if .IsEdit}}
|
||||
<button type="submit" name="action" value="delete" class="danger"{{if .LastGlobalLocked}} disabled{{end}}>Delete user</button>
|
||||
{{if .LastGlobalLocked}}<p class="muted">The only global administrator cannot be deleted.</p>{{end}}
|
||||
{{end}}
|
||||
</form>
|
||||
{{if .IsEdit}}
|
||||
{{if .LastGlobalLocked}}
|
||||
<p class="muted">The only global administrator cannot be deleted.</p>
|
||||
{{else}}
|
||||
<a class="danger" href="/users/{{.UserID}}/delete">Delete user</a>
|
||||
{{end}}
|
||||
{{end}}
|
||||
</div>
|
||||
{{end}}
|
||||
|
||||
@@ -282,6 +282,7 @@ func TestSettingsPageDocumentsRateLimits(t *testing.T) {
|
||||
func TestDrillDownPagesPlaceBackLinkAboveContent(t *testing.T) {
|
||||
drillDown := map[string]bool{
|
||||
"user_form.html": true,
|
||||
"user_delete.html": true,
|
||||
"domain_detail.html": true,
|
||||
"domain_delete.html": true,
|
||||
"delivery.html": true,
|
||||
|
||||
@@ -37,6 +37,7 @@ var pageFiles = map[string][]string{
|
||||
"settings": {"templates/settings.html"},
|
||||
"users": {"templates/users.html"},
|
||||
"user_form": {"templates/user_form.html"},
|
||||
"user_delete": {"templates/user_delete.html"},
|
||||
"backup": {"templates/backup.html", "templates/encrypt_fields.html"},
|
||||
"domain_detail": {"templates/domain_detail.html", "templates/encrypt_fields.html"},
|
||||
"domain_delete": {"templates/domain_delete.html"},
|
||||
|
||||
@@ -166,6 +166,8 @@ func (s *Server) Handler() http.Handler {
|
||||
authed.HandleFunc("POST /users/new", h.HandleUserNew)
|
||||
authed.HandleFunc("GET /users/{uid}", h.HandleUserEdit)
|
||||
authed.HandleFunc("POST /users/{uid}", h.HandleUserEdit)
|
||||
authed.HandleFunc("GET /users/{uid}/delete", h.HandleUserDeleteConfirm)
|
||||
authed.HandleFunc("POST /users/{uid}/delete", h.HandleUserDelete)
|
||||
|
||||
authed.HandleFunc("GET /backup", h.HandleBackupPage)
|
||||
authed.HandleFunc("POST /backup", h.HandleBackup)
|
||||
|
||||
Reference in New Issue
Block a user