diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fdb818..4e5b70f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version in the new design. The log refreshes its rows without touching the filter being chosen. `deliveries`, `delivery`, `dmarc`, `dmarc_domain` and `dmarc_report` are off the ratchet. +- panel-redesign (stage 2): the **Inbound** pages are in the new design: the + domain list, the domain page (MX record, upstream, valid recipients) and + the delete confirmation. Every inbound domain page has a **Spam filter** + box and the list names the filter's state: the panel reads + `INBOUND_ANTISPAM_MILTER` and `INBOUND_ANTISPAM_MILTER_ACTION` and says + 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. ### Changed diff --git a/cmd/panel/antispam_test.go b/cmd/panel/antispam_test.go new file mode 100644 index 0000000..e892005 --- /dev/null +++ b/cmd/panel/antispam_test.go @@ -0,0 +1,34 @@ +package main + +import "testing" + +// The panel reads the inbound filter's two variables with the semantics of +// build/postfix-config.sh: the action is accept unless it says tempfail, and a +// value the script would refuse is not guessed at. +func TestAntispamActionMirrorsTheBuildScript(t *testing.T) { + for raw, want := range map[string]string{ + "": "accept", "accept": "accept", "tempfail": "tempfail", + "reject": "", "Accept": "", "TEMPFAIL": "", " accept": "", + } { + if got := antispamAction(raw); got != want { + t.Errorf("antispamAction(%q) = %q, want %q", raw, got, want) + } + } +} + +func TestLoadConfigReadsTheInboundFilter(t *testing.T) { + t.Setenv("INBOUND_RELAY_ENABLE", "true") + t.Setenv("INBOUND_ANTISPAM_MILTER", "inet:antispam:11332") + t.Setenv("INBOUND_ANTISPAM_MILTER_ACTION", "tempfail") + cfg := loadConfig() + if !cfg.inboundEnabled || cfg.inboundAntispamMilter != "inet:antispam:11332" || cfg.inboundAntispamAction != "tempfail" { + t.Errorf("config = %+v", cfg) + } + + t.Setenv("INBOUND_ANTISPAM_MILTER", "") + t.Setenv("INBOUND_ANTISPAM_MILTER_ACTION", "") + cfg = loadConfig() + if cfg.inboundAntispamMilter != "" || cfg.inboundAntispamAction != "accept" { + t.Errorf("unset filter: milter %q, action %q", cfg.inboundAntispamMilter, cfg.inboundAntispamAction) + } +} diff --git a/cmd/panel/envdoc_test.go b/cmd/panel/envdoc_test.go index 3425f38..cfb7675 100644 --- a/cmd/panel/envdoc_test.go +++ b/cmd/panel/envdoc_test.go @@ -89,6 +89,8 @@ var loadConfigKeys = []string{ "POSTFIX_DIR", "SELFPOST_DEPLOY_ROOT", "INBOUND_RELAY_ENABLE", + "INBOUND_ANTISPAM_MILTER", + "INBOUND_ANTISPAM_MILTER_ACTION", "DMARC_REPORTS_ENABLE", } diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index 35f1d9f..5c84a76 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -110,6 +110,8 @@ func newPanel(cfg config, st *store.Store) (*web.Server, error) { RateLimitWindowSeconds: cfg.rateLimitWindowSeconds, RetryPolicy: retryPolicy, InboundEnabled: cfg.inboundEnabled, + InboundAntispamMilter: cfg.inboundAntispamMilter, + InboundAntispamAction: cfg.inboundAntispamAction, DMARCEnabled: cfg.dmarcEnabled, SendLogRetentionEnvDefault: cfg.retentionDays, }, cfg.setupTokenPath) diff --git a/cmd/panel/main.go b/cmd/panel/main.go index e50c7c9..0905ff3 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -91,7 +91,12 @@ type config struct { deployRoot string inboundEnabled bool - dmarcEnabled bool + // The optional milter on the inbound listener (INBOUND_ANTISPAM_MILTER) and + // what Postfix does when it is down. Read-only: build/postfix-config.sh is + // what attaches it; the panel only says so. See antispamAction. + inboundAntispamMilter string + inboundAntispamAction string + dmarcEnabled bool } func loadConfig() config { @@ -154,12 +159,28 @@ func loadConfig() config { postfixDir: envDefault("POSTFIX_DIR", filepath.Join(dataDir, "postfix")), deployRoot: envDefault("SELFPOST_DEPLOY_ROOT", "/selfpost-deploy"), // Optional inbound relay (backup-MX / forwarder). Off unless exactly "true". - inboundEnabled: os.Getenv("INBOUND_RELAY_ENABLE") == "true", + inboundEnabled: os.Getenv("INBOUND_RELAY_ENABLE") == "true", + inboundAntispamMilter: os.Getenv("INBOUND_ANTISPAM_MILTER"), + inboundAntispamAction: antispamAction(os.Getenv("INBOUND_ANTISPAM_MILTER_ACTION")), // Optional DMARC aggregate ingest on port 25. Off unless exactly "true". dmarcEnabled: os.Getenv("DMARC_REPORTS_ENABLE") == "true", } } +// antispamAction reads INBOUND_ANTISPAM_MILTER_ACTION the way +// build/postfix-config.sh does: empty means accept, and accept and tempfail are +// the only other values it lets the container start with. Anything else is +// returned as "" — the panel says nothing about the action rather than guess. +func antispamAction(raw string) string { + switch raw { + case "", "accept": + return "accept" + case "tempfail": + return "tempfail" + } + return "" +} + // saslRealm chooses the realm new SASL accounts live under. It mirrors the // hostname Postfix's SASL layer uses so a client authenticating with a bare // login resolves to the right account. diff --git a/docs/assets/panel-redesign/evidence/in-domain-delete.png b/docs/assets/panel-redesign/evidence/in-domain-delete.png new file mode 100644 index 0000000..9f554af Binary files /dev/null and b/docs/assets/panel-redesign/evidence/in-domain-delete.png differ diff --git a/docs/assets/panel-redesign/evidence/in-domain.png b/docs/assets/panel-redesign/evidence/in-domain.png new file mode 100644 index 0000000..c1d60c9 Binary files /dev/null and b/docs/assets/panel-redesign/evidence/in-domain.png differ diff --git a/docs/assets/panel-redesign/evidence/in-domains.png b/docs/assets/panel-redesign/evidence/in-domains.png new file mode 100644 index 0000000..1a4242e Binary files /dev/null and b/docs/assets/panel-redesign/evidence/in-domains.png differ diff --git a/docs/plans/panel-redesign.md b/docs/plans/panel-redesign.md index 9b20b05..62e54a3 100644 --- a/docs/plans/panel-redesign.md +++ b/docs/plans/panel-redesign.md @@ -517,7 +517,7 @@ only when both are done. Every stage-2 step ends with the evidence of - [x] Overview, Health, and the Account / Settings split — **Sonnet** - [x] Outbound: domains, domain, domain settings, application form, shown-once password, delete — **Sonnet** - [x] Outbound: log, message, DMARC hub / domain / report — **Sonnet** -- [ ] Inbound: domains, domain with the *Spam filter* box, delete — **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** - [ ] 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 273d30b..b378292 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 | 19/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | +| panel-redesign | Panel redesign (accepted mockups, design contract) | agreed | 20/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.go b/internal/web/handlers/handlers.go index c8742f0..286e0d9 100644 --- a/internal/web/handlers/handlers.go +++ b/internal/web/handlers/handlers.go @@ -39,6 +39,12 @@ type Config struct { // InboundEnabled mirrors INBOUND_RELAY_ENABLE: the inbound panel and // routes exist only when this is true. InboundEnabled bool + // InboundAntispamMilter mirrors INBOUND_ANTISPAM_MILTER and + // InboundAntispamAction INBOUND_ANTISPAM_MILTER_ACTION ("accept", + // "tempfail", or "" for a value build/postfix-config.sh would refuse). The + // filter is on only when the inbound relay is on and the milter is set. + InboundAntispamMilter string + InboundAntispamAction string // DMARCEnabled mirrors DMARC_REPORTS_ENABLE. DMARCEnabled bool // SendLogRetentionEnvDefault is SEND_LOG_RETENTION_DAYS at panel start; used diff --git a/internal/web/handlers/handlers_delegation_test.go b/internal/web/handlers/handlers_delegation_test.go index 1c243a6..1d999a3 100644 --- a/internal/web/handlers/handlers_delegation_test.go +++ b/internal/web/handlers/handlers_delegation_test.go @@ -224,7 +224,7 @@ func TestInboundListShowsOnlyTheAssignedDomains(t *testing.T) { if !strings.Contains(body, "a.example.com") || strings.Contains(body, "b.example.com") { t.Errorf("the administrator of A must see A and not B:\n%s", body) } - for _, globalOnly := range []string{"Add inbound domain", "/delete"} { + for _, globalOnly := range []string{"Add a domain", "/delete"} { if strings.Contains(body, globalOnly) { t.Errorf("the list offers a domain administrator %q", globalOnly) } @@ -249,7 +249,7 @@ func TestInboundListShowsOnlyTheAssignedDomains(t *testing.T) { t.Errorf("list with %s = %d, want 404", who, rec.Code) } } - if body := list(&s.global).Body.String(); !strings.Contains(body, "Add inbound domain") || !strings.Contains(body, "b.example.com") { + if body := list(&s.global).Body.String(); !strings.Contains(body, "Add a domain") || !strings.Contains(body, "b.example.com") { t.Errorf("the global role lost the full list or the add form:\n%s", body) } } diff --git a/internal/web/handlers/handlers_inbound.go b/internal/web/handlers/handlers_inbound.go index 6630b32..b5179f3 100644 --- a/internal/web/handlers/handlers_inbound.go +++ b/internal/web/handlers/handlers_inbound.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "strconv" - "strings" "sync" "github.com/mixeme/selfpost/internal/dnscheck" @@ -13,16 +12,9 @@ import ( "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/auth" "github.com/mixeme/selfpost/internal/web/validate" + "github.com/mixeme/selfpost/internal/web/view" ) -type inboundRow struct { - store.InboundDomain - DNS health.Status - Upstream string - TLSLabel string - RcptLabel string -} - // Inbound is delegated to domain administrators separately from outbound (plan // § Who sees what), so its handlers come in three strengths: // @@ -124,34 +116,37 @@ func (h *Handlers) renderInboundList(w http.ResponseWriter, r *http.Request, sta http.Error(w, "internal error", http.StatusInternalServerError) return } - data := h.pageBase(r) - data["Title"] = "SelfPost — inbound" - data["Active"] = "inbound" - data["Domains"] = h.inboundRows(list) - data["Error"] = formErr - data["FormName"] = formName + flash := "" if r.URL.Query().Get("deleted") != "" { - data["Flash"] = "Inbound domain deleted." + flash = "Inbound domain deleted." } - h.view.Render(w, status, "inbound", data) + on, _, _ := h.inboundFilter() + page := view.NewInDomains(h.shellMeta(r), p.IsGlobal()). + WithFilter(on). + WithRows(h.inboundRows(list, p.IsGlobal())). + WithResult(flash, formErr, formName) + h.view.Render(w, status, "in-domains", page) } -func (h *Handlers) inboundRows(domains []store.InboundDomain) []inboundRow { - rows := make([]inboundRow, len(domains)) +// inboundRows is one row per inbound domain: the verdict of the MX check +// (cached for a few minutes, as on the domain page), the upstream, who is +// accepted and the TLS to the upstream. The checks run side by side. +func (h *Handlers) inboundRows(domains []store.InboundDomain, canDelete bool) []view.InDomainRow { + rows := make([]view.InDomainRow, len(domains)) var wg sync.WaitGroup for i, d := range domains { - rows[i] = inboundRow{ - InboundDomain: d, - DNS: health.StatusUnknown, - Upstream: inboundUpstream(d), - TLSLabel: tlsLabel(d.TLSMode), - RcptLabel: rcptLabel(d), + in := view.InDomainInput{ + ID: d.ID, Name: d.Name, DNSStatus: string(health.StatusUnknown), + Host: d.Host, Port: d.Port, TLSMode: d.TLSMode, + RecipientMode: d.RecipientMode, RecipientCount: d.RecipientCount, } + rows[i] = view.NewInDomainRow(in, canDelete) wg.Add(1) go func() { defer wg.Done() if h.dns != nil && h.cfg.Hostname != "" { - rows[i].DNS = h.dns.InboundMX(d.Name, h.cfg.Hostname, false).Status + in.DNSStatus = string(h.dns.InboundMX(d.Name, h.cfg.Hostname, false).Status) + rows[i] = view.NewInDomainRow(in, canDelete) } }() } @@ -159,42 +154,6 @@ func (h *Handlers) inboundRows(domains []store.InboundDomain) []inboundRow { return rows } -func inboundUpstream(d store.InboundDomain) string { - if d.Host == "" { - return "—" - } - return fmt.Sprintf("%s:%d", d.Host, d.Port) -} - -func tlsLabel(mode string) string { - switch mode { - case store.TLSModeEncrypt: - return "required" - case store.TLSModeNone: - return "off" - default: - return "opportunistic" - } -} - -func tlsStatusClass(mode string) string { - if mode == store.TLSModeEncrypt { - return "ok" - } - return "unknown" -} - -func rcptLabel(d store.InboundDomain) string { - if d.RecipientMode == store.RecipientModeAny { - return "any" - } - n := d.RecipientCount - if n == 1 { - return "1 listed" - } - return fmt.Sprintf("%d listed", n) -} - // HandleAddInbound validates the name, creates the inbound domain, and // redirects to its page. func (h *Handlers) HandleAddInbound(w http.ResponseWriter, r *http.Request) { @@ -244,21 +203,35 @@ func (h *Handlers) renderInboundDetail(w http.ResponseWriter, r *http.Request, s if h.dns != nil && h.cfg.Hostname != "" { mx = h.dns.InboundMX(d.Name, h.cfg.Hostname, false) } - data := h.pageBase(r) - data["Title"] = "SelfPost — " + d.Name - data["Active"] = "inbound" - data["Domain"] = d - data["MX"] = mx - data["MXValue"] = "10 " + strings.TrimSuffix(h.cfg.Hostname, ".") + "." - data["Hostname"] = h.cfg.Hostname - data["TLSLabel"] = tlsLabel(d.TLSMode) - data["TLSClass"] = tlsStatusClass(d.TLSMode) - data["RecipientText"] = strings.Join(d.Recipients, "\n") - data["Flash"] = inboundFlash(r) - data["FormErr"] = extra.FormErr - data["TransportErr"] = extra.TransportErr - data["RecipientErr"] = extra.RecipientErr - h.view.Render(w, status, "inbound_domain", data) + on, milter, action := h.inboundFilter() + page := view.NewInDomain(h.shellMeta(r), view.InDomainInput{ + ID: d.ID, Name: d.Name, DNSStatus: string(mx.Status), Host: d.Host, Port: d.Port, TLSMode: d.TLSMode, + RecipientMode: d.RecipientMode, RecipientCount: d.RecipientCount, Addresses: d.Recipients, + }). + WithMX(h.cfg.Hostname, view.DNSCheck{Status: string(mx.Status), Detail: mx.Detail, Found: mx.Records}). + WithFilter(on, milter, action). + WithResult(inboundFlash(r), firstNonEmpty(extra.FormErr, extra.TransportErr, extra.RecipientErr)) + h.view.Render(w, status, "in-domain", page) +} + +// inboundFilter says whether the instance filters inbound mail, and with what: +// the milter address and what Postfix does when it is down. It mirrors +// build/postfix-config.sh, which attaches the filter only when the inbound +// relay is on and a milter is set. +func (h *Handlers) inboundFilter() (on bool, milter, action string) { + if !h.cfg.InboundEnabled || h.cfg.InboundAntispamMilter == "" { + return false, "", "" + } + return true, h.cfg.InboundAntispamMilter, h.cfg.InboundAntispamAction +} + +func firstNonEmpty(ss ...string) string { + for _, s := range ss { + if s != "" { + return s + } + } + return "" } func inboundFlash(r *http.Request) string { @@ -330,12 +303,12 @@ func (h *Handlers) HandleInboundDeleteConfirm(w http.ResponseWriter, r *http.Req if !ok { return } - data := h.pageBase(r) - data["Title"] = "SelfPost — delete " + d.Name - data["Active"] = "inbound" - data["Domain"] = d - data["Upstream"] = inboundUpstream(d) - h.view.Render(w, http.StatusOK, "inbound_delete", data) + listed := 0 + if d.RecipientMode != store.RecipientModeAny { + listed = d.RecipientCount + } + h.view.Render(w, http.StatusOK, "in-domain-delete", + view.NewInDomainDelete(h.shellMeta(r), d.ID, d.Name, d.Host != "", listed)) } func (h *Handlers) HandleInboundDelete(w http.ResponseWriter, r *http.Request) { diff --git a/internal/web/handlers/handlers_inbound_domain_test.go b/internal/web/handlers/handlers_inbound_domain_test.go new file mode 100644 index 0000000..3d9ea55 --- /dev/null +++ b/internal/web/handlers/handlers_inbound_domain_test.go @@ -0,0 +1,130 @@ +package handlers + +import ( + "net/http" + "net/url" + "reflect" + "testing" +) + +// The page of one inbound domain is filled from the store, the DNS checker and +// the filter settings; each form's refusal comes back on it as a flash. +func TestInboundDomainPageFromTheStore(t *testing.T) { + h, st := inboundHandlers(t) + d, err := st.AddInboundDomain("lists.example.com") + if err != nil { + t.Fatal(err) + } + id := itoa(d.ID) + if err := h.inbound.SetTransport(d.ID, "mx.internal", "2525", "encrypt"); err != nil { + t.Fatal(err) + } + if err := h.inbound.SetRecipients(d.ID, "list", []string{"a@lists.example.com", "b@lists.example.com"}); err != nil { + t.Fatal(err) + } + + body := inboundCall(h, http.MethodGet, "/inbound/domains/"+id, nil, globalPrincipal).Body.String() + has(t, "domain page", body, `
INBOUND_ANTISPAM_MILTER)`)
+ lacks(t, "filter off", body, `Applies to`)
+ has(t, "filter off, domain administrator", get(true), `set by the server's administrator`)
+ lacks(t, "filter off, domain administrator", get(true), `INBOUND_ANTISPAM_MILTER`)
+
+ s.h.cfg.InboundAntispamMilter, s.h.cfg.InboundAntispamAction = "inet:antispam:11332", "tempfail"
+ body = get(false)
+ has(t, "filter on", body, `Every inbound domain of this server`, `inet:antispam:11332`, `Mail is deferred until it answers`)
+ body = get(true)
+ has(t, "filter on, domain administrator", body, `Every inbound domain of this server`, `Mail is deferred until it answers`)
+ lacks(t, "filter on, domain administrator", body, `inet:antispam`, `/delete`)
+
+ s.h.cfg.InboundAntispamAction = "accept"
+ has(t, "filter on, accept", get(false), `Mail is accepted unfiltered`)
+ s.h.cfg.InboundAntispamAction = ""
+ lacks(t, "filter on, action unknown", get(false), `If the filter is down`)
+
+ // The list says the same in its head.
+ list := send(s.h.HandleInboundList, &s.global, http.MethodGet, "/inbound/domains", nil, nil).Body.String()
+ has(t, "list", list, `Spam filter on · 2 domains`)
+ s.h.cfg.InboundAntispamMilter = ""
+ list = send(s.h.HandleInboundList, &s.global, http.MethodGet, "/inbound/domains", nil, nil).Body.String()
+ has(t, "list", list, `Spam filter off · 2 domains`)
+}
diff --git a/internal/web/handlers/handlers_inbound_test.go b/internal/web/handlers/handlers_inbound_test.go
index 20f4615..0094c6e 100644
--- a/internal/web/handlers/handlers_inbound_test.go
+++ b/internal/web/handlers/handlers_inbound_test.go
@@ -82,7 +82,7 @@ func TestInboundListAndAdd(t *testing.T) {
if rec.Code != http.StatusOK {
t.Fatalf("list = %d\n%s", rec.Code, rec.Body.String())
}
- if !strings.Contains(rec.Body.String(), "Add inbound domain") {
+ if !strings.Contains(rec.Body.String(), "Add a domain") {
t.Fatal("list missing add form")
}
@@ -158,3 +158,72 @@ func TestInboundDelete(t *testing.T) {
t.Fatal("domain still present")
}
}
+
+// The list and the confirmation are the kit pages: the handlers fill them from
+// what is stored, and a refused add comes back on the list with what was typed.
+func TestInboundListPageFromTheStore(t *testing.T) {
+ h, st := inboundHandlers(t)
+ d, err := st.AddInboundDomain("lists.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := h.inbound.SetTransport(d.ID, "mx.internal", "2525", "encrypt"); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := st.AddInboundDomain("bare.example.com"); err != nil {
+ t.Fatal(err)
+ }
+
+ body := inboundCall(h, http.MethodGet, "/inbound/domains", nil, globalPrincipal).Body.String()
+ has(t, "list", body, `INBOUND_ANTISPAM_MILTER).`)
+ pageLacks(t, "filter off, global", out, `