fix(panel): code-review P6 optional cleanup

Close the remaining low-risk items from the full-tree review: rename the settings handler, query assigned domains in SQL, bound the login limiter map, collapse panel.js show/hide helpers, and soften DMARC copy that promised a future in-panel receiver.

Co-authored-by: Cursor <cursoragent@cursor.com>

internal/web/handlers/authz.go

@@ -32,17 +32,7 @@ func (h *Handlers) assignedDomains(p auth.Principal) ([]store.Domain, error) {
 	if p.IsGlobal() {
 		return h.store.ListDomains()
 	}
-	all, err := h.store.ListDomains()
-	if err != nil {
-		return nil, err
-	}
-	var out []store.Domain
-	for _, d := range all {
-		if p.CanAccessDomain(d.ID) {
-			out = append(out, d)
-		}
-	}
-	return out, nil
+	return h.store.ListDomainsForUser(p.ID)
 }
 
 func domainNameSet(domains []store.Domain) map[string]bool {

internal/web/handlers/authz_test.go

@@ -57,7 +57,7 @@ var globalOnlyRoutes = []route{
 // case the send-log leak (P0, code-review.md) showed is easy to get wrong.
 func TestGlobalOnlyRoutesAnswerADomainAdmin404(t *testing.T) {
 	h, domains := serverWithTwoDomains(t)
-	p := domainAdmin(domains["first.example.ru"].ID)
+	p := domainAdmin(t, h.store, "global-only", domains["first.example.ru"].ID)
 
 	for _, rt := range globalOnlyRoutes {
 		rec := call(h, rt, p)

internal/web/handlers/handlers_monitor_test.go

@@ -222,34 +222,41 @@ func TestSendLogScopedToAssignedDomains(t *testing.T) {
 	h, domains := serverWithTwoDomains(t)
 
 	for name, tc := range map[string]struct {
-		principal auth.Principal
+		username  string
+		domainIDs []int64
 		want      []string
 		unwanted  []string
 	}{
 		"global sees both": {
-			globalPrincipal, []string{"First message", "Second message"}, nil,
+			"", nil, []string{"First message", "Second message"}, nil,
 		},
 		"one assigned domain": {
-			domainAdmin(domains["first.example.ru"].ID),
+			"one-domain", []int64{domains["first.example.ru"].ID},
 			[]string{"First message"}, []string{"Second message", "second-app"},
 		},
 		"two assigned domains": {
-			domainAdmin(domains["first.example.ru"].ID, domains["second.example.ru"].ID),
+			"two-domains", []int64{domains["first.example.ru"].ID, domains["second.example.ru"].ID},
 			[]string{"First message", "Second message"}, nil,
 		},
 		// Every assigned domain deleted cascades the assignments away. That
 		// leaves a principal entitled to nothing, which is an empty log — the
 		// case that used to hand over the whole journal.
 		"no assigned domains": {
-			domainAdmin(), []string{"No messages logged yet."},
+			"no-domains", nil, []string{"No messages logged yet."},
 			[]string{"First message", "Second message"},
 		},
 	} {
+		var p auth.Principal
+		if tc.username == "" {
+			p = globalPrincipal
+		} else {
+			p = domainAdmin(t, h.store, tc.username, tc.domainIDs...)
+		}
 		for view, handler := range map[string]http.HandlerFunc{
 			"page":     h.HandleDeliveries,
 			"fragment": h.HandleDeliveriesRows,
 		} {
-			out := getBodyAs(t, handler, "/deliveries", tc.principal)
+			out := getBodyAs(t, handler, "/deliveries", p)
 			for _, want := range tc.want {
 				if !strings.Contains(out, want) {
 					t.Errorf("%s (%s): missing %q:\n%s", name, view, want, out)
@@ -270,7 +277,7 @@ func TestSendLogScopedToAssignedDomains(t *testing.T) {
 // trusted for having been rendered by us.
 func TestSendLogIgnoresForgedFilters(t *testing.T) {
 	h, domains := serverWithTwoDomains(t)
-	p := domainAdmin(domains["first.example.ru"].ID)
+	p := domainAdmin(t, h.store, "forged-filter", domains["first.example.ru"].ID)
 
 	for _, target := range []string{
 		"/deliveries?domain=second.example.ru",
@@ -299,7 +306,7 @@ func TestDeliveryPageForeignDomainNotFound(t *testing.T) {
 	rec := httptest.NewRecorder()
 	req := httptest.NewRequest(http.MethodGet, "/deliveries/"+itoa(rows[0].ID), nil)
 	req.SetPathValue("id", itoa(rows[0].ID))
-	req = auth.RequestWithPrincipal(req, domainAdmin(domains["first.example.ru"].ID))
+	req = auth.RequestWithPrincipal(req, domainAdmin(t, h.store, "foreign-detail", domains["first.example.ru"].ID))
 	h.HandleDelivery(rec, req)
 	if rec.Code != http.StatusNotFound {
 		t.Errorf("delivery page for a foreign domain = %d, want 404", rec.Code)
@@ -343,13 +350,37 @@ func serverWithTwoDomains(t *testing.T) (*Handlers, map[string]store.Domain) {
 
 var globalPrincipal = auth.Principal{ID: 1, Username: "admin", Role: auth.RoleGlobal}
 
-func domainAdmin(domainIDs ...int64) auth.Principal {
-	return auth.Principal{
-		ID:       2,
-		Username: "domain-admin",
-		Role:     auth.RoleDomainAdmin,
-		Domains:  domainIDs,
+func domainAdmin(t *testing.T, st *store.Store, username string, domainIDs ...int64) auth.Principal {
+	t.Helper()
+	const hash = "test-hash"
+	if len(domainIDs) == 0 {
+		placeholder, err := st.AddDomain(username+".placeholder.invalid", "mail")
+		if err != nil {
+			t.Fatalf("add placeholder domain: %v", err)
+		}
+		id, err := st.CreateUser(username, hash, store.RoleDomainAdmin, []int64{placeholder.ID})
+		if err != nil {
+			t.Fatalf("create domain admin %s: %v", username, err)
+		}
+		if err := st.DeleteDomain(placeholder.ID); err != nil {
+			t.Fatalf("delete placeholder domain: %v", err)
+		}
+		domainIDs = nil
+		u, err := st.GetUser(id)
+		if err != nil {
+			t.Fatalf("get domain admin %s: %v", username, err)
+		}
+		return auth.Principal{ID: u.ID, Username: u.Username, Role: u.Role, Domains: u.DomainIDs}
 	}
+	id, err := st.CreateUser(username, hash, store.RoleDomainAdmin, domainIDs)
+	if err != nil {
+		t.Fatalf("create domain admin %s: %v", username, err)
+	}
+	u, err := st.GetUser(id)
+	if err != nil {
+		t.Fatalf("get domain admin %s: %v", username, err)
+	}
+	return auth.Principal{ID: u.ID, Username: u.Username, Role: u.Role, Domains: u.DomainIDs}
 }
 
 // serverWithDelivery builds a panel over a store holding one delivery, written

internal/web/handlers/handlers_settings.go

@@ -14,8 +14,8 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
-// HandleAccount serves the signed-in user's account settings.
-func (h *Handlers) HandleAccount(w http.ResponseWriter, r *http.Request) {
+// HandleSettings serves the signed-in user's panel settings.
+func (h *Handlers) HandleSettings(w http.ResponseWriter, r *http.Request) {
 	switch r.Method {
 	case http.MethodGet:
 		p, ok := h.principal(r)
@@ -25,20 +25,20 @@ func (h *Handlers) HandleAccount(w http.ResponseWriter, r *http.Request) {
 		}
 		u, err := h.store.GetUser(p.ID)
 		if err != nil {
-			logf("panel: account: get user failed: %v", err)
+			logf("panel: settings: get user failed: %v", err)
 			http.Error(w, "internal error", http.StatusInternalServerError)
 			return
 		}
-		h.renderAccount(w, r, http.StatusOK, "", u.Username, u.DMARCReportEmail, p.IsGlobal())
+		h.renderSettings(w, r, http.StatusOK, "", u.Username, u.DMARCReportEmail, p.IsGlobal())
 	case http.MethodPost:
-		h.submitAccount(w, r)
+		h.submitSettings(w, r)
 	default:
 		w.Header().Set("Allow", "GET, POST")
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 	}
 }
 
-func (h *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, status int, formErr, formUsername, formDMARCEmail string, showDMARC bool) {
+func (h *Handlers) renderSettings(w http.ResponseWriter, r *http.Request, status int, formErr, formUsername, formDMARCEmail string, showDMARC bool) {
 	var reportAuth dnscheck.Result
 	if showDMARC && formDMARCEmail != "" {
 		if hub := dnscheck.EmailDomain(formDMARCEmail); hub != "" {
@@ -58,13 +58,13 @@ func (h *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, status
 	data["ReportAuthDNS"] = reportAuth
 	data["ReportAuthHub"] = dnscheck.EmailDomain(formDMARCEmail)
 	data["Error"] = formErr
-	data["Flash"] = accountFlash(r)
+	data["Flash"] = settingsFlash(r)
 	data["L1Messages"] = h.l1Messages()
 	data["L1Window"] = h.l1Window()
 	h.view.Render(w, status, "settings", data)
 }
 
-func accountFlash(r *http.Request) string {
+func settingsFlash(r *http.Request) string {
 	switch r.URL.Query().Get("updated") {
 	case "username":
 		return "Username changed."
@@ -85,16 +85,16 @@ func accountFlash(r *http.Request) string {
 	}
 }
 
-func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
+func (h *Handlers) submitSettings(w http.ResponseWriter, r *http.Request) {
 	if !h.auth.AllowLoginAttempt(r) {
 		p, _ := h.principal(r)
-		h.renderAccount(w, r, http.StatusTooManyRequests,
+		h.renderSettings(w, r, http.StatusTooManyRequests,
 			"Too many attempts. Please wait and try again.", auth.CurrentUser(r), "", p.IsGlobal())
 		return
 	}
 	if err := r.ParseForm(); err != nil {
 		p, _ := h.principal(r)
-		h.renderAccount(w, r, http.StatusBadRequest, "Invalid form submission.", auth.CurrentUser(r), "", p.IsGlobal())
+		h.renderSettings(w, r, http.StatusBadRequest, "Invalid form submission.", auth.CurrentUser(r), "", p.IsGlobal())
 		return
 	}
 
@@ -105,7 +105,7 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 	}
 	user, err := h.store.GetUser(p.ID)
 	if err != nil {
-		logf("panel: account: get user failed: %v", err)
+		logf("panel: settings: get user failed: %v", err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
 		return
 	}
@@ -123,21 +123,21 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := bcrypt.CompareHashAndPassword([]byte(user.PasswordHash), []byte(current)); err != nil {
-		h.renderAccount(w, r, http.StatusUnauthorized, "Current password is incorrect.", username, dmarcEmail, p.IsGlobal())
+		h.renderSettings(w, r, http.StatusUnauthorized, "Current password is incorrect.", username, dmarcEmail, p.IsGlobal())
 		return
 	}
 
 	renaming := username != user.Username
 	if renaming {
 		if err := validate.Username(username); err != nil {
-			h.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, p.IsGlobal())
+			h.renderSettings(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, p.IsGlobal())
 			return
 		}
 	}
 
 	if p.IsGlobal() {
 		if err := validate.Email(dmarcEmail); err != nil {
-			h.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, true)
+			h.renderSettings(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, true)
 			return
 		}
 	}
@@ -147,16 +147,16 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 	repassword := password != "" || confirm != ""
 	if repassword {
 		if password != confirm {
-			h.renderAccount(w, r, http.StatusBadRequest, "New passwords do not match.", username, dmarcEmail, p.IsGlobal())
+			h.renderSettings(w, r, http.StatusBadRequest, "New passwords do not match.", username, dmarcEmail, p.IsGlobal())
 			return
 		}
 		if err := validate.AdminPassword(password); err != nil {
-			h.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, p.IsGlobal())
+			h.renderSettings(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail, p.IsGlobal())
 			return
 		}
 	}
 	if !renaming && !repassword && !emailChanging {
-		h.renderAccount(w, r, http.StatusBadRequest,
+		h.renderSettings(w, r, http.StatusBadRequest,
 			"Nothing to change: enter a new username, password, or DMARC report address.", username, dmarcEmail, p.IsGlobal())
 		return
 	}
@@ -165,8 +165,8 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 	if repassword {
 		newHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
 		if err != nil {
-			logf("panel: account: hashing password failed: %v", err)
-			h.renderAccount(w, r, http.StatusInternalServerError,
+			logf("panel: settings: hashing password failed: %v", err)
+			h.renderSettings(w, r, http.StatusInternalServerError,
 				"Internal error. Please try again.", username, dmarcEmail, p.IsGlobal())
 			return
 		}
@@ -174,17 +174,17 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := h.store.UpdateUser(user.ID, username, hash, dmarcEmail); err != nil {
-		logf("panel: account: update user failed: %v", err)
+		logf("panel: settings: update user failed: %v", err)
 		msg := "Could not save the changes. Please check the logs and try again."
 		if errors.Is(err, store.ErrUserNotFound) {
 			msg = "There is no user account to update."
 		}
 		if errors.Is(err, store.ErrUserExists) {
 			msg = "That username is already in use."
-			h.renderAccount(w, r, http.StatusConflict, msg, username, dmarcEmail, p.IsGlobal())
+			h.renderSettings(w, r, http.StatusConflict, msg, username, dmarcEmail, p.IsGlobal())
 			return
 		}
-		h.renderAccount(w, r, http.StatusInternalServerError, msg, username, dmarcEmail, p.IsGlobal())
+		h.renderSettings(w, r, http.StatusInternalServerError, msg, username, dmarcEmail, p.IsGlobal())
 		return
 	}
 
@@ -197,7 +197,7 @@ func (h *Handlers) submitAccount(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	logf("panel: user %d account updated (username: %t, password: %t, dmarc email: %t)", user.ID, renaming, repassword, emailChanging)
+	logf("panel: user %d settings updated (username: %t, password: %t, dmarc email: %t)", user.ID, renaming, repassword, emailChanging)
 	http.Redirect(w, r, "/settings?updated="+updatedFlag(renaming, repassword, emailChanging), http.StatusSeeOther)
 }