diff --git a/internal/db/allowblock_junk_migration_test.go b/internal/db/allowblock_junk_migration_test.go new file mode 100644 index 0000000..76da7d1 --- /dev/null +++ b/internal/db/allowblock_junk_migration_test.go @@ -0,0 +1,79 @@ +package db + +import ( + "database/sql" + "path/filepath" + "testing" +) + +// TestAddAllowBlockJunkEntryWorksAfterMigration reproduces the same class of bug as +// the filter-rules CHECK migrations: a DB created before 'junk' was added to +// esrv_mailbox_allowblock's list_type CHECK constraint (the self-service webmail +// Blocklist feature) kept the old, narrower constraint forever, since SQLite can't +// ALTER a CHECK on an existing table. +func TestAddAllowBlockJunkEntryWorksAfterMigration(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.db") + + raw, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(`CREATE TABLE esrv_mailboxes (id INTEGER PRIMARY KEY AUTOINCREMENT)`); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(` + CREATE TABLE esrv_mailbox_allowblock ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), + list_type TEXT NOT NULL CHECK(list_type IN ('allow','block')), + pattern TEXT NOT NULL, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(mailbox_id, list_type, pattern) + ) + `); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(`INSERT INTO esrv_mailboxes (id) VALUES (1)`); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(` + INSERT INTO esrv_mailbox_allowblock (mailbox_id, list_type, pattern) VALUES (1, 'allow', 'trusted@example.com') + `); err != nil { + t.Fatal(err) + } + if err := raw.Close(); err != nil { + t.Fatal(err) + } + + database, err := Open(path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { database.Close() }) + + if _, err := database.AddAllowBlockEntry(1, "junk", "spammer@example.com"); err != nil { + t.Fatalf("AddAllowBlockEntry with 'junk' after migrating a legacy DB: %v", err) + } + + entries, err := database.ListAllowBlock(1) + if err != nil { + t.Fatal(err) + } + if len(entries) != 2 { + t.Fatalf("expected the pre-existing allow entry to survive the table rebuild alongside the new junk one, got %d entries", len(entries)) + } + found := false + for _, e := range entries { + if e.ListType == "allow" && e.Pattern == "trusted@example.com" { + found = true + } + } + if !found { + t.Fatalf("pre-existing allow entry's data was not preserved across the migration: %+v", entries) + } + + junked, err := database.IsJunked(1, "spammer@example.com") + if err != nil || !junked { + t.Fatalf("expected spammer@example.com to be junked, got junked=%v err=%v", junked, err) + } +} diff --git a/internal/db/crud_mailbox_contacts.go b/internal/db/crud_mailbox_contacts.go new file mode 100644 index 0000000..42d3896 --- /dev/null +++ b/internal/db/crud_mailbox_contacts.go @@ -0,0 +1,70 @@ +package db + +import ( + "database/sql" + "errors" +) + +const contactColumns = `id, mailbox_id, email, name, phone, created_at` + +func scanContact(scan func(dest ...any) error) (MailboxContact, error) { + var c MailboxContact + var createdAt string + err := scan(&c.ID, &c.MailboxID, &c.Email, &c.Name, &c.Phone, &createdAt) + if err != nil { + return c, err + } + c.CreatedAt, _ = parseTime(createdAt) + return c, nil +} + +// ListContacts returns a mailbox's saved contacts, alphabetical by name. +func (d *DB) ListContacts(mailboxID int64) ([]MailboxContact, error) { + rows, err := d.Query(`SELECT `+contactColumns+` FROM esrv_mailbox_contacts WHERE mailbox_id = ? ORDER BY name COLLATE NOCASE`, mailboxID) + if err != nil { + return nil, err + } + defer rows.Close() + var out []MailboxContact + for rows.Next() { + c, err := scanContact(rows.Scan) + if err != nil { + return nil, err + } + out = append(out, c) + } + return out, rows.Err() +} + +// GetContactByID scopes the lookup to mailboxID so one mailbox can never read or (via +// UpdateContact/DeleteContact, which reuse this same WHERE clause) modify another's +// contact by guessing an id. +func (d *DB) GetContactByID(mailboxID, id int64) (*MailboxContact, error) { + row := d.QueryRow(`SELECT `+contactColumns+` FROM esrv_mailbox_contacts WHERE id = ? AND mailbox_id = ?`, id, mailboxID) + c, err := scanContact(row.Scan) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + return nil, err + } + return &c, nil +} + +func (d *DB) CreateContact(mailboxID int64, email, name, phone string) (int64, error) { + res, err := d.Exec(`INSERT INTO esrv_mailbox_contacts (mailbox_id, email, name, phone) VALUES (?, ?, ?, ?)`, mailboxID, email, name, phone) + if err != nil { + return 0, err + } + return res.LastInsertId() +} + +func (d *DB) UpdateContact(mailboxID, id int64, email, name, phone string) error { + _, err := d.Exec(`UPDATE esrv_mailbox_contacts SET email = ?, name = ?, phone = ? WHERE id = ? AND mailbox_id = ?`, email, name, phone, id, mailboxID) + return err +} + +func (d *DB) DeleteContact(mailboxID, id int64) error { + _, err := d.Exec(`DELETE FROM esrv_mailbox_contacts WHERE id = ? AND mailbox_id = ?`, id, mailboxID) + return err +} diff --git a/internal/db/crud_mailbox_contacts_test.go b/internal/db/crud_mailbox_contacts_test.go new file mode 100644 index 0000000..1fcfb64 --- /dev/null +++ b/internal/db/crud_mailbox_contacts_test.go @@ -0,0 +1,66 @@ +package db + +import "testing" + +// TestContactCreateUpdateDelete exercises the full contact CRUD flow. +func TestContactCreateUpdateDelete(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + id, err := d.CreateContact(mailboxID, "jane@example.com", "Jane Doe", "555-1234") + if err != nil { + t.Fatal(err) + } + got, err := d.GetContactByID(mailboxID, id) + if err != nil || got == nil || got.Name != "Jane Doe" || got.Phone != "555-1234" { + t.Fatalf("expected created contact, got %+v (err=%v)", got, err) + } + + if err := d.UpdateContact(mailboxID, id, "jane@example.com", "Jane D.", ""); err != nil { + t.Fatal(err) + } + updated, err := d.GetContactByID(mailboxID, id) + if err != nil || updated == nil || updated.Name != "Jane D." || updated.Phone != "" { + t.Fatalf("expected updated contact with phone cleared, got %+v (err=%v)", updated, err) + } + + if err := d.DeleteContact(mailboxID, id); err != nil { + t.Fatal(err) + } + gone, err := d.GetContactByID(mailboxID, id) + if err != nil || gone != nil { + t.Fatalf("expected contact deleted, got %+v (err=%v)", gone, err) + } +} + +// TestContactEmailUniquePerMailbox confirms the UNIQUE(mailbox_id, email) constraint +// rejects a second contact with the same email in the same mailbox. +func TestContactEmailUniquePerMailbox(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + if _, err := d.CreateContact(mailboxID, "dup@example.com", "First", ""); err != nil { + t.Fatal(err) + } + if _, err := d.CreateContact(mailboxID, "dup@example.com", "Second", ""); err == nil { + t.Fatal("expected a UNIQUE constraint error for a duplicate email in the same mailbox") + } +} + +// TestSuggestRecipientsIncludesContacts confirms a saved contact shows up in compose +// autocomplete, formatted "Name " like the message-history-derived entries. +func TestSuggestRecipientsIncludesContacts(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + if _, err := d.CreateContact(mailboxID, "alice@example.com", "Alice Smith", ""); err != nil { + t.Fatal(err) + } + suggestions, err := d.SuggestRecipients(mailboxID, "Alice") + if err != nil { + t.Fatal(err) + } + if len(suggestions) != 1 || suggestions[0] != "Alice Smith " { + t.Fatalf("expected contact suggested as 'Alice Smith ', got %+v", suggestions) + } +} diff --git a/internal/db/crud_mailbox_lists.go b/internal/db/crud_mailbox_lists.go index 06916c4..546481c 100644 --- a/internal/db/crud_mailbox_lists.go +++ b/internal/db/crud_mailbox_lists.go @@ -45,6 +45,13 @@ func (d *DB) IsAllowed(mailboxID int64, senderAddr string) (bool, error) { return matchesAllowBlock(d, mailboxID, "allow", senderAddr) } +// IsJunked reports whether senderAddr matches the mailbox owner's own self-service +// Blocklist ('junk' entries — see esrv_mailbox_allowblock's schema comment for how +// this differs from admin's IsBlocked). +func (d *DB) IsJunked(mailboxID int64, senderAddr string) (bool, error) { + return matchesAllowBlock(d, mailboxID, "junk", senderAddr) +} + // matchesAllowBlock checks senderAddr against every pattern of listType for mailboxID // — an exact address match, or a "@domain.com" wildcard matching senderAddr's domain. func matchesAllowBlock(d *DB, mailboxID int64, listType, senderAddr string) (bool, error) { diff --git a/internal/db/crud_mailbox_messages.go b/internal/db/crud_mailbox_messages.go index b6e4e74..2fd6fb5 100644 --- a/internal/db/crud_mailbox_messages.go +++ b/internal/db/crud_mailbox_messages.go @@ -349,9 +349,10 @@ func (d *DB) CountMessagesByFolder(mailboxID int64) (map[string]int, error) { // SuggestRecipients returns up to 10 distinct addresses (as originally cached — a // display name like "Name " is kept as-is, not parsed apart, since // that's exactly what a To/Cc/Bcc field already accepts) this mailbox has previously -// exchanged mail with — its own Sent "To" list plus INBOX "From" senders — whose -// value contains prefix. Backs the compose recipient autocomplete; deliberately -// reuses message history already stored rather than a dedicated contacts table. +// exchanged mail with — its own Sent "To" list plus INBOX "From" senders, plus its +// saved address book (esrv_mailbox_contacts, formatted the same "Name " way so +// a contact's name is searchable and shown, not just its address) — whose value +// contains prefix. Backs the compose recipient autocomplete. func (d *DB) SuggestRecipients(mailboxID int64, prefix string) ([]string, error) { like := "%" + escapeLike(prefix) + "%" rows, err := d.Query(` @@ -359,9 +360,11 @@ func (d *DB) SuggestRecipients(mailboxID int64, prefix string) ([]string, error) SELECT cached_to AS addr FROM esrv_mailbox_messages WHERE mailbox_id = ? AND folder = 'Sent' AND cached_to != '' UNION SELECT cached_from AS addr FROM esrv_mailbox_messages WHERE mailbox_id = ? AND folder = 'INBOX' AND cached_from != '' + UNION + SELECT name || ' <' || email || '>' AS addr FROM esrv_mailbox_contacts WHERE mailbox_id = ? ) WHERE addr LIKE ? ESCAPE '\' - ORDER BY addr LIMIT 10`, mailboxID, mailboxID, like) + ORDER BY addr LIMIT 10`, mailboxID, mailboxID, mailboxID, like) if err != nil { return nil, err } diff --git a/internal/db/crud_mailbox_rules.go b/internal/db/crud_mailbox_rules.go index 37dbab4..d54c552 100644 --- a/internal/db/crud_mailbox_rules.go +++ b/internal/db/crud_mailbox_rules.go @@ -2,38 +2,59 @@ package db import "encoding/json" +const filterRuleColumns = `id, mailbox_id, name, priority, condition_field, condition_op, condition_value, action, action_value, action_options_json, is_active, conditions_json, match_type, created_at` + +func scanFilterRule(scan func(dest ...any) error) (MailboxFilterRule, error) { + var r MailboxFilterRule + var createdAt string + err := scan(&r.ID, &r.MailboxID, &r.Name, &r.Priority, &r.ConditionField, &r.ConditionOp, &r.ConditionValue, &r.Action, &r.ActionValue, &r.ActionOptionsJSON, &r.IsActive, &r.ConditionsJSON, &r.MatchType, &createdAt) + if err != nil { + return r, err + } + r.CreatedAt, _ = parseTime(createdAt) + return r, nil +} + func (d *DB) ListRulesForMailbox(mailboxID int64) ([]MailboxFilterRule, error) { - rows, err := d.Query(`SELECT id, mailbox_id, priority, condition_field, condition_op, condition_value, action, action_value, is_active, conditions_json, match_type, created_at - FROM esrv_mailbox_filter_rules WHERE mailbox_id = ? ORDER BY priority ASC, id ASC`, mailboxID) + rows, err := d.Query(`SELECT `+filterRuleColumns+` FROM esrv_mailbox_filter_rules WHERE mailbox_id = ? ORDER BY priority ASC, id ASC`, mailboxID) if err != nil { return nil, err } defer rows.Close() var out []MailboxFilterRule for rows.Next() { - var r MailboxFilterRule - var createdAt string - if err := rows.Scan(&r.ID, &r.MailboxID, &r.Priority, &r.ConditionField, &r.ConditionOp, &r.ConditionValue, &r.Action, &r.ActionValue, &r.IsActive, &r.ConditionsJSON, &r.MatchType, &createdAt); err != nil { + r, err := scanFilterRule(rows.Scan) + if err != nil { return nil, err } - r.CreatedAt, _ = parseTime(createdAt) out = append(out, r) } return out, rows.Err() } +// GetRuleByID scopes the lookup to mailboxID, mirroring GetSignatureByID — backs the +// rule builder's edit mode (?edit=). +func (d *DB) GetRuleByID(mailboxID, id int64) (*MailboxFilterRule, error) { + row := d.QueryRow(`SELECT `+filterRuleColumns+` FROM esrv_mailbox_filter_rules WHERE id = ? AND mailbox_id = ?`, id, mailboxID) + r, err := scanFilterRule(row.Scan) + if err != nil { + return nil, nil // sql.ErrNoRows and any scan error both just mean "not found/not yours" + } + return &r, nil +} + // CreateRule creates a single-condition rule — a thin wrapper over CreateRuleMulti // for the common one-condition case (and for existing callers/tests written before // multi-condition rules existed). func (d *DB) CreateRule(mailboxID int64, priority int, field, op, value, action, actionValue string) (int64, error) { - return d.CreateRuleMulti(mailboxID, priority, []RuleCondition{{Field: field, Op: op, Value: value}}, "all", action, actionValue) + return d.CreateRuleMulti(mailboxID, priority, []RuleCondition{{Field: field, Op: op, Value: value}}, "all", "", action, actionValue, "") } // CreateRuleMulti creates a rule with one or more conditions combined per matchType // ("all"=AND, "any"=OR, defaulting to "all" for anything else). The first condition // also mirrors into the legacy condition_field/op/value columns so old code paths // reading them directly still see something sane. -func (d *DB) CreateRuleMulti(mailboxID int64, priority int, conditions []RuleCondition, matchType, action, actionValue string) (int64, error) { +func (d *DB) CreateRuleMulti(mailboxID int64, priority int, conditions []RuleCondition, matchType, name, action, actionValue, actionOptionsJSON string) (int64, error) { if matchType != "any" { matchType = "all" } @@ -42,14 +63,38 @@ func (d *DB) CreateRuleMulti(mailboxID int64, priority int, conditions []RuleCon return 0, err } first := conditions[0] - res, err := d.Exec(`INSERT INTO esrv_mailbox_filter_rules (mailbox_id, priority, condition_field, condition_op, condition_value, action, action_value, conditions_json, match_type) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, mailboxID, priority, first.Field, first.Op, first.Value, action, actionValue, string(conditionsJSON), matchType) + res, err := d.Exec(`INSERT INTO esrv_mailbox_filter_rules (mailbox_id, name, priority, condition_field, condition_op, condition_value, action, action_value, action_options_json, conditions_json, match_type) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, mailboxID, name, priority, first.Field, first.Op, first.Value, action, actionValue, actionOptionsJSON, string(conditionsJSON), matchType) if err != nil { return 0, err } return res.LastInsertId() } +// UpdateRuleMulti overwrites an existing rule's fields in place — used by the rule +// builder's edit mode, scoped to mailboxID so one mailbox can never modify another's +// rule by guessing an id (mirrors UpdateSignature). +func (d *DB) UpdateRuleMulti(mailboxID, id int64, priority int, conditions []RuleCondition, matchType, name, action, actionValue, actionOptionsJSON string) error { + if matchType != "any" { + matchType = "all" + } + conditionsJSON, err := json.Marshal(conditions) + if err != nil { + return err + } + first := conditions[0] + _, err = d.Exec(`UPDATE esrv_mailbox_filter_rules SET name = ?, priority = ?, condition_field = ?, condition_op = ?, condition_value = ?, action = ?, action_value = ?, action_options_json = ?, conditions_json = ?, match_type = ? + WHERE id = ? AND mailbox_id = ?`, name, priority, first.Field, first.Op, first.Value, action, actionValue, actionOptionsJSON, string(conditionsJSON), matchType, id, mailboxID) + return err +} + +// SetRuleActive flips a rule's enabled/disabled state — a quick toggle from the rules +// list, no need to open the full edit builder just to pause a rule. +func (d *DB) SetRuleActive(id, mailboxID int64, active bool) error { + _, err := d.Exec(`UPDATE esrv_mailbox_filter_rules SET is_active = ? WHERE id = ? AND mailbox_id = ?`, active, id, mailboxID) + return err +} + // RemoveRule deletes a rule, scoped to mailboxID (mirrors RemoveAppPassword/RemoveAlias). func (d *DB) RemoveRule(id, mailboxID int64) error { _, err := d.Exec(`DELETE FROM esrv_mailbox_filter_rules WHERE id = ? AND mailbox_id = ?`, id, mailboxID) diff --git a/internal/db/crud_mailbox_signatures.go b/internal/db/crud_mailbox_signatures.go index 7cda84d..bbd076f 100644 --- a/internal/db/crud_mailbox_signatures.go +++ b/internal/db/crud_mailbox_signatures.go @@ -3,10 +3,21 @@ package db import ( "database/sql" "errors" + "strings" ) const signatureColumns = `id, mailbox_id, name, content_html, is_default_new, is_default_reply, created_at` +// signatureColumnsPrefixed is signatureColumns qualified with a table alias, for +// queries that JOIN esrv_mailbox_signatures against another table. +func signatureColumnsPrefixed(alias string) string { + cols := strings.Split(signatureColumns, ", ") + for i, c := range cols { + cols[i] = alias + "." + c + } + return strings.Join(cols, ", ") +} + func scanSignature(scan func(dest ...any) error) (MailboxSignature, error) { var s MailboxSignature var createdAt string @@ -51,9 +62,26 @@ func (d *DB) GetSignatureByID(mailboxID, id int64) (*MailboxSignature, error) { return &s, nil } -// GetDefaultSignature returns the mailbox's default-for-new (forReply=false) or +// GetDefaultSignature returns the default-for-new (forReply=false) or // default-for-reply/forward (forReply=true) signature, or nil if none is set. -func (d *DB) GetDefaultSignature(mailboxID int64, forReply bool) (*MailboxSignature, error) { +// forEmail scopes this to a specific send-as alias — see +// esrv_mailbox_signature_alias_defaults' schema comment: an alias with its own +// override uses that; forEmail == "" (or the mailbox's own primary address, which +// never has its own override row) falls straight through to the mailbox-wide +// is_default_new/is_default_reply columns, same as an alias with no override does. +func (d *DB) GetDefaultSignature(mailboxID int64, forReply bool, forEmail string) (*MailboxSignature, error) { + if forEmail != "" { + row := d.QueryRow(`SELECT `+signatureColumnsPrefixed("s")+` + FROM esrv_mailbox_signature_alias_defaults ad JOIN esrv_mailbox_signatures s ON s.id = ad.signature_id + WHERE ad.mailbox_id = ? AND ad.for_email = ? AND ad.for_reply = ?`, mailboxID, forEmail, forReply) + s, err := scanSignature(row.Scan) + if err == nil { + return &s, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + } col := "is_default_new" if forReply { col = "is_default_reply" @@ -83,6 +111,10 @@ func (d *DB) UpdateSignature(mailboxID, id int64, name, contentHTML string) erro } func (d *DB) DeleteSignature(mailboxID, id int64) error { + // Best-effort — a leftover alias-default row pointing at a deleted signature + // would just silently fail its JOIN in GetDefaultSignature (falls through to + // the mailbox-wide default), but there's no reason to leave it dangling. + d.Exec(`DELETE FROM esrv_mailbox_signature_alias_defaults WHERE signature_id = ?`, id) _, err := d.Exec(`DELETE FROM esrv_mailbox_signatures WHERE id = ? AND mailbox_id = ?`, id, mailboxID) return err } @@ -105,3 +137,40 @@ func (d *DB) SetDefaultSignature(mailboxID, id int64, forReply bool) error { _, err := d.Exec(`UPDATE esrv_mailbox_signatures SET `+col+` = 1 WHERE id = ? AND mailbox_id = ?`, id, mailboxID) return err } + +// ListSignatureAliasDefaults returns every per-alias default override for a mailbox +// (across all its signatures) — used to render "default for " badges on the +// signatures list without a query per signature. +func (d *DB) ListSignatureAliasDefaults(mailboxID int64) ([]SignatureAliasDefault, error) { + rows, err := d.Query(`SELECT id, mailbox_id, for_email, for_reply, signature_id FROM esrv_mailbox_signature_alias_defaults WHERE mailbox_id = ?`, mailboxID) + if err != nil { + return nil, err + } + defer rows.Close() + var out []SignatureAliasDefault + for rows.Next() { + var s SignatureAliasDefault + if err := rows.Scan(&s.ID, &s.MailboxID, &s.ForEmail, &s.ForReply, &s.SignatureID); err != nil { + return nil, err + } + out = append(out, s) + } + return out, rows.Err() +} + +// SetSignatureAliasDefault makes signatureID the default-for-new/default-for-reply +// signature when composing as forEmail (a send-as alias) — upserts so re-setting the +// same scope just repoints it rather than erroring on the UNIQUE constraint. +func (d *DB) SetSignatureAliasDefault(mailboxID, signatureID int64, forEmail string, forReply bool) error { + _, err := d.Exec(`INSERT INTO esrv_mailbox_signature_alias_defaults (mailbox_id, for_email, for_reply, signature_id) VALUES (?, ?, ?, ?) + ON CONFLICT(mailbox_id, for_email, for_reply) DO UPDATE SET signature_id = excluded.signature_id`, + mailboxID, forEmail, forReply, signatureID) + return err +} + +// ClearSignatureAliasDefault removes forEmail's default-for-new/default-for-reply +// override, falling back to the mailbox-wide default again. +func (d *DB) ClearSignatureAliasDefault(mailboxID int64, forEmail string, forReply bool) error { + _, err := d.Exec(`DELETE FROM esrv_mailbox_signature_alias_defaults WHERE mailbox_id = ? AND for_email = ? AND for_reply = ?`, mailboxID, forEmail, forReply) + return err +} diff --git a/internal/db/crud_mailbox_signatures_test.go b/internal/db/crud_mailbox_signatures_test.go new file mode 100644 index 0000000..d4b7bc9 --- /dev/null +++ b/internal/db/crud_mailbox_signatures_test.go @@ -0,0 +1,123 @@ +package db + +import "testing" + +// TestGetDefaultSignatureFallsBackToMailboxWideDefault confirms an alias with no +// override of its own still gets the mailbox's regular default-for-new/default-for- +// reply signature, rather than "no signature" — the fallback chain +// GetDefaultSignature relies on for every alias that hasn't been given its own +// override. +func TestGetDefaultSignatureFallsBackToMailboxWideDefault(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + id, err := d.CreateSignature(mailboxID, "Work", "

Work sig

") + if err != nil { + t.Fatal(err) + } + if err := d.SetDefaultSignature(mailboxID, id, false); err != nil { + t.Fatal(err) + } + + sig, err := d.GetDefaultSignature(mailboxID, false, "alias@example.com") + if err != nil || sig == nil || sig.ID != id { + t.Fatalf("expected fallback to the mailbox-wide default, got %+v (err=%v)", sig, err) + } +} + +// TestGetDefaultSignatureAliasOverrideWins confirms an alias with its own override +// gets that signature instead of the mailbox-wide default, even when both exist. +func TestGetDefaultSignatureAliasOverrideWins(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + generalID, err := d.CreateSignature(mailboxID, "General", "

General

") + if err != nil { + t.Fatal(err) + } + if err := d.SetDefaultSignature(mailboxID, generalID, false); err != nil { + t.Fatal(err) + } + supportID, err := d.CreateSignature(mailboxID, "Support", "

Support

") + if err != nil { + t.Fatal(err) + } + if err := d.SetSignatureAliasDefault(mailboxID, supportID, "support@example.com", false); err != nil { + t.Fatal(err) + } + + sig, err := d.GetDefaultSignature(mailboxID, false, "support@example.com") + if err != nil || sig == nil || sig.ID != supportID { + t.Fatalf("expected the alias override (Support), got %+v (err=%v)", sig, err) + } + + // A different, unrelated address still gets the mailbox-wide default. + sig, err = d.GetDefaultSignature(mailboxID, false, "someone-else@example.com") + if err != nil || sig == nil || sig.ID != generalID { + t.Fatalf("expected the mailbox-wide default (General) for an address with no override, got %+v (err=%v)", sig, err) + } +} + +// TestSetSignatureAliasDefaultUpsertsRepointing confirms re-setting the same +// (mailbox, alias, new-or-reply) scope to a different signature repoints it instead +// of erroring on the UNIQUE constraint. +func TestSetSignatureAliasDefaultUpsertsRepointing(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + firstID, _ := d.CreateSignature(mailboxID, "First", "

First

") + secondID, _ := d.CreateSignature(mailboxID, "Second", "

Second

") + + if err := d.SetSignatureAliasDefault(mailboxID, firstID, "alias@example.com", false); err != nil { + t.Fatal(err) + } + if err := d.SetSignatureAliasDefault(mailboxID, secondID, "alias@example.com", false); err != nil { + t.Fatalf("expected re-setting the same scope to upsert cleanly, got: %v", err) + } + + sig, err := d.GetDefaultSignature(mailboxID, false, "alias@example.com") + if err != nil || sig == nil || sig.ID != secondID { + t.Fatalf("expected the scope repointed to Second, got %+v (err=%v)", sig, err) + } +} + +// TestClearSignatureAliasDefaultFallsBack confirms clearing an alias override falls +// back to the mailbox-wide default again, rather than leaving "no signature". +func TestClearSignatureAliasDefaultFallsBack(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + generalID, _ := d.CreateSignature(mailboxID, "General", "

General

") + d.SetDefaultSignature(mailboxID, generalID, false) + supportID, _ := d.CreateSignature(mailboxID, "Support", "

Support

") + d.SetSignatureAliasDefault(mailboxID, supportID, "support@example.com", false) + + if err := d.ClearSignatureAliasDefault(mailboxID, "support@example.com", false); err != nil { + t.Fatal(err) + } + sig, err := d.GetDefaultSignature(mailboxID, false, "support@example.com") + if err != nil || sig == nil || sig.ID != generalID { + t.Fatalf("expected fallback to General after clearing the override, got %+v (err=%v)", sig, err) + } +} + +// TestDeleteSignatureCleansUpAliasDefaults confirms deleting a signature that's an +// alias's default doesn't leave a dangling row pointing at a nonexistent signature. +func TestDeleteSignatureCleansUpAliasDefaults(t *testing.T) { + d := openTestDB(t) + const mailboxID = int64(1) + + supportID, _ := d.CreateSignature(mailboxID, "Support", "

Support

") + d.SetSignatureAliasDefault(mailboxID, supportID, "support@example.com", false) + + if err := d.DeleteSignature(mailboxID, supportID); err != nil { + t.Fatal(err) + } + defaults, err := d.ListSignatureAliasDefaults(mailboxID) + if err != nil { + t.Fatal(err) + } + if len(defaults) != 0 { + t.Fatalf("expected the alias default row cleaned up alongside the deleted signature, got %+v", defaults) + } +} diff --git a/internal/db/filter_rules_advanced_migration_test.go b/internal/db/filter_rules_advanced_migration_test.go new file mode 100644 index 0000000..cdfb877 --- /dev/null +++ b/internal/db/filter_rules_advanced_migration_test.go @@ -0,0 +1,83 @@ +package db + +import ( + "database/sql" + "path/filepath" + "testing" +) + +// TestCreateRuleForwardWorksAfterAdvancedCheckMigration reproduces the same class of +// bug as TestCreateRuleMarkAsSpamWorksAfterLegacyCheckMigration one migration later: a +// DB created after 'mark_as_spam' was added but before the advanced rule builder +// (forward action, body/has_attachment/recipient_type conditions, name/ +// action_options_json columns) kept the old, narrower CHECK constraints forever, +// since SQLite can't ALTER a CHECK on an existing table. +func TestCreateRuleForwardWorksAfterAdvancedCheckMigration(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.db") + + raw, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(`CREATE TABLE esrv_mailboxes (id INTEGER PRIMARY KEY AUTOINCREMENT)`); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(` + CREATE TABLE esrv_mailbox_filter_rules ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), + priority INTEGER NOT NULL DEFAULT 0, + condition_field TEXT NOT NULL CHECK(condition_field IN ('from','to','subject')), + condition_op TEXT NOT NULL CHECK(condition_op IN ('contains','equals','starts_with')), + condition_value TEXT NOT NULL, + action TEXT NOT NULL CHECK(action IN ('move_to_folder','delete','mark_read','mark_as_spam')), + action_value TEXT NOT NULL DEFAULT '', + is_active INTEGER NOT NULL DEFAULT 1, + conditions_json TEXT NOT NULL DEFAULT '', + match_type TEXT NOT NULL DEFAULT 'all', + created_at DATETIME DEFAULT CURRENT_TIMESTAMP + ) + `); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(`INSERT INTO esrv_mailboxes (id) VALUES (1)`); err != nil { + t.Fatal(err) + } + if _, err := raw.Exec(` + INSERT INTO esrv_mailbox_filter_rules (mailbox_id, priority, condition_field, condition_op, condition_value, action, action_value) + VALUES (1, 0, 'subject', 'contains', 'existing-rule', 'move_to_folder', 'Archive') + `); err != nil { + t.Fatal(err) + } + if err := raw.Close(); err != nil { + t.Fatal(err) + } + + database, err := Open(path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { database.Close() }) + + conditions := []RuleCondition{{Field: "has_attachment", Op: "equals", Value: "yes"}} + if _, err := database.CreateRuleMulti(1, 0, conditions, "all", "Forward invoices", "forward", "billing@example.com", `{"keep_copy":true}`); err != nil { + t.Fatalf("CreateRuleMulti with forward action + has_attachment condition after migrating a legacy DB: %v", err) + } + + rules, err := database.ListRulesForMailbox(1) + if err != nil { + t.Fatal(err) + } + if len(rules) != 2 { + t.Fatalf("expected the pre-existing rule to survive the table rebuild alongside the new one, got %d rules", len(rules)) + } + found := false + for _, r := range rules { + if r.ConditionValue == "existing-rule" && r.ActionValue == "Archive" { + found = true + } + } + if !found { + t.Fatalf("pre-existing rule's data was not preserved across the migration: %+v", rules) + } +} diff --git a/internal/db/mailbox_models.go b/internal/db/mailbox_models.go index 3f8b6bd..39d66c6 100644 --- a/internal/db/mailbox_models.go +++ b/internal/db/mailbox_models.go @@ -67,40 +67,76 @@ type MailboxAlias struct { CreatedAt time.Time } -// MailboxAllowBlockEntry is one allow- or block-list pattern for a mailbox. +// MailboxAllowBlockEntry is one allow-, block-, or junk-list pattern for a mailbox — +// see esrv_mailbox_allowblock's schema comment for what each list_type means. type MailboxAllowBlockEntry struct { ID int64 MailboxID int64 - ListType string // "allow" | "block" + ListType string // "allow" | "block" | "junk" Pattern string CreatedAt time.Time } +// MailboxContact is one entry in a mailbox owner's own address book. +type MailboxContact struct { + ID int64 + MailboxID int64 + Email string + Name string + Phone string + CreatedAt time.Time +} + // MailboxFilterRule is one priority-ordered, first-match-wins delivery rule. // ConditionField/Op/Value are the legacy single-condition columns; ConditionsJSON // (when non-empty) is the current multi-condition representation — see Conditions(). type MailboxFilterRule struct { - ID int64 - MailboxID int64 - Priority int - ConditionField string // "from" | "to" | "subject" - ConditionOp string // "contains" | "equals" | "starts_with" - ConditionValue string - Action string // "move_to_folder" | "delete" | "mark_read" | "mark_as_spam" - ActionValue string - IsActive bool - ConditionsJSON string - MatchType string // "all" (AND, default) | "any" (OR) - CreatedAt time.Time + ID int64 + MailboxID int64 + Name string + Priority int + ConditionField string // "from" | "to" | "subject" | "body" | "has_attachment" | "recipient_type" + ConditionOp string // "contains" | "equals" | "starts_with" + ConditionValue string + Action string // "move_to_folder" | "delete" | "mark_read" | "mark_as_spam" | "forward" + ActionValue string + ActionOptionsJSON string + IsActive bool + ConditionsJSON string + MatchType string // "all" (AND, default) | "any" (OR) + CreatedAt time.Time } -// RuleCondition is one condition within a filter rule's "if" clause. +// RuleCondition is one condition within a filter rule's "if" clause. Value may hold +// several "\n"-joined values (e.g. several From addresses, or several subject +// keywords picked in the rule builder's chip input) — matches if any one does, see +// mailstore.matchCondition. type RuleCondition struct { Field string `json:"field"` Op string `json:"op"` Value string `json:"value"` } +// RuleActionOptions holds action-specific parameters that only some actions need — +// currently just "forward"'s KeepCopy. Stored as MailboxFilterRule.ActionOptionsJSON. +type RuleActionOptions struct { + // KeepCopy: for the "forward" action, whether a copy is also stored in this + // mailbox (true) or the message is forwarded and dropped otherwise (false). + // Defaults true (see ActionOptions) — the safer default is to not silently lose + // mail just because forwarding was set up. + KeepCopy bool `json:"keep_copy"` +} + +// ActionOptions parses ActionOptionsJSON, defaulting KeepCopy true for a rule created +// before this existed (or with the field simply left unset). +func (r MailboxFilterRule) ActionOptions() RuleActionOptions { + opts := RuleActionOptions{KeepCopy: true} + if r.ActionOptionsJSON != "" { + json.Unmarshal([]byte(r.ActionOptionsJSON), &opts) + } + return opts +} + // Conditions returns this rule's conditions and how they combine ("all"=AND, // "any"=OR) — parses ConditionsJSON when present, falling back to the single legacy // condition_field/op/value columns for rules created before multi-condition support @@ -168,6 +204,16 @@ type MailboxSignature struct { CreatedAt time.Time } +// SignatureAliasDefault is one per-alias override of which signature is the default +// for a mailbox with send-as aliases — see GetDefaultSignature. +type SignatureAliasDefault struct { + ID int64 + MailboxID int64 + ForEmail string + ForReply bool + SignatureID int64 +} + // MailboxSMIMEIdentity is one of a mailbox's own S/MIME certificate + private key // pairs — a mailbox may hold several. Both halves are stored plain: S/MIME is // sign-only in this codebase, so the key never protects anything beyond what the diff --git a/internal/db/schema.go b/internal/db/schema.go index 393a5e3..0bc2520 100644 --- a/internal/db/schema.go +++ b/internal/db/schema.go @@ -270,36 +270,71 @@ CREATE TABLE IF NOT EXISTS esrv_mailbox_aliases ( created_at DATETIME DEFAULT CURRENT_TIMESTAMP ); --- Per-mailbox sender allow/block list. pattern is either an exact address +-- Per-mailbox sender allow/block/junk list. pattern is either an exact address -- ("spam@evil.com") or a whole-domain wildcard ("@evil.com"). A single table with a --- list_type column, not two near-identical tables. +-- list_type column, not three near-identical tables. +-- 'block' is admin-only (Prefix+"/mailboxes/{id}/lists") and hard-rejects at RCPT +-- time (db.IsBlocked, checked in smtpserver's Rcpt()) — an anti-abuse tool, not +-- something an end user self-manages. 'junk' is the mailbox owner's own self-service +-- Blocklist (webmail Settings, or the message view's "Mark as Junk" action) and is a +-- *soft* block instead: mail is still accepted and delivered, just straight to Junk +-- (db.IsJunked, checked in smtpserver's deliverLocally, bypassing spam scoring +-- entirely the same way 'allow' does) rather than bounced. Different tools for +-- different jobs — collapsing them into one would either strip admins of a real hard +-- reject or hand end users the ability to bounce mail on another user's behalf. CREATE TABLE IF NOT EXISTS esrv_mailbox_allowblock ( id INTEGER PRIMARY KEY AUTOINCREMENT, mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), - list_type TEXT NOT NULL CHECK(list_type IN ('allow','block')), + list_type TEXT NOT NULL CHECK(list_type IN ('allow','block','junk')), pattern TEXT NOT NULL, created_at DATETIME DEFAULT CURRENT_TIMESTAMP, UNIQUE(mailbox_id, list_type, pattern) ); +-- A mailbox owner's own address book — deliberately separate from +-- esrv_mailbox_smime_contacts/esrv_mailbox_pgp_contacts (those hold a certificate/key +-- per address for signing/encryption, not a person's name/phone) and from +-- SuggestRecipients' history-based autocomplete (mailbox_messages.go — reuses +-- cached_to/cached_from rather than a dedicated table, so it has no name/phone either +-- and can't be edited). name is required; phone is optional free text (no format +-- enforced — international numbers, extensions, etc. all vary too much to validate +-- usefully here). +CREATE TABLE IF NOT EXISTS esrv_mailbox_contacts ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), + email TEXT NOT NULL, + name TEXT NOT NULL, + phone TEXT NOT NULL DEFAULT '', + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(mailbox_id, email) +); + -- Simple first-match-wins filter rules, evaluated in priority order (lower first) at --- delivery time, before a message is encrypted and stored — so from/to/subject +-- delivery time, before a message is encrypted and stored — so from/to/subject/body -- matching works against the real message, not just the plaintext cache columns below. -- condition_field/op/value are the legacy single-condition columns, kept for rows -- created before multi-condition support existed. Every rule created since then -- stores its full condition list in conditions_json (a JSON array of --- {field,op,value}) instead, combined per match_type ("all"=AND, "any"=OR); a rule --- with an empty conditions_json falls back to the legacy columns as a single +-- {field,op,value}, value optionally "\n"-joined to mean "any of these" — see +-- mailstore.matchCondition) instead, combined per match_type ("all"=AND, "any"=OR); a +-- rule with an empty conditions_json falls back to the legacy columns as a single -- condition — see MailboxFilterRule.Conditions() in mailbox_models.go. +-- has_attachment/recipient_type conditions match against a synthetic "yes"/"no" or +-- "to"/"cc"/"bcc" header value computed at delivery time (see smtpserver's +-- deliverLocally), not a real message header. +-- action_options_json holds action-specific parameters that don't apply to every +-- action (currently just "forward"'s keep_copy) — see MailboxFilterRule.ActionOptions(). CREATE TABLE IF NOT EXISTS esrv_mailbox_filter_rules ( id INTEGER PRIMARY KEY AUTOINCREMENT, mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), + name TEXT NOT NULL DEFAULT '', priority INTEGER NOT NULL DEFAULT 0, - condition_field TEXT NOT NULL CHECK(condition_field IN ('from','to','subject')), + condition_field TEXT NOT NULL CHECK(condition_field IN ('from','to','subject','body','has_attachment','recipient_type')), condition_op TEXT NOT NULL CHECK(condition_op IN ('contains','equals','starts_with')), condition_value TEXT NOT NULL, - action TEXT NOT NULL CHECK(action IN ('move_to_folder','delete','mark_read','mark_as_spam')), + action TEXT NOT NULL CHECK(action IN ('move_to_folder','delete','mark_read','mark_as_spam','forward')), action_value TEXT NOT NULL DEFAULT '', + action_options_json TEXT NOT NULL DEFAULT '', is_active INTEGER NOT NULL DEFAULT 1, conditions_json TEXT NOT NULL DEFAULT '', match_type TEXT NOT NULL DEFAULT 'all', @@ -470,6 +505,23 @@ CREATE TABLE IF NOT EXISTS esrv_mailbox_signatures ( ); CREATE INDEX IF NOT EXISTS idx_mailbox_signatures_mailbox ON esrv_mailbox_signatures(mailbox_id); +-- Per-alias override of which signature is the default-for-new/default-for-reply, +-- for a mailbox with one or more send-as aliases (esrv_mailbox_aliases) wanting a +-- different signature depending which address they're composing as — e.g. a +-- "Support" signature for support@ and a personal one for their own address. +-- Purely additive on top of esrv_mailbox_signatures' own is_default_new/is_default_reply +-- columns, which remain the fallback default (for_email = the mailbox's own primary +-- address, or an alias with no override row here) — see GetDefaultSignature. +CREATE TABLE IF NOT EXISTS esrv_mailbox_signature_alias_defaults ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + mailbox_id INTEGER NOT NULL REFERENCES esrv_mailboxes(id), + for_email TEXT NOT NULL, + for_reply INTEGER NOT NULL DEFAULT 0, + signature_id INTEGER NOT NULL REFERENCES esrv_mailbox_signatures(id), + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(mailbox_id, for_email, for_reply) +); + -- Other people's PGP public keys a mailbox owner has collected, added by hand — -- mirrors esrv_mailbox_smime_contacts. Used to offer "Encrypt (PGP)" for a -- recipient in compose. @@ -525,6 +577,12 @@ func migrateAddedColumns(db *sql.DB) { `ALTER TABLE esrv_mailbox_folders ADD COLUMN restore_parent_root TEXT NOT NULL DEFAULT ''`, `ALTER TABLE esrv_mailbox_messages ADD COLUMN restore_folder TEXT NOT NULL DEFAULT ''`, `ALTER TABLE esrv_mailboxes ADD COLUMN remote_images_mode TEXT NOT NULL DEFAULT 'ask'`, + // The condition_field/action CHECK constraints (adding body/has_attachment/ + // recipient_type and forward) aren't retrofittable via ALTER TABLE either — see + // migrateFilterRulesAdvancedCheck below. name/action_options_json themselves are + // plain columns and migrate fine here. + `ALTER TABLE esrv_mailbox_filter_rules ADD COLUMN name TEXT NOT NULL DEFAULT ''`, + `ALTER TABLE esrv_mailbox_filter_rules ADD COLUMN action_options_json TEXT NOT NULL DEFAULT ''`, } // The three old columns above were NOT NULL with no default, so simply adding // key_pem left them behind still blocking every new insert (which only ever sets @@ -544,12 +602,37 @@ func migrateAddedColumns(db *sql.DB) { db.Exec(`UPDATE esrv_admin_users SET must_change_username = 1 WHERE username = ? AND must_change_password = 1`, DefaultAdminUsername) migrateSpamRenamedToJunk(db) migrateFilterRulesMarkAsSpamCheck(db) + migrateFilterRulesAdvancedCheck(db) + migrateAllowBlockJunkCheck(db) +} + +// migrateAllowBlockJunkCheck rebuilds esrv_mailbox_allowblock for any DB created +// before 'junk' was added to the list_type CHECK constraint (the self-service +// webmail Blocklist feature) — same rename/recreate/copy/drop approach as +// migrateFilterRulesMarkAsSpamCheck above. +func migrateAllowBlockJunkCheck(db *sql.DB) { + var tableSQL string + if err := db.QueryRow(`SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'esrv_mailbox_allowblock'`).Scan(&tableSQL); err != nil { + return + } + if strings.Contains(tableSQL, "'junk'") { + return + } + if _, err := db.Exec(`ALTER TABLE esrv_mailbox_allowblock RENAME TO esrv_mailbox_allowblock_old`); err != nil { + return + } + if _, err := db.Exec(schema); err != nil { + return + } + db.Exec(`INSERT INTO esrv_mailbox_allowblock (id, mailbox_id, list_type, pattern, created_at) + SELECT id, mailbox_id, list_type, pattern, created_at FROM esrv_mailbox_allowblock_old`) + db.Exec(`DROP TABLE esrv_mailbox_allowblock_old`) } // migrateFilterRulesMarkAsSpamCheck rebuilds esrv_mailbox_filter_rules for any DB -// created before 'mark_as_spam' was added to the action CHECK constraint (webmail's -// "Mark as Junk" auto-blacklist rule, see webmail_mail.go's ensureJunkRuleForSender) — -// SQLite can't ALTER a CHECK constraint on an existing table, so the only way to widen +// created before 'mark_as_spam' was added to the action CHECK constraint (still a +// valid rule-builder action today — see the Rules section of Settings) — SQLite +// can't ALTER a CHECK constraint on an existing table, so the only way to widen // it is to recreate the table under the current schema and copy the rows across. // Detects the stale constraint by inspecting sqlite_master rather than tracking a // schema-version number, so it stays a no-op forever once a DB is caught up. @@ -574,6 +657,35 @@ func migrateFilterRulesMarkAsSpamCheck(db *sql.DB) { db.Exec(`DROP TABLE esrv_mailbox_filter_rules_old`) } +// migrateFilterRulesAdvancedCheck rebuilds esrv_mailbox_filter_rules for any DB created +// before the advanced rule builder (body/has_attachment/recipient_type conditions, +// forward action) widened condition_field/action's CHECK constraints — same +// rename/recreate/copy/drop approach as migrateFilterRulesMarkAsSpamCheck above, and +// deliberately run after it so a DB that predates both migrations gets caught up by +// each in turn without either one needing to know about the other's column set. +// name/action_options_json are plain columns already added by migrateAddedColumns +// above by the time this runs, so the INSERT below can select them unconditionally. +func migrateFilterRulesAdvancedCheck(db *sql.DB) { + var tableSQL string + if err := db.QueryRow(`SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'esrv_mailbox_filter_rules'`).Scan(&tableSQL); err != nil { + return + } + if strings.Contains(tableSQL, "'forward'") { + return + } + if _, err := db.Exec(`ALTER TABLE esrv_mailbox_filter_rules RENAME TO esrv_mailbox_filter_rules_old`); err != nil { + return + } + if _, err := db.Exec(schema); err != nil { + return + } + db.Exec(`INSERT INTO esrv_mailbox_filter_rules + (id, mailbox_id, name, priority, condition_field, condition_op, condition_value, action, action_value, action_options_json, is_active, conditions_json, match_type, created_at) + SELECT id, mailbox_id, name, priority, condition_field, condition_op, condition_value, action, action_value, action_options_json, is_active, conditions_json, match_type, created_at + FROM esrv_mailbox_filter_rules_old`) + db.Exec(`DROP TABLE esrv_mailbox_filter_rules_old`) +} + // migrateSpamRenamedToJunk renames the standard "Spam" folder to "Junk" for mailboxes // that already had messages/records under the old name — "Junk" is what most desktop // IMAP clients look for by name (see db.StandardMailboxFolders' doc comment). Always diff --git a/internal/mailstore/rules.go b/internal/mailstore/rules.go index 245b5f3..9004daa 100644 --- a/internal/mailstore/rules.go +++ b/internal/mailstore/rules.go @@ -12,13 +12,21 @@ type FilterAction struct { Folder string // non-empty: store here instead of INBOX MarkRead bool Drop bool // don't store at all + // ForwardTo: non-empty means also relay a copy of this message out to this + // address — see smtpserver's deliverLocally, which actually sends it (this + // package has no outbound SMTP capability of its own). KeepCopy controls whether + // the recipient's own local copy is still stored too. + ForwardTo string + KeepCopy bool } // ApplyRules evaluates a mailbox's filter rules in priority order and returns the // first match's action (zero value if none match, meaning "store in INBOX, unread"). -// headers should have "from"/"to"/"subject" keys — rules run at delivery time, before -// the message is encrypted and stored, so real header values are available, not just -// the plaintext cache columns used for fast IMAP listing. +// headers should have "from"/"to"/"subject"/"body"/"has_attachment"/"recipient_type" +// keys — rules run at delivery time, before the message is encrypted and stored, so +// real header/body values are available, not just the plaintext cache columns used +// for fast IMAP listing. has_attachment is "yes"/"no" and recipient_type is +// "to"/"cc"/"bcc", both computed by the caller rather than being real headers. func (s *Store) ApplyRules(mailboxID int64, headers map[string]string) (FilterAction, error) { rules, err := s.DB.ListRulesForMailbox(mailboxID) if err != nil { @@ -43,6 +51,8 @@ func (s *Store) ApplyRules(mailboxID int64, headers map[string]string) (FilterAc return FilterAction{Drop: true}, nil case "mark_read": return FilterAction{MarkRead: true}, nil + case "forward": + return FilterAction{ForwardTo: r.ActionValue, KeepCopy: r.ActionOptions().KeepCopy}, nil } } return FilterAction{}, nil @@ -68,16 +78,31 @@ func ruleMatches(r db.MailboxFilterRule, headers map[string]string) bool { return true } +// matchCondition compares a header's actual value against a condition's target. +// target may hold several "\n"-joined values (the rule builder's chip input, e.g. +// multiple From addresses or multiple subject keywords) — matches if any one does, +// mirroring how a real mail client's "is any of" condition works. func matchCondition(op, value, target string) bool { value = strings.ToLower(value) - target = strings.ToLower(target) - switch op { - case "contains": - return strings.Contains(value, target) - case "equals": - return value == target - case "starts_with": - return strings.HasPrefix(value, target) + for _, t := range strings.Split(target, "\n") { + t = strings.ToLower(strings.TrimSpace(t)) + if t == "" { + continue + } + switch op { + case "contains": + if strings.Contains(value, t) { + return true + } + case "equals": + if value == t { + return true + } + case "starts_with": + if strings.HasPrefix(value, t) { + return true + } + } } return false } diff --git a/internal/mailstore/rules_test.go b/internal/mailstore/rules_test.go index 439c70a..db9b41e 100644 --- a/internal/mailstore/rules_test.go +++ b/internal/mailstore/rules_test.go @@ -14,7 +14,7 @@ func TestApplyRulesMultiConditionAnd(t *testing.T) { {Field: "to", Op: "contains", Value: "sales"}, {Field: "subject", Op: "contains", Value: "invoice"}, } - if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "move_to_folder", "Invoices"); err != nil { + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "move_to_folder", "Invoices", ""); err != nil { t.Fatal(err) } @@ -45,7 +45,7 @@ func TestApplyRulesMultiConditionOr(t *testing.T) { {Field: "from", Op: "contains", Value: "boss@work.example"}, {Field: "subject", Op: "contains", Value: "urgent"}, } - if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "any", "mark_read", ""); err != nil { + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "any", "", "mark_read", "", ""); err != nil { t.Fatal(err) } @@ -71,7 +71,7 @@ func TestApplyRulesMultiConditionOr(t *testing.T) { func TestApplyRulesMarkAsSpam(t *testing.T) { s, mailboxID := newTestMailbox(t, 1024*1024) conditions := []db.RuleCondition{{Field: "subject", Op: "contains", Value: "viagra"}} - if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "mark_as_spam", ""); err != nil { + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "mark_as_spam", "", ""); err != nil { t.Fatal(err) } @@ -84,6 +84,108 @@ func TestApplyRulesMarkAsSpam(t *testing.T) { } } +// TestApplyRulesMultiValueConditionMatchesAny confirms a single condition holding +// several "\n"-joined values (the rule builder's chip input, e.g. several From +// addresses) matches if any one of them does. +func TestApplyRulesMultiValueConditionMatchesAny(t *testing.T) { + s, mailboxID := newTestMailbox(t, 1024*1024) + conditions := []db.RuleCondition{{Field: "from", Op: "contains", Value: "aaa@gogogo.com\nflower@monster.com"}} + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "delete", "", ""); err != nil { + t.Fatal(err) + } + + action, err := s.ApplyRules(mailboxID, map[string]string{"from": "flower@monster.com"}) + if err != nil { + t.Fatal(err) + } + if !action.Drop { + t.Fatalf("expected match against the second of two chip values, got %+v", action) + } + + action, err = s.ApplyRules(mailboxID, map[string]string{"from": "someone-else@example.com"}) + if err != nil { + t.Fatal(err) + } + if action.Drop { + t.Fatalf("expected no match when neither chip value matches, got %+v", action) + } +} + +// TestApplyRulesBodyAndHasAttachmentConditions confirms the two synthetic condition +// fields computed by the caller (smtpserver's deliverLocally, not real headers) match +// like any other field once present in the headers map. +func TestApplyRulesBodyAndHasAttachmentConditions(t *testing.T) { + s, mailboxID := newTestMailbox(t, 1024*1024) + conditions := []db.RuleCondition{ + {Field: "body", Op: "contains", Value: "invoice"}, + {Field: "has_attachment", Op: "equals", Value: "yes"}, + } + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "mark_as_spam", "", ""); err != nil { + t.Fatal(err) + } + + action, err := s.ApplyRules(mailboxID, map[string]string{"body": "please pay this invoice", "has_attachment": "yes"}) + if err != nil { + t.Fatal(err) + } + if action.Folder != "Junk" { + t.Fatalf("expected match when both body and has_attachment match, got %+v", action) + } + + action, err = s.ApplyRules(mailboxID, map[string]string{"body": "please pay this invoice", "has_attachment": "no"}) + if err != nil { + t.Fatal(err) + } + if action.Folder == "Junk" { + t.Fatalf("expected no match when has_attachment is no, got %+v", action) + } +} + +// TestApplyRulesRecipientTypeCondition confirms recipient_type (the per-delivery +// to/cc/bcc-ness computed by the caller, mirroring Outlook's "I'm on the Cc line") +// only matches deliveries of that specific type. +func TestApplyRulesRecipientTypeCondition(t *testing.T) { + s, mailboxID := newTestMailbox(t, 1024*1024) + conditions := []db.RuleCondition{{Field: "recipient_type", Op: "equals", Value: "cc"}} + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "mark_read", "", ""); err != nil { + t.Fatal(err) + } + + action, err := s.ApplyRules(mailboxID, map[string]string{"recipient_type": "cc"}) + if err != nil { + t.Fatal(err) + } + if !action.MarkRead { + t.Fatalf("expected match on cc delivery, got %+v", action) + } + + action, err = s.ApplyRules(mailboxID, map[string]string{"recipient_type": "to"}) + if err != nil { + t.Fatal(err) + } + if action.MarkRead { + t.Fatalf("expected no match on to delivery, got %+v", action) + } +} + +// TestApplyRulesForwardActionReturnsKeepCopy confirms the forward action carries its +// destination address and keep_copy option through from ActionOptionsJSON. +func TestApplyRulesForwardActionReturnsKeepCopy(t *testing.T) { + s, mailboxID := newTestMailbox(t, 1024*1024) + conditions := []db.RuleCondition{{Field: "subject", Op: "contains", Value: "fwd"}} + if _, err := s.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "forward", "elsewhere@example.com", `{"keep_copy":false}`); err != nil { + t.Fatal(err) + } + + action, err := s.ApplyRules(mailboxID, map[string]string{"subject": "fwd: hi"}) + if err != nil { + t.Fatal(err) + } + if action.ForwardTo != "elsewhere@example.com" || action.KeepCopy { + t.Fatalf("expected forward to elsewhere@example.com with keep_copy=false, got %+v", action) + } +} + // TestApplyRulesLegacySingleConditionFallback confirms a rule row with an empty // ConditionsJSON (as any rule created before multi-condition support existed would // have) still evaluates correctly via the legacy condition_field/op/value columns. diff --git a/internal/smtpserver/mailbox_rules_test.go b/internal/smtpserver/mailbox_rules_test.go index ced7fff..a41f5a3 100644 --- a/internal/smtpserver/mailbox_rules_test.go +++ b/internal/smtpserver/mailbox_rules_test.go @@ -4,6 +4,8 @@ import ( "net/smtp" "strings" "testing" + + "mailgoserver/internal/db" ) func TestBlockedSenderRejectedAtRcpt(t *testing.T) { @@ -87,6 +89,57 @@ func TestAllowListBypassesSpamQuarantine(t *testing.T) { } } +// TestJunkListQuarantinesWithoutRejecting confirms a "junk" allowblock entry (the +// self-service webmail Blocklist — distinct from "block", which hard-rejects at RCPT +// instead) is still accepted at SMTP level but always lands in Junk, bypassing spam +// scoring entirely — the same soft-quarantine outcome as a heuristic/rspamd score hit, +// but decided unconditionally rather than computed. +func TestJunkListQuarantinesWithoutRejecting(t *testing.T) { + backend, mailboxID := newTestBackendWithMailbox(t) + if _, err := backend.DB.AddAllowBlockEntry(mailboxID, "junk", "test@example.com"); err != nil { + t.Fatal(err) + } + addr := startTestServer(t, backend) + + c, err := smtp.Dial(addr) + if err != nil { + t.Fatal(err) + } + defer c.Close() + if err := c.Auth(smtp.PlainAuth("", "test@example.com", "testpass123", "127.0.0.1")); err != nil { + t.Fatalf("auth: %v", err) + } + if err := c.Mail("test@example.com"); err != nil { + t.Fatalf("MAIL FROM: %v", err) + } + if err := c.Rcpt("inbox@example.com"); err != nil { + t.Fatalf("expected RCPT accepted (junk quarantines, doesn't reject), got: %v", err) + } + w, err := c.Data() + if err != nil { + t.Fatal(err) + } + w.Write([]byte("Subject: hi\r\n\r\nhi")) + if err := w.Close(); err != nil { + t.Fatalf("DATA: %v", err) + } + + inbox, err := backend.DB.ListMessagesInFolder(mailboxID, "INBOX") + if err != nil { + t.Fatal(err) + } + if len(inbox) != 0 { + t.Fatalf("expected nothing in INBOX, found %d", len(inbox)) + } + junk, err := backend.DB.ListMessagesInFolder(mailboxID, "Junk") + if err != nil { + t.Fatal(err) + } + if len(junk) != 1 { + t.Fatalf("expected 1 message quarantined in Junk, got %d", len(junk)) + } +} + func TestFilterRuleDeleteDropsMessage(t *testing.T) { backend, mailboxID := newTestBackendWithMailbox(t) if _, err := backend.DB.CreateRule(mailboxID, 0, "subject", "contains", "drop-me", "delete", ""); err != nil { @@ -168,6 +221,94 @@ func TestFilterRuleMarkReadSetsSeenFlag(t *testing.T) { } } +// TestFilterRuleForwardKeepCopyFalseDropsLocalCopy confirms a forward rule with +// keep_copy=false doesn't also store a local copy — forwarding itself happens +// fire-and-forget in a goroutine (see deliverLocally), so this only asserts on the +// local-storage decision, not on the (real, DNS-dependent) outbound delivery +// succeeding. ".invalid" is an RFC 2606 TLD guaranteed to never resolve, so the +// background forward attempt fails fast instead of hanging the test on a timeout. +func TestFilterRuleForwardKeepCopyFalseDropsLocalCopy(t *testing.T) { + backend, mailboxID := newTestBackendWithMailbox(t) + conditions := []db.RuleCondition{{Field: "subject", Op: "contains", Value: "fwd-drop"}} + if _, err := backend.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "forward", "elsewhere@forward-test.invalid", `{"keep_copy":false}`); err != nil { + t.Fatal(err) + } + addr := startTestServer(t, backend) + + c, err := smtp.Dial(addr) + if err != nil { + t.Fatal(err) + } + defer c.Close() + if err := c.Auth(smtp.PlainAuth("", "test@example.com", "testpass123", "127.0.0.1")); err != nil { + t.Fatalf("auth: %v", err) + } + if err := c.Mail("test@example.com"); err != nil { + t.Fatalf("MAIL FROM: %v", err) + } + if err := c.Rcpt("inbox@example.com"); err != nil { + t.Fatalf("RCPT: %v", err) + } + w, err := c.Data() + if err != nil { + t.Fatal(err) + } + w.Write([]byte("Subject: fwd-drop this\r\n\r\nhi")) + if err := w.Close(); err != nil { + t.Fatalf("expected DATA to still report success even though the rule forwards-and-drops, got: %v", err) + } + + msgs, err := backend.DB.ListMessagesInFolder(mailboxID, "INBOX") + if err != nil { + t.Fatal(err) + } + if len(msgs) != 0 { + t.Fatalf("expected forward with keep_copy=false to skip local storage, found %d messages", len(msgs)) + } +} + +// TestFilterRuleForwardKeepCopyTrueStillStoresLocally confirms the default +// (keep_copy=true) still stores the mailbox owner's own copy alongside forwarding. +func TestFilterRuleForwardKeepCopyTrueStillStoresLocally(t *testing.T) { + backend, mailboxID := newTestBackendWithMailbox(t) + conditions := []db.RuleCondition{{Field: "subject", Op: "contains", Value: "fwd-keep"}} + if _, err := backend.DB.CreateRuleMulti(mailboxID, 0, conditions, "all", "", "forward", "elsewhere@forward-test.invalid", `{"keep_copy":true}`); err != nil { + t.Fatal(err) + } + addr := startTestServer(t, backend) + + c, err := smtp.Dial(addr) + if err != nil { + t.Fatal(err) + } + defer c.Close() + if err := c.Auth(smtp.PlainAuth("", "test@example.com", "testpass123", "127.0.0.1")); err != nil { + t.Fatalf("auth: %v", err) + } + if err := c.Mail("test@example.com"); err != nil { + t.Fatalf("MAIL FROM: %v", err) + } + if err := c.Rcpt("inbox@example.com"); err != nil { + t.Fatalf("RCPT: %v", err) + } + w, err := c.Data() + if err != nil { + t.Fatal(err) + } + w.Write([]byte("Subject: fwd-keep this\r\n\r\nhi")) + if err := w.Close(); err != nil { + t.Fatalf("DATA: %v", err) + } + + msgs, err := backend.DB.ListMessagesInFolder(mailboxID, "INBOX") + if err != nil { + t.Fatal(err) + } + if len(msgs) != 1 { + t.Fatalf("expected forward with keep_copy=true to still store locally, found %d messages", len(msgs)) + } +} + func TestFilterRuleMoveToFolderStoresInNamedFolder(t *testing.T) { backend, mailboxID := newTestBackendWithMailbox(t) if _, err := backend.DB.CreateRule(mailboxID, 0, "subject", "contains", "spam", "move_to_folder", "Junk"); err != nil { diff --git a/internal/smtpserver/session.go b/internal/smtpserver/session.go index 4b2d591..2bdeead 100644 --- a/internal/smtpserver/session.go +++ b/internal/smtpserver/session.go @@ -10,11 +10,13 @@ import ( "time" "github.com/emersion/go-smtp" + "github.com/microcosm-cc/bluemonday" "gopkg.in/ini.v1" "mailgoserver/internal/abuseguard" "mailgoserver/internal/db" "mailgoserver/internal/dkim" "mailgoserver/internal/mailstore" + "mailgoserver/internal/mailview" "mailgoserver/internal/relay" "mailgoserver/internal/toolbox" ) @@ -428,6 +430,21 @@ func (s *Session) deliverLocally(rcpts, types []string, signedContent, messageID rspamdURL := s.backend.Cfg.Section("Rspamd").Key("url").MustString("http://127.0.0.1:11333") rspamdRejectScore := s.backend.Cfg.Section("Rspamd").Key("reject_score").MustInt(15) + // Parsed once for every local recipient (not per-recipient — same message body for + // all of them) so filter rules can match on body text / attachment presence + // without every mailbox needing its own parse pass. Best-effort: a message this + // package's own parser can't handle just never matches those two condition types. + bodyText, hasAttachment := "", "no" + if parsedForRules, err := mailview.Parse([]byte(signedContent)); err == nil { + bodyText = parsedForRules.TextBody + if bodyText == "" && parsedForRules.HTMLBody != "" { + bodyText = bluemonday.StrictPolicy().Sanitize(parsedForRules.HTMLBody) + } + if len(parsedForRules.Attachments) > 0 { + hasAttachment = "yes" + } + } + results := make([]relay.Result, 0, len(rcpts)) for i, rcpt := range rcpts { mbox := s.localMailboxes[strings.ToLower(rcpt)] @@ -439,32 +456,45 @@ func (s *Session) deliverLocally(rcpts, types []string, signedContent, messageID // other (additive, not either/or), but neither runs at all once allow-listed. spamGated := false if allowed, _ := s.backend.DB.IsAllowed(mbox.ID, s.mailFrom); !allowed { - quarantine := heuristicScore >= rejectScore - hardReject := false - if rspamdEnabled { - if score, rAction, err := mailstore.CheckRspamd(rspamdURL, []byte(signedContent), s.mailFrom, rcpt); err == nil { - // rspamd's own "reject" action is a considered policy decision - // (DNSBL hit, greylisting, etc.) worth still hard-rejecting at - // SMTP time to avoid backscatter; a bare score threshold hit - // (from either scorer) is quarantined instead of rejected, so a - // false positive is recoverable from the Junk folder rather than - // silently bounced with no trace. - if rAction == "reject" { - hardReject = true - } else if score >= float64(rspamdRejectScore) { - quarantine = true - } - } - // rspamd unreachable/erroring must not block mail — errors are swallowed, - // the built-in heuristic above is still the baseline gate either way. - } - if hardReject { - results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "550", ErrorMessage: "Message rejected as spam"}) - continue - } - if quarantine { + // The mailbox owner's own Blocklist (webmail Settings, or the message-view + // "Mark as Junk" action — internal/webui's webmailMarkAsJunk) also bypasses + // scoring entirely, straight to Junk: the user already told us how to + // treat this sender, so there's nothing left to compute (and no rspamd + // round-trip to make). Deliberately a *soft* quarantine (still delivered, + // just hidden), unlike admin's separate hard-reject block list + // (IsBlocked, checked at RCPT time — see Rcpt()) — those are different + // tools for different jobs, not two ways to do the same thing. + if junked, _ := s.backend.DB.IsJunked(mbox.ID, s.mailFrom); junked { folder = "Junk" spamGated = true + } else { + quarantine := heuristicScore >= rejectScore + hardReject := false + if rspamdEnabled { + if score, rAction, err := mailstore.CheckRspamd(rspamdURL, []byte(signedContent), s.mailFrom, rcpt); err == nil { + // rspamd's own "reject" action is a considered policy decision + // (DNSBL hit, greylisting, etc.) worth still hard-rejecting at + // SMTP time to avoid backscatter; a bare score threshold hit + // (from either scorer) is quarantined instead of rejected, so a + // false positive is recoverable from the Junk folder rather than + // silently bounced with no trace. + if rAction == "reject" { + hardReject = true + } else if score >= float64(rspamdRejectScore) { + quarantine = true + } + } + // rspamd unreachable/erroring must not block mail — errors are swallowed, + // the built-in heuristic above is still the baseline gate either way. + } + if hardReject { + results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "550", ErrorMessage: "Message rejected as spam"}) + continue + } + if quarantine { + folder = "Junk" + spamGated = true + } } } @@ -472,11 +502,34 @@ func (s *Session) deliverLocally(rcpts, types []string, signedContent, messageID // in their INBOX — a quarantined message skips them entirely and always lands // in Junk, rather than a rule accidentally routing spam back into view. if !spamGated { - action, err := s.backend.Mailstore.ApplyRules(mbox.ID, map[string]string{"from": s.mailFrom, "to": rcpt, "subject": subject}) + action, err := s.backend.Mailstore.ApplyRules(mbox.ID, map[string]string{ + "from": s.mailFrom, "to": rcpt, "subject": subject, + "body": bodyText, "has_attachment": hasAttachment, "recipient_type": types[i], + }) if err != nil { results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "450", ErrorMessage: err.Error()}) continue } + if action.ForwardTo != "" { + // Fire-and-forget: forwarding is a side effect layered on top of this + // recipient's own local delivery, not a substitute for it — a slow or + // unreachable forward target must never delay the SMTP response. + // Envelope-from is the mailbox's own address (not the original + // sender's) so this doesn't masquerade as a relay of someone else's + // mail; no SRS rewriting or Resent-* headers, matching every other + // send path in this codebase. + forwardTo, mailboxEmail := action.ForwardTo, mbox.Email + go func() { + res := s.backend.Relay.RelayEmailAsync(mailboxEmail, []string{forwardTo}, signedContent, []string{"to"}) + if len(res) > 0 && res[0].Status != "success" { + s.backend.Logger.Error("forward rule: delivery to %s failed: %s", forwardTo, res[0].ErrorMessage) + } + }() + if !action.KeepCopy { + results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: "Forwarded to " + forwardTo + ", not kept locally"}) + continue + } + } if action.Drop { results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: "Discarded by filter rule"}) continue diff --git a/internal/webui/mailbox_rules.go b/internal/webui/mailbox_rules.go index db6cf57..a2144f5 100644 --- a/internal/webui/mailbox_rules.go +++ b/internal/webui/mailbox_rules.go @@ -8,9 +8,14 @@ import ( "mailgoserver/internal/db" ) -var validConditionFields = map[string]bool{"from": true, "to": true, "subject": true} +var validConditionFields = map[string]bool{ + "from": true, "to": true, "subject": true, + "body": true, "has_attachment": true, "recipient_type": true, +} var validConditionOps = map[string]bool{"contains": true, "equals": true, "starts_with": true} -var validActions = map[string]bool{"move_to_folder": true, "delete": true, "mark_read": true, "mark_as_spam": true} +var validActions = map[string]bool{ + "move_to_folder": true, "delete": true, "mark_read": true, "mark_as_spam": true, "forward": true, +} // parseRuleConditions reads the rule-builder's parallel condition_field/op/value // arrays (one value per condition row, same index across all three) — shared by the @@ -35,8 +40,15 @@ func parseRuleConditions(r *http.Request) ([]db.RuleCondition, bool) { return conditions, true } +var conditionFieldLabels = map[string]string{ + "from": "from", "to": "to", "subject": "subject", "body": "body", + "has_attachment": "has attachment", "recipient_type": "I'm on the", +} + // summarizeConditions renders a rule's condition list as a human-readable string for -// display, e.g. `to contains "sales" AND subject contains "invoice"`. +// display, e.g. `to contains "sales" AND subject contains "invoice"`. A "\n"-joined +// multi-value condition (the rule builder's chip input) renders as a quoted, +// comma-separated list, e.g. `from is any of "a@x.com, b@y.com"`. func summarizeConditions(r db.MailboxFilterRule) string { conditions, matchType := r.Conditions() joiner := " AND " @@ -45,7 +57,25 @@ func summarizeConditions(r db.MailboxFilterRule) string { } parts := make([]string, len(conditions)) for i, c := range conditions { - parts[i] = c.Field + " " + strings.ReplaceAll(c.Op, "_", " ") + ` "` + c.Value + `"` + label := conditionFieldLabels[c.Field] + if label == "" { + label = c.Field + } + values := strings.Split(c.Value, "\n") + switch c.Field { + case "has_attachment": + parts[i] = label + ": " + c.Value + case "recipient_type": + parts[i] = label + " " + strings.ToUpper(c.Value) + " line" + case "": + parts[i] = "" + default: + op := strings.ReplaceAll(c.Op, "_", " ") + if len(values) > 1 { + op = "is any of" + } + parts[i] = label + " " + op + ` "` + strings.Join(values, ", ") + `"` + } } return strings.Join(parts, joiner) } @@ -90,7 +120,7 @@ func (a *App) addRule(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, Prefix+"/mailboxes/"+idStr(r)+"/rules", http.StatusFound) return } - if _, err := a.DB.CreateRuleMulti(mailbox.ID, priority, conditions, matchType, action, actionValue); err != nil { + if _, err := a.DB.CreateRuleMulti(mailbox.ID, priority, conditions, matchType, "", action, actionValue, ""); err != nil { setFlash(w, "error", "Error creating rule") } else { setFlash(w, "success", "Rule added") diff --git a/internal/webui/render.go b/internal/webui/render.go index a31c20c..d9c18f3 100644 --- a/internal/webui/render.go +++ b/internal/webui/render.go @@ -170,7 +170,7 @@ var standalonePages = []string{ "login.html", "login_mfa.html", "mfa_setup_required.html", "totp_setup.html", "webmail_login.html", "webmail_login_mfa.html", "webmail_account.html", "webmail_totp_setup.html", "webmail_mfa_setup_required.html", "webmail_folder.html", "webmail_message.html", "webmail_compose.html", "webmail_rules.html", "webmail_certs.html", - "webmail_signatures.html", + "webmail_signatures.html", "webmail_contacts.html", "webmail_blocklist.html", // A bare HTML fragment (no /base.html chrome at all), fetched via JS and // injected into logs.html's full-screen modal — not a page anyone navigates to // directly, so it doesn't need to look like a standalone document the way the @@ -188,7 +188,7 @@ var standalonePages = []string{ // something that opens a popup of its own. var pagesWithComposeWidget = []string{ "webmail_folder.html", "webmail_message.html", "webmail_rules.html", "webmail_certs.html", "webmail_account.html", - "webmail_signatures.html", + "webmail_signatures.html", "webmail_contacts.html", "webmail_blocklist.html", } func hasComposeWidget(page string) bool { @@ -200,6 +200,22 @@ func hasComposeWidget(page string) bool { return false } +// pagesWithSettingsChrome are the settings sections consolidated into one +// Outlook-style left-nav/right-content layout — see webmail_settings_chrome.html. +var pagesWithSettingsChrome = []string{ + "webmail_account.html", "webmail_rules.html", "webmail_signatures.html", "webmail_certs.html", + "webmail_contacts.html", "webmail_blocklist.html", +} + +func hasSettingsChrome(page string) bool { + for _, p := range pagesWithSettingsChrome { + if p == page { + return true + } + } + return false +} + // pagesWithShortcuts are the two pages keyboard shortcuts make sense on — the // message list (j/k/Enter/o) and a single open message (r/a/f/#). See // webmail_shortcuts.html's {{define "webmail_shortcuts"}}. @@ -233,6 +249,9 @@ func (a *App) loadTemplates() error { if hasComposeWidget(page) { files = append(files, "templates/webmail_compose_widget.html") } + if hasSettingsChrome(page) { + files = append(files, "templates/webmail_settings_chrome.html") + } if hasShortcuts(page) { files = append(files, "templates/webmail_shortcuts.html") } diff --git a/internal/webui/templates/csrf_script.html b/internal/webui/templates/csrf_script.html index 7990a17..1eb28a2 100644 --- a/internal/webui/templates/csrf_script.html +++ b/internal/webui/templates/csrf_script.html @@ -9,8 +9,12 @@ var token = window.__csrfToken; if (!token) return; // no session cookie yet (e.g. the login page itself) - document.addEventListener('DOMContentLoaded', function() { - document.querySelectorAll('form').forEach(function(form) { + // Exposed so content injected later (e.g. an AJAX-swapped settings section, + // see webmail_settings_chrome.html) can get the same treatment — forms + // added after DOMContentLoaded already fired would otherwise submit with no + // token and get rejected by the server-side check. + window.__applyCsrfToForms = function(root) { + (root || document).querySelectorAll('form').forEach(function(form) { if ((form.getAttribute('method') || '').toLowerCase() !== 'post') return; if (form.querySelector('input[name="csrf_token"]')) return; var input = document.createElement('input'); @@ -19,7 +23,8 @@ input.value = token; form.appendChild(input); }); - }); + }; + document.addEventListener('DOMContentLoaded', function() { window.__applyCsrfToForms(document); }); var mutating = { POST: true, PUT: true, PATCH: true, DELETE: true }; var originalFetch = window.fetch.bind(window); diff --git a/internal/webui/templates/webmail_account.html b/internal/webui/templates/webmail_account.html index e592fb7..e93346c 100644 --- a/internal/webui/templates/webmail_account.html +++ b/internal/webui/templates/webmail_account.html @@ -4,26 +4,24 @@ - {{.mailbox.Email}} - Webmail + Settings - Webmail + + {{template "webmail_settings_style" .}} {{template "csrf_script" .}} -