diff --git a/internal/web/handlers/handlers_inbound_log.go b/internal/web/handlers/handlers_inbound_log.go index 3e8430d..8499f3a 100644 --- a/internal/web/handlers/handlers_inbound_log.go +++ b/internal/web/handlers/handlers_inbound_log.go @@ -90,7 +90,7 @@ func (h *Handlers) HandleInboundLog(w http.ResponseWriter, r *http.Request) { table := make([]view.InLogRow, len(rows)) for i, row := range rows { table[i] = out.NewRow(view.InLogRowInput{ - At: row.CreatedAt, From: row.From, Client: row.ClientIP, To: row.To, Subject: row.Subject, + ID: row.ID, At: row.CreatedAt, From: row.From, Client: row.ClientIP, To: row.To, Subject: row.Subject, Decision: row.Decision, Engine: row.Engine, Detail: row.Detail, }) } diff --git a/internal/web/handlers/handlers_inbound_log_test.go b/internal/web/handlers/handlers_inbound_log_test.go index 1c52389..9cd9864 100644 --- a/internal/web/handlers/handlers_inbound_log_test.go +++ b/internal/web/handlers/handlers_inbound_log_test.go @@ -94,9 +94,9 @@ func TestInboundLogForTheGlobalRole(t *testing.T) { `Rows older than 30 days are deleted — change that in Settings.`, `alice@a.example.com`, `bob@b.example.com`, `Hello A`, `Hello B`, ``, ``, - `accept`, `quarantine`, + `class="tag is-success is-light" popovertarget="why-`, `class="tag is-primary is-light" popovertarget="why-`, `203.0.113.9`, - `rspamd · detail of Hello A`, + `Decided byrspamdVerdictdetail of Hello A`, `Deny sender`, `Allow sender`, `Page 1 of 1`, `Allow / Deny sender opens Filter lists`, diff --git a/internal/web/view/components.go b/internal/web/view/components.go index 7c70294..21e0870 100644 --- a/internal/web/view/components.go +++ b/internal/web/view/components.go @@ -105,6 +105,27 @@ type Tag struct { Label string } +// Sheet is what does not fit a table row, opened over the page from the row's +// tag (partials "sheet_tag" and "sheet"; used by the Inbound log, where it says +// why a message was decided the way it was). It is the browser's own popover: +// the tag is a button that names the sheet by ID, so ID is unique on the page. +// Its content is a table of name and value. +type Sheet struct { + ID string + Tag Tag + Title string + Rows []SheetRow +} + +// SheetRow is one name and its value in a sheet. A value of several lines is +// one line each; Mono is for what is read as written - a reply, a list of +// symbols. +type SheetRow struct { + Label string + Lines []string + Mono bool +} + // statusClass is the one mapping of status to Bulma tag classes — box 03 of the // kit page. A page passes the status it knows (a DKIM check says ok, a message // says deferred) and never picks a colour. diff --git a/internal/web/view/fixtures_test.go b/internal/web/view/fixtures_test.go index 0254c83..a799946 100644 --- a/internal/web/view/fixtures_test.go +++ b/internal/web/view/fixtures_test.go @@ -420,12 +420,14 @@ func userDeleteFixture() *UserDelete { func inLogFixture() *InLog { p := NewInLog(admin(), 90, []string{"lists.example.org", "acme.io"}, InLogFilter{From: "2026-09-14", To: "2026-09-21"}) + var id int64 row := func(clock, from, client, to, subject, decision, engine, detail string) InLogRow { at, err := time.Parse("2006-01-02 15:04:05", "2026-09-21 "+clock) if err != nil { panic(err) } - return p.NewRow(InLogRowInput{At: at, From: from, Client: client, To: to, Subject: subject, + id++ + return p.NewRow(InLogRowInput{ID: id, At: at, From: from, Client: client, To: to, Subject: subject, Decision: decision, Engine: engine, Detail: detail}) } return p.WithRows([]InLogRow{ diff --git a/internal/web/view/inbound_log.go b/internal/web/view/inbound_log.go index 0c37041..d29f93c 100644 --- a/internal/web/view/inbound_log.go +++ b/internal/web/view/inbound_log.go @@ -12,6 +12,7 @@ package view import ( "net/url" "strconv" + "strings" "time" ) @@ -67,23 +68,24 @@ type InLog struct { // InLogRow is one recipient of one message: when it was decided, who the mail // was from (and the address of the server that offered it) and for, its subject, -// the decision as a tag, who decided and why, and — for the global role, who -// alone edits the filter lists — the way to correct the decision. +// the decision as a tag that opens the sheet saying who decided and why, and — +// for the global role, who alone edits the filter lists — the way to correct +// the decision. type InLogRow struct { - Time string - From string - Client string - To string - Subject string - Decision Tag - Why string - Action *Anchor + Time string + From string + Client string + To string + Subject string + Why Sheet + Action *Anchor } // InLogRowInput is what the journal holds about one row. Subject, Detail and // From may be empty: refused mail is read from mail.log, which has no subject, // and a filter that said nothing has no detail. type InLogRowInput struct { + ID int64 At time.Time From string Client string @@ -140,13 +142,12 @@ func NewInLog(m Meta, retentionDays int, domains []string, filter InLogFilter) * // NewRow words one row of the journal for the viewer of the page. func (p *InLog) NewRow(in InLogRowInput) InLogRow { - why := in.Engine - if in.Detail != "" { - why += " · " + in.Detail - } r := InLogRow{ Time: FormatLogTime(in.At), From: orDash(in.From), Client: in.Client, To: in.To, Subject: in.Subject, - Decision: Tag{Status: in.Decision}, Why: why, + Why: Sheet{ + ID: "why-" + strconv.FormatInt(in.ID, 10), Tag: Tag{Status: in.Decision}, Title: "Why", + Rows: whyRows(in.Engine, in.Detail, in.Subject), + }, } if p.IsGlobal { r.Action = senderAction(in.Decision, in.Engine, in.From) @@ -154,11 +155,36 @@ func (p *InLog) NewRow(in InLogRowInput) InLogRow { return r } +// whyRows is the content of a row's sheet: who decided, what they said, and +// the subject whole, which the table clips. What was said has one of two +// shapes, told apart by how it starts. The inbound journal milter writes the +// filter's verdict on mail that passed - an action and a score, then the +// symbols after a semicolon. The log tailer writes the reply a refusal was +// given, which starts with its code. +func whyRows(engine, detail, subject string) []SheetRow { + rows := []SheetRow{{Label: "Decided by", Lines: []string{engine}}} + switch { + case detail == "": + case detail[0] >= '0' && detail[0] <= '9': + rows = append(rows, SheetRow{Label: "Reply", Lines: []string{detail}, Mono: true}) + default: + verdict, symbols, _ := strings.Cut(detail, "; ") + rows = append(rows, SheetRow{Label: "Verdict", Lines: []string{verdict}}) + if symbols != "" { + rows = append(rows, SheetRow{Label: "Symbols", Lines: strings.Fields(symbols), Mono: true}) + } + } + if subject != "" { + rows = append(rows, SheetRow{Label: "Subject", Lines: []string{subject}}) + } + return rows +} + // senderAction is the way to correct a decision from its row, if there is one: -// a message that got through is the one to deny its sender for, a message the -// filter turned away or put off is the one to allow its sender for, and a -// refusal by Postfix's own policy (an unknown recipient, say) can only be -// answered by denying — allowing the sender would not change what Postfix +// a message that got through is the one to deny its sender for, and a message +// the filter turned away, put off or held is the one to allow its sender for. +// A refusal by Postfix's own policy (an unknown recipient, say) offers nothing: +// the message is refused already and the lists would not change what Postfix // thinks of the recipient. A row with no sender has nobody to list. func senderAction(decision, engine, from string) *Anchor { if from == "" { @@ -170,8 +196,6 @@ func senderAction(decision, engine, from string) *Anchor { list = "deny" case engine == "rspamd": list = "allow" - case decision == "reject" && engine == "postfix": - list = "deny" default: return nil } @@ -199,7 +223,9 @@ func (p *InLog) WithRows(rows []InLogRow, page, last int) *InLog { p.Foot.Link = &Anchor{Href: InLogHref(p.filter, page+1), Label: "Older →"} } if p.IsGlobal { - p.Foot.End = Rich("Allow / Deny sender opens ", Link(inboundFilterLists, "Filter lists"), " with the entry filled in") + p.Foot.End = Rich("A decision opens its reason · Allow / Deny sender opens ", Link(inboundFilterLists, "Filter lists"), " with the entry filled in") + } else { + p.Foot.End = Plain("A decision opens its reason") } return p } diff --git a/internal/web/view/inbound_log_test.go b/internal/web/view/inbound_log_test.go index 50b6df1..a151f69 100644 --- a/internal/web/view/inbound_log_test.go +++ b/internal/web/view/inbound_log_test.go @@ -22,19 +22,29 @@ func TestInLogShowsTheRowsAndTheirFilters(t *testing.T) { ``, ``, `href="/help#inbound-log" title="What the decisions mean"`, - `Time, UTCFromToSubjectDecisionWhy`, + `Time, UTCFromToSubjectDecision`, `09-21 14:00:18`, `t.berg@uni.example
192.0.2.77`, `dev@lists.example.org`, `title="Re: [dev] patch series v3"`, `—`, - `accept`, `reject`, - `tempfail`, - `rspamd · no action, score 0.40 / 15.00`, - `postfix · 550 5.1.1 <sales@lists.example.org>: Recipient address rejected`, + // the decision is a tag that opens the sheet with who decided and why + `
`, + `
accept

Why

`, + ``, + `Decided byrspamdVerdictno action, score 0.40 / 15.00SubjectRe: [dev] patch series v3`, + `Verdictadd header, score 9.80 / 15.00SymbolsPHISHING(5.00)
FORGED_SENDER(3.00)
R_SPF_FAIL(1.00)`, + `Decided bypostfixReply550 5.1.1 <sales@lists.example.org>: Recipient address rejected: User unknown in relay recipient table`, `Page 1 of 38`, `Older →`, - `Allow / Deny sender opens Filter lists with the entry filled in`, + `A decision opens its reason · Allow / Deny sender opens Filter lists with the entry filled in`, ) - pageLacks(t, "Inbound log", out, `← Newer`, `data-poll`, `hx-get`) + pageLacks(t, "Inbound log", out, `← Newer`, `data-poll`, `hx-get`, `Why`) + // A message Postfix itself refused offers no way into the lists: one Deny + // for each of the three accepted messages, one Allow for each the filter + // refused or put off. + if deny, allow := strings.Count(out, `>Deny sender`), strings.Count(out, `>Allow sender`); deny != 3 || allow != 2 { + t.Errorf("the fixture's six rows offer Deny %d times and Allow %d times, want 3 and 2", deny, allow) + } // Without the global role there is nowhere to change the retention, and // nowhere to send a sender. @@ -96,11 +106,11 @@ func TestInLogRowWording(t *testing.T) { p := NewInLog(admin(), 90, nil, InLogFilter{}) at := time.Date(2026, 9, 21, 14, 0, 18, 0, time.UTC) r := p.NewRow(InLogRowInput{At: at, From: "a@example.net", To: "b@acme.io", Decision: "accept", Engine: "selfpost"}) - if r.Why != "selfpost" || r.Time != "09-21 14:00:18" || r.Client != "" { + if len(r.Why.Rows) != 1 || r.Why.Rows[0].Lines[0] != "selfpost" || r.Time != "09-21 14:00:18" || r.Client != "" { t.Errorf("row = %+v", r) } - r = p.NewRow(InLogRowInput{At: at, To: "b@acme.io", Decision: "reject", Engine: "postfix", Detail: "550 5.1.1 no"}) - if r.Why != "postfix · 550 5.1.1 no" || r.From != "—" || r.Action != nil { + r = p.NewRow(InLogRowInput{ID: 7, At: at, To: "b@acme.io", Decision: "reject", Engine: "postfix", Detail: "550 5.1.1 no"}) + if r.Why.ID != "why-7" || r.Why.Tag.Status != "reject" || len(r.Why.Rows) != 2 || r.Why.Rows[1].Label != "Reply" || r.From != "—" || r.Action != nil { t.Errorf("row = %+v", r) } out := renderSignedIn(t, "in-log", p.WithRows([]InLogRow{r}, 1, 1)) @@ -110,7 +120,7 @@ func TestInLogRowWording(t *testing.T) { // Which decisions lead to which list, for the global role: the sender of mail // that got through can be denied, the sender of mail the filter turned away or -// put off can be allowed, and a refusal that is Postfix's own can only be denied. +// put off can be allowed, and a refusal that is Postfix's own offers neither. func TestInLogSenderAction(t *testing.T) { for _, c := range []struct { decision, engine, from, label, href string @@ -120,7 +130,7 @@ func TestInLogSenderAction(t *testing.T) { {"reject", "rspamd", "a+b@example.net", "Allow sender", "/inbound/filter-lists?list=allow&kind=address&value=a%2Bb%40example.net"}, {"tempfail", "rspamd", "a@example.net", "Allow sender", "/inbound/filter-lists?list=allow&kind=address&value=a%40example.net"}, {"quarantine", "rspamd", "a@example.net", "Allow sender", "/inbound/filter-lists?list=allow&kind=address&value=a%40example.net"}, - {"reject", "postfix", "a@example.net", "Deny sender", "/inbound/filter-lists?list=deny&kind=address&value=a%40example.net"}, + {"reject", "postfix", "a@example.net", "", ""}, {"tempfail", "postfix", "a@example.net", "", ""}, {"reject", "selfpost", "a@example.net", "", ""}, {"accept", "rspamd", "", "", ""}, @@ -254,3 +264,29 @@ func TestInboundMenuHasLogAndFilterLists(t *testing.T) { pageHas(t, "Inbound log", out, ``, ``) pageLacks(t, "Inbound log", out, `/inbound/filter-lists`) } + +// What a sheet says, from the two shapes a row's detail has: the filter's +// verdict on mail that passed, and the reply a refusal was given. +func TestWhyRows(t *testing.T) { + flat := func(rows []SheetRow) string { + var out []string + for _, r := range rows { + out = append(out, r.Label+"="+strings.Join(r.Lines, "|")) + } + return strings.Join(out, "; ") + } + for _, c := range []struct{ engine, detail, subject, want string }{ + {"selfpost", "", "", "Decided by=selfpost"}, + {"selfpost", "", "Hello", "Decided by=selfpost; Subject=Hello"}, + {"rspamd", "no action, score 0.40 / 15.00", "Hi", "Decided by=rspamd; Verdict=no action, score 0.40 / 15.00; Subject=Hi"}, + {"rspamd", "add header, score 9.80 / 15.00; A(5.00) B(-0.10)", "", "Decided by=rspamd; Verdict=add header, score 9.80 / 15.00; Symbols=A(5.00)|B(-0.10)"}, + // a verdict with symbols and no action or score + {"rspamd", "A(5.00)", "", "Decided by=rspamd; Verdict=A(5.00)"}, + {"rspamd", "5.7.1 Spam message rejected", "", "Decided by=rspamd; Reply=5.7.1 Spam message rejected"}, + {"postfix", "450 4.1.8 : Sender address rejected; see x", "", "Decided by=postfix; Reply=450 4.1.8 : Sender address rejected; see x"}, + } { + if got := flat(whyRows(c.engine, c.detail, c.subject)); got != c.want { + t.Errorf("whyRows(%q, %q, %q)\n got: %s\nwant: %s", c.engine, c.detail, c.subject, got, c.want) + } + } +} diff --git a/internal/web/view/kit.go b/internal/web/view/kit.go index fdbbe54..15334ac 100644 --- a/internal/web/view/kit.go +++ b/internal/web/view/kit.go @@ -21,6 +21,7 @@ type Kit struct { Postmarks []Postmark StatusTag Box StatusRows []KitStatusRow + Sheet Sheet BoxHead Box BoxFoot Foot DangerZone Box @@ -91,6 +92,10 @@ func KitPage() *Kit { }, StatusTag: Box{No: "03", Title: "status_tag", End: Plain("One mapping, one function")}, + Sheet: Sheet{ID: "kit-sheet", Tag: Tag{Status: "reject"}, Title: "sheet", Rows: []SheetRow{ + {Label: "What it holds", Lines: []string{"What does not fit the row, as name and value"}}, + {Label: "How it closes", Lines: []string{"Esc", "a click outside", "the cross"}, Mono: true}, + }}, StatusRows: []KitStatusRow{ {"ok, published, running, sent, accept", Tag{Status: "ok"}}, {"warn, deferred, quiet, tempfail", Tag{Status: "warn"}}, diff --git a/internal/web/view/static/panel.css b/internal/web/view/static/panel.css index 0e09b0d..d12ead6 100644 --- a/internal/web/view/static/panel.css +++ b/internal/web/view/static/panel.css @@ -327,3 +327,27 @@ code { background: var(--sp-well); color: var(--sp-ink); border-radius: 3px; pad .sp-record-head small { margin-left: 0; text-align: left; flex-basis: 100%; } .sp-postmark { display: none; } } + +/* sheet: what does not fit a table row, opened over the page from the row's + tag. It is the browser's own popover (the tag is a button with + popovertarget), so it needs no script: Esc, a click outside and the close + button all shut it, and focus returns to the tag. It rises from the bottom + edge across the whole window - the one component that is not laid out by the + page, hence the widths - with its content, a table of name and value, in a + column of reading width in the middle. */ +.sp-sheet { + inset: auto 0 0 0; width: auto; margin: 0; padding: 0; border: 0; border-top: 3px solid var(--sp-brick); + max-height: 75vh; overflow: auto; background: #fff; color: inherit; box-shadow: 0 -12px 40px rgba(40, 20, 15, .22); +} +.sp-sheet-in { max-width: 44rem; margin: 0 auto; padding: 0 1.25rem; } +.sp-sheet::backdrop { background: rgba(40, 20, 15, .38); } +.sp-sheet-head { display: flex; align-items: center; gap: .7rem; padding: 1rem 0 .8rem; border-bottom: 1px solid var(--sp-rule); } +.sp-sheet-head h2 { font-size: 1.15rem; font-weight: 600; margin-right: auto; } +.sp-sheet-head button { border: 0; background: none; cursor: pointer; font-size: 1.3rem; color: var(--sp-muted); } +.sp-sheet-body { padding: .4rem 0 1.4rem; } +/* The sheet's content is a table of name and value: the name is a label, set like the dt of facts. */ +.sp-sheet .table th { font-family: var(--sp-mono); font-size: var(--sp-fs-xs); font-weight: 500; letter-spacing: .08em; text-transform: uppercase; color: var(--sp-muted); white-space: nowrap; vertical-align: top; width: 1%; padding-right: 2rem; } +.sp-sheet .table td { overflow-wrap: anywhere; } +/* A tag that opens a sheet is a button: it keeps the tag's look and says it can + be pressed with the dotted underline links to more detail carry. */ +button.tag { border: 0; cursor: pointer; text-decoration: underline dotted; text-underline-offset: 3px; } diff --git a/internal/web/view/templates/components.html b/internal/web/view/templates/components.html index 9e82191..b6fea26 100644 --- a/internal/web/view/templates/components.html +++ b/internal/web/view/templates/components.html @@ -27,6 +27,13 @@ mapping of status to Bulma classes; the label defaults to the status. */}} {{define "tag"}}{{if .Label}}{{.Label}}{{else}}{{.Status}}{{end}}{{end}} +{{/* sheet_tag is a status tag that opens a sheet (input: Sheet): the tag as a + button, and the sheet it names right after it. sheet is what opens: the tag + again, a title, the cross that closes it, and a table of name and value. + The browser's popover does the rest - no script. */}} +{{define "sheet_tag"}}{{template "sheet" .}}{{end}} +{{define "sheet"}}
{{template "tag" .Tag}}

{{.Title}}

{{range .Rows}}{{range $i, $line := .Lines}}{{if $i}}
{{end}}{{$line}}{{end}}{{end}}
{{.Label}}
{{end}} + {{/* kicker is the upper-case mono micro-label on its own (input: string). page_head writes its own kicker as the line above the title. */}} {{define "kicker"}}{{.}}{{end}} diff --git a/internal/web/view/templates/in_log.html b/internal/web/view/templates/in_log.html index c3ffe1e..013dd35 100644 --- a/internal/web/view/templates/in_log.html +++ b/internal/web/view/templates/in_log.html @@ -1,17 +1,18 @@ {{/* Inbound log (mockup in-log.html): what port 25 decided about each message offered for an inbound domain, one row per recipient, newest first, - filtered by domain, decision and day. It does not poll. The last cell is - the way to correct a decision in Filter lists, and only the global role, - who alone edits the lists, is given it. */}} + filtered by domain, decision and day. It does not poll. The decision is a + tag that opens a sheet with who decided and why. The last cell is the way + to correct a decision in Filter lists, and only the global role, who alone + edits the lists, is given it. */}} {{define "content"}} {{template "page_head" .Head}} {{template "box_open" .Messages}} {{if .Rows}}
- + {{range .Rows}} - {{if .Subject}}{{else}}{{end}}{{end}} + {{if .Subject}}{{else}}{{end}}{{end}}
Time, UTCFromToSubjectDecisionWhy
Time, UTCFromToSubjectDecision
{{.Time}}{{wbr_at .From}}{{if .Client}}
{{.Client}}{{end}}		{{wbr_at .To}}{{.Subject}}—{{template "tag" .Decision}}{{.Why}}{{if .Action}}{{.Action.Label}}{{end}}
{{.Time}}{{wbr_at .From}}{{if .Client}}
{{.Client}}{{end}}		{{wbr_at .To}}{{.Subject}}—{{template "sheet_tag" .Why}}{{if .Action}}{{.Action.Label}}{{end}}
{{template "box_foot" .Foot}} diff --git a/internal/web/view/templates/kit.html b/internal/web/view/templates/kit.html index 6371450..f0526c8 100644 --- a/internal/web/view/templates/kit.html +++ b/internal/web/view/templates/kit.html @@ -31,6 +31,7 @@ StatusTagBulma classes {{range .StatusRows}} {{.Words}}{{template "tag" .Tag}}{{status_tag .Tag.Status}}{{end}} + a tag that opens a sheet{{template "sheet_tag" .Sheet}}button.tag + sp-sheet (popover)
{{template "box_close"}} diff --git a/test/e2e/port25_features_test.go b/test/e2e/port25_features_test.go index 017ae88..9d81bff 100644 --- a/test/e2e/port25_features_test.go +++ b/test/e2e/port25_features_test.go @@ -297,7 +297,9 @@ func testInboundRelay(t *testing.T, sc *scenario) { body = b rows = rows[:0] for _, row := range strings.Split(body, "")]) + // The row's own end is the last one: the sheet in its + // decision cell holds a table of its own. + rows = append(rows, row[:strings.LastIndex(row, "")]) } if strings.Contains(body, want) { return rows @@ -326,7 +328,7 @@ func testInboundRelay(t *testing.T, sc *scenario) { rows := logRows("e2e " + token) rowWith(rows, "someone@stranger.e2e.test", "user@"+inboundDomain, "e2e "+token, - `accept`, `selfpost`, + `class="tag is-success is-light" popovertarget="why-`, `Decided byselfpost`, // the way to correct it, which only the global role is given `href="/inbound/filter-lists?list=deny&kind=address&value=someone%40stranger.e2e.test">Deny sender`) // A report mailed to the server's own name earlier in the run came through @@ -346,10 +348,14 @@ func testInboundRelay(t *testing.T, sc *scenario) { // the mail log into the journal; the refusal to relay for a stranger's // domain is not the relay's mail and is not. rows = logRows("nobody@" + inboundDomain) - rowWith(rows, + refused := rowWith(rows, "someone@stranger.e2e.test", "nobody@"+inboundDomain, `—`, - `reject`, `postfix`, - `>Deny sender`) + `class="tag is-danger is-light" popovertarget="why-`, `Decided bypostfix`, + `Reply550 5.1.1 `) + // A message Postfix itself refused offers no way into the lists. + if strings.Contains(refused, "sender") { + t.Fatalf("the refused row offers a way into the filter lists:\n%s", refused) + } if len(rows) != 2 { t.Fatalf("the Inbound log holds %d rows, want the relayed message and one refusal:\n%s", len(rows), strings.Join(rows, "\n")) }