diff --git a/CHANGELOG.md b/CHANGELOG.md index 8af116e..f53bddb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,17 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version seventeen topics for Overview, Outbound, Inbound, Server and Account - and the boxes of nearly every page carry a `?` that opens their topic (owner, 2026-10-10). A reader sees the topics of the sections their menu shows. +- panel-redesign (design, stage 3): **a domain's DMARC report address is one + of three choices** - no reports (a new domain), the SelfPost hosted + address, or an address typed for the domain - set in Domain settings, with + a button that fills in one's own profile e-mail. The per-user default + report address, a domain "following" a user and the DMARC box on Account + are gone, and with them `users.dmarc_default_mode`, + `users.dmarc_default_address` and `domains.dmarc_rua_user_id` (owner, + 2026-10-10; replaces the design of 2026-09-21 described above). +- panel-redesign (design, stage 3): **New password** and **Delete** are in + each row of a domain's Applications table again, as POST forms that look + like the links beside them. - panel-redesign (stage 2): the **Outbound queue** page is in the new shell with its content unchanged - the listing of `postqueue -p` and how retries work. With it every page is built from the component kit: the old diff --git a/docs/assets/panel-redesign/evidence/account.png b/docs/assets/panel-redesign/evidence/account.png index a98f1f3..5867d10 100644 Binary files a/docs/assets/panel-redesign/evidence/account.png and b/docs/assets/panel-redesign/evidence/account.png differ diff --git a/docs/assets/panel-redesign/evidence/help.png b/docs/assets/panel-redesign/evidence/help.png index c87e66f..3713218 100644 Binary files a/docs/assets/panel-redesign/evidence/help.png and b/docs/assets/panel-redesign/evidence/help.png differ diff --git a/docs/assets/panel-redesign/evidence/out-domain-settings.png b/docs/assets/panel-redesign/evidence/out-domain-settings.png index 9ddeb21..f85401d 100644 Binary files a/docs/assets/panel-redesign/evidence/out-domain-settings.png and b/docs/assets/panel-redesign/evidence/out-domain-settings.png differ diff --git a/docs/assets/panel-redesign/evidence/out-domain.png b/docs/assets/panel-redesign/evidence/out-domain.png index 94d65e9..a1cf629 100644 Binary files a/docs/assets/panel-redesign/evidence/out-domain.png and b/docs/assets/panel-redesign/evidence/out-domain.png differ diff --git a/docs/assets/panel-redesign/evidence/user-delete.png b/docs/assets/panel-redesign/evidence/user-delete.png index f62b626..691b99c 100644 Binary files a/docs/assets/panel-redesign/evidence/user-delete.png and b/docs/assets/panel-redesign/evidence/user-delete.png differ diff --git a/internal/dmarc/service.go b/internal/dmarc/service.go index 7b1baa0..8cc59c7 100644 --- a/internal/dmarc/service.go +++ b/internal/dmarc/service.go @@ -64,26 +64,10 @@ func (s *Service) AllowedRecipients() ([]string, error) { seen[addr] = true out = append(out, addr) } - // A user's own default that points at this host is accepted even before a - // domain follows it, so the address can be published first. With no such - // default anywhere, the instance's generic mailbox stays open — the address - // the guide tells a fresh install to publish. - users, err := s.store.ListUsers() - if err != nil { - return nil, err - } - anyDefault := false - for _, u := range users { - if def := u.DMARCDefault(); def.Mode == store.DMARCDefaultAccount || def.Mode == store.DMARCDefaultCustom { - if addr := def.Resolve(""); addr != "" { - anyDefault = true - add(addr) - } - } - } - if !anyDefault { - add(DefaultHostedReportAddress(s.hostname)) - } + // The instance's generic mailbox is always open: it is the address the + // guide tells a fresh install to publish. A domain's own report address + // opens the mailbox it names, if that is on this host. + add(DefaultHostedReportAddress(s.hostname)) domains, err := s.store.ListDomains() if err != nil { return nil, err @@ -91,11 +75,7 @@ func (s *Service) AllowedRecipients() ([]string, error) { for _, d := range domains { // add keeps only addresses on this host, so a domain reporting elsewhere // (or nowhere) opens nothing here. - rua, err := s.store.DomainDMARCRua(d, HostedReportAddress(s.hostname, d.Name)) - if err != nil { - return nil, err - } - add(rua) + add(d.DMARCRua) } sort.Strings(out) return out, nil diff --git a/internal/dmarc/service_test.go b/internal/dmarc/service_test.go new file mode 100644 index 0000000..4486cc5 --- /dev/null +++ b/internal/dmarc/service_test.go @@ -0,0 +1,43 @@ +package dmarc + +import ( + "reflect" + "testing" + + "github.com/mixeme/selfpost/internal/store" +) + +// The ingest allow-list is the generic mailbox plus the report address of each +// domain that points at this host: a new domain (no reports) and a domain +// reporting to another host open nothing of their own. +func TestAllowedRecipientsFollowTheDomainAddresses(t *testing.T) { + st, err := store.Open(t.TempDir() + "/test.db") + if err != nil { + t.Fatalf("Open: %v", err) + } + defer st.Close() + hosted, _ := st.AddDomain("hosted.example", "sel") + typed, _ := st.AddDomain("typed.example", "sel") + if _, err := st.AddDomain("silent.example", "sel"); err != nil { + t.Fatalf("AddDomain: %v", err) + } + local, _ := st.AddDomain("local.example", "sel") + if err := st.SetDomainDMARCAddress(hosted.ID, HostedReportAddress("mail.example.com", "hosted.example")); err != nil { + t.Fatal(err) + } + if err := st.SetDomainDMARCAddress(typed.ID, "reports@elsewhere.example"); err != nil { + t.Fatal(err) + } + if err := st.SetDomainDMARCAddress(local.ID, "ops@mail.example.com"); err != nil { + t.Fatal(err) + } + + got, err := NewService(st, nil, "mail.example.com", true).AllowedRecipients() + if err != nil { + t.Fatalf("AllowedRecipients: %v", err) + } + want := []string{"dmarc-reports+hosted.example@mail.example.com", "dmarc-reports@mail.example.com", "ops@mail.example.com"} + if !reflect.DeepEqual(got, want) { + t.Errorf("allowed recipients = %v, want %v", got, want) + } +} diff --git a/internal/domain/transfer.go b/internal/domain/transfer.go index 07c9637..6148f8a 100644 --- a/internal/domain/transfer.go +++ b/internal/domain/transfer.go @@ -23,7 +23,7 @@ type DomainExport struct { Domain string `json:"domain"` DKIMSelector string `json:"dkim_selector"` DKIMPrivateKey string `json:"dkim_private_key"` // PKCS#1 PEM - DMARCRua *string `json:"dmarc_rua,omitempty"` // nil = follows a user's default; set = the domain's own address ("" = none) + DMARCRua string `json:"dmarc_rua,omitempty"` // the domain's report address; absent = no reports RateLimit *RateLimitExport `json:"rate_limit,omitempty"` Applications []AppExport `json:"applications"` } @@ -71,14 +71,9 @@ func (s *Service) Export(id int64) (DomainExport, error) { Domain: d.Name, DKIMSelector: d.DKIMSelector, DKIMPrivateKey: string(pem), + DMARCRua: d.DMARCRua, Applications: make([]AppExport, 0, len(apps)), } - // A domain that follows a user's default exports no address: users do not - // travel with a domain, and whoever imports it becomes the one it follows. - if !d.DMARCRuaUserID.Valid { - rua := d.DMARCRua - exp.DMARCRua = &rua - } rl, ok, err := s.store.GetRateLimit(store.RateLimitScopeDomain, id) if err != nil { return DomainExport{}, fmt.Errorf("export rate limit for %s: %w", d.Name, err) @@ -147,12 +142,12 @@ func (s *Service) Import(exp DomainExport) (store.Domain, error) { return store.Domain{}, err } - if exp.DMARCRua != nil { - if err := s.store.SetDomainDMARCAddress(d.ID, *exp.DMARCRua); err != nil { + if exp.DMARCRua != "" { + if err := s.store.SetDomainDMARCAddress(d.ID, exp.DMARCRua); err != nil { s.importRollback(d.ID) return store.Domain{}, err } - d.DMARCRua = *exp.DMARCRua + d.DMARCRua = exp.DMARCRua } if exp.RateLimit != nil { if err := s.importRateLimit(store.RateLimitScopeDomain, d.ID, *exp.RateLimit); err != nil { diff --git a/internal/domain/transfer_test.go b/internal/domain/transfer_test.go index 8cb5b67..846077e 100644 --- a/internal/domain/transfer_test.go +++ b/internal/domain/transfer_test.go @@ -110,11 +110,11 @@ func TestExportImportRoundTrip(t *testing.T) { if got.Name != "example.com" || got.DKIMSelector != "selfpost" { t.Errorf("imported domain = %+v", got) } - if got.DMARCRuaUserID.Valid || got.DMARCRua != "reports@hub.example" { - t.Errorf("imported dmarc rua = %q (follows user: %v)", got.DMARCRua, got.DMARCRuaUserID.Valid) + if got.DMARCRua != "reports@hub.example" { + t.Errorf("imported dmarc rua = %q", got.DMARCRua) } - if exp.DMARCRua == nil || *exp.DMARCRua != "reports@hub.example" { - t.Errorf("exported dmarc rua = %v", exp.DMARCRua) + if exp.DMARCRua != "reports@hub.example" { + t.Errorf("exported dmarc rua = %q", exp.DMARCRua) } // The DKIM key was imported byte-for-byte, so the DNS record is unchanged. dstKey, err := dstOdk.ExportKey("example.com", "selfpost") diff --git a/internal/store/domains.go b/internal/store/domains.go index 0e0821a..3d21a0e 100644 --- a/internal/store/domains.go +++ b/internal/store/domains.go @@ -25,13 +25,13 @@ type Domain struct { ID int64 Name string DKIMSelector string - // Where DMARC aggregate reports go. With DMARCRuaUserID set the domain - // follows that user's default (DMARCDefault) and DMARCRua is ignored; - // otherwise DMARCRua is the address itself, "" meaning no reports. - DMARCRua string - DMARCRuaUserID sql.NullInt64 - CreatedAt time.Time - AppCount int + // Where DMARC aggregate reports go: the address itself, "" meaning no + // reports (which is what a new domain has). Whether it is the hosted + // address or one typed for the domain is told by its value; the store does + // not know the server's hostname. + DMARCRua string + CreatedAt time.Time + AppCount int } // AddDomain inserts a new sending domain. The caller is responsible for having @@ -60,7 +60,7 @@ func (s *Store) AddDomain(name, selector string) (Domain, error) { // ordered by name. func (s *Store) ListDomains() ([]Domain, error) { rows, err := s.db.Query(` - SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.dmarc_rua_user_id, d.created_at, + SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.created_at, (SELECT COUNT(*) FROM applications a WHERE a.domain_id = d.id) FROM domains d ORDER BY d.name`) @@ -86,7 +86,7 @@ func (s *Store) ListDomains() ([]Domain, error) { // The filter is in the query, so a caller cannot forget it. func (s *Store) ListDomainsForUser(userID int64) ([]Domain, error) { rows, err := s.db.Query(` - SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.dmarc_rua_user_id, d.created_at, + SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.created_at, (SELECT COUNT(*) FROM applications a WHERE a.domain_id = d.id) FROM domains d WHERE EXISTS (SELECT 1 FROM users u WHERE u.id = ? AND u.all_domains = 1) @@ -112,7 +112,7 @@ func (s *Store) ListDomainsForUser(userID int64) ([]Domain, error) { // ErrDomainNotFound. func (s *Store) GetDomain(id int64) (Domain, error) { row := s.db.QueryRow(` - SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.dmarc_rua_user_id, d.created_at, + SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.created_at, (SELECT COUNT(*) FROM applications a WHERE a.domain_id = d.id) FROM domains d WHERE d.id = ?`, id) @@ -154,7 +154,7 @@ func scanDomain(r scanRow) (Domain, error) { d Domain createdAt string ) - if err := r.Scan(&d.ID, &d.Name, &d.DKIMSelector, &d.DMARCRua, &d.DMARCRuaUserID, &createdAt, &d.AppCount); err != nil { + if err := r.Scan(&d.ID, &d.Name, &d.DKIMSelector, &d.DMARCRua, &createdAt, &d.AppCount); err != nil { return Domain{}, err } d.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) @@ -172,21 +172,10 @@ func isUniqueViolation(err error) bool { return false } -// SetDomainDMARCAddress gives the domain a report address of its own; "" means -// no aggregate reports. The domain stops following anyone's default. +// SetDomainDMARCAddress sets where the domain's aggregate reports go; "" means +// no reports. func (s *Store) SetDomainDMARCAddress(id int64, rua string) error { - return s.updateDomainDMARC(id, rua, sql.NullInt64{}) -} - -// SetDomainDMARCUser makes the domain follow userID's default report address -// (DMARCDefault). If that user is later deleted the domain falls back to no -// reports rather than to someone else's default. -func (s *Store) SetDomainDMARCUser(id, userID int64) error { - return s.updateDomainDMARC(id, "", sql.NullInt64{Int64: userID, Valid: true}) -} - -func (s *Store) updateDomainDMARC(id int64, rua string, userID sql.NullInt64) error { - res, err := s.db.Exec("UPDATE domains SET dmarc_rua = ?, dmarc_rua_user_id = ? WHERE id = ?", rua, userID, id) + res, err := s.db.Exec("UPDATE domains SET dmarc_rua = ? WHERE id = ?", rua, id) if err != nil { return fmt.Errorf("update domain dmarc rua: %w", err) } @@ -200,24 +189,6 @@ func (s *Store) updateDomainDMARC(id int64, rua string, userID sql.NullInt64) er return nil } -// DomainDMARCRua resolves where the domain's aggregate reports go: its own -// address, or the default of the user it follows. hosted is the SelfPost-hosted -// mailbox for this domain, "" when report ingest is off (the store does not -// know the server's hostname). The result is "" for a policy-only record. -func (s *Store) DomainDMARCRua(d Domain, hosted string) (string, error) { - if !d.DMARCRuaUserID.Valid { - return d.DMARCRua, nil - } - def, err := s.GetDMARCDefault(d.DMARCRuaUserID.Int64) - if errors.Is(err, ErrUserNotFound) { - return "", nil - } - if err != nil { - return "", err - } - return def.Resolve(hosted), nil -} - // DomainExists reports whether a sending domain with this name is configured. // Used by the DMARC ingest path to refuse reports for domains this relay does // not send for (security.md: report XML is attacker-supplied input). diff --git a/internal/store/domains_dmarc_test.go b/internal/store/domains_dmarc_test.go index 74a35e1..d9b08e3 100644 --- a/internal/store/domains_dmarc_test.go +++ b/internal/store/domains_dmarc_test.go @@ -5,11 +5,9 @@ import ( "testing" ) -const hostedForTest = "dmarc+example.com@mail.example.org" - -// A domain either has a report address of its own or follows a user's default; -// the two never mix, and a fresh domain has neither. -func TestDomainDMARCAddressAndUser(t *testing.T) { +// A domain has one report address and nothing else: a fresh domain has none, +// the address is whatever was set last, and "" is no reports. +func TestDomainDMARCAddress(t *testing.T) { st := openTestStore(t) d, err := st.AddDomain("example.com", "sel") if err != nil { @@ -19,123 +17,50 @@ func TestDomainDMARCAddressAndUser(t *testing.T) { if err != nil { t.Fatalf("GetDomain: %v", err) } - if got.DMARCRua != "" || got.DMARCRuaUserID.Valid { - t.Fatalf("a new domain = %q / follows %v, want no reports", got.DMARCRua, got.DMARCRuaUserID) + if got.DMARCRua != "" { + t.Fatalf("a new domain reports to %q, want no reports", got.DMARCRua) } - if err := st.SetDomainDMARCAddress(d.ID, "reports@hub.com"); err != nil { - t.Fatalf("SetDomainDMARCAddress: %v", err) - } - got, _ = st.GetDomain(d.ID) - if rua, err := st.DomainDMARCRua(got, hostedForTest); err != nil || rua != "reports@hub.com" { - t.Fatalf("own address resolves to %q, %v", rua, err) - } - - if err := st.CreateGlobalUser("admin", "hash"); err != nil { - t.Fatalf("CreateGlobalUser: %v", err) - } - u, _ := st.GetUserByUsername("admin") - if err := st.SetDomainDMARCUser(d.ID, u.ID); err != nil { - t.Fatalf("SetDomainDMARCUser: %v", err) - } - got, _ = st.GetDomain(d.ID) - if got.DMARCRua != "" || !got.DMARCRuaUserID.Valid || got.DMARCRuaUserID.Int64 != u.ID { - t.Fatalf("following a user left %q / %v; the own address must be dropped", got.DMARCRua, got.DMARCRuaUserID) - } - - if err := st.SetDomainDMARCAddress(d.ID, ""); err != nil { - t.Fatalf("SetDomainDMARCAddress none: %v", err) - } - got, _ = st.GetDomain(d.ID) - if got.DMARCRua != "" || got.DMARCRuaUserID.Valid { - t.Fatalf("none = %q / %v", got.DMARCRua, got.DMARCRuaUserID) + for _, rua := range []string{"reports@hub.com", "dmarc+example.com@mail.example.org", ""} { + if err := st.SetDomainDMARCAddress(d.ID, rua); err != nil { + t.Fatalf("SetDomainDMARCAddress(%q): %v", rua, err) + } + if got, _ = st.GetDomain(d.ID); got.DMARCRua != rua { + t.Fatalf("set %q, read %q", rua, got.DMARCRua) + } } if err := st.SetDomainDMARCAddress(d.ID+100, "x@y.com"); !errors.Is(err, ErrDomainNotFound) { - t.Fatalf("unknown domain = %v, want ErrDomainNotFound", err) + t.Errorf("unknown domain = %v, want ErrDomainNotFound", err) } } -// A domain that follows a user resolves to whatever that user's default says -// today — every mode — and changes with it. -func TestDomainFollowsUserDefault(t *testing.T) { +// Deleting a user changes no domain's report address: the address belongs to +// the domain, not to whoever set it. +func TestDeletingAUserLeavesEveryDomainAddressAlone(t *testing.T) { st := openTestStore(t) if err := st.CreateGlobalUser("admin", "hash"); err != nil { t.Fatalf("CreateGlobalUser: %v", err) } - u, _ := st.GetUserByUsername("admin") - if err := st.UpdateUser(u.ID, "admin", "hash", "mix@example.org"); err != nil { - t.Fatalf("UpdateUser: %v", err) - } - d, _ := st.AddDomain("example.com", "sel") - if err := st.SetDomainDMARCUser(d.ID, u.ID); err != nil { - t.Fatalf("SetDomainDMARCUser: %v", err) - } - d, _ = st.GetDomain(d.ID) - - for _, tc := range []struct { - mode, address string - hosted string - want string - }{ - {DMARCDefaultNone, "", hostedForTest, ""}, - {DMARCDefaultAccount, "", hostedForTest, "mix@example.org"}, - {DMARCDefaultCustom, "dmarc@hub.example", hostedForTest, "dmarc@hub.example"}, - {DMARCDefaultHosted, "", hostedForTest, hostedForTest}, - // Ingest switched off: a hosted default has no mailbox to point at. - {DMARCDefaultHosted, "", "", ""}, - // The custom address belongs to the custom mode only. - {DMARCDefaultAccount, "ignored@hub.example", hostedForTest, "mix@example.org"}, - } { - if err := st.SetDMARCDefault(u.ID, tc.mode, tc.address); err != nil { - t.Fatalf("SetDMARCDefault(%s): %v", tc.mode, err) - } - got, err := st.DomainDMARCRua(d, tc.hosted) - if err != nil || got != tc.want { - t.Errorf("mode %s, hosted %q: resolved %q, %v; want %q", tc.mode, tc.hosted, got, err, tc.want) - } - } - def, err := st.GetDMARCDefault(u.ID) - if err != nil || def.Address != "" || def.Username != "admin" { - t.Errorf("default after a non-custom mode = %+v, %v; the custom address must be cleared", def, err) - } - if err := st.SetDMARCDefault(u.ID, "profile", ""); err == nil { - t.Error("an unknown mode was stored") - } - if err := st.SetDMARCDefault(u.ID+100, DMARCDefaultNone, ""); !errors.Is(err, ErrUserNotFound) { - t.Errorf("unknown user = %v, want ErrUserNotFound", err) - } -} - -// Deleting the user a domain follows must leave the domain with no report -// address — not with another user's default, and not with a dangling id. -func TestDeletingTheFollowedUserLeavesNoReports(t *testing.T) { - st := openTestStore(t) - if err := st.CreateGlobalUser("admin", "hash"); err != nil { - t.Fatalf("CreateGlobalUser: %v", err) - } - d, _ := st.AddDomain("example.com", "sel") id, err := st.CreateUser("second", "hash", RoleGlobal, Reach{}) if err != nil { t.Fatalf("CreateUser: %v", err) } - if err := st.SetDMARCDefault(id, DMARCDefaultCustom, "dmarc@hub.example"); err != nil { - t.Fatalf("SetDMARCDefault: %v", err) + if err := st.UpdateUser(id, "second", "hash", "second@hub.example"); err != nil { + t.Fatalf("UpdateUser: %v", err) } - if err := st.SetDomainDMARCUser(d.ID, id); err != nil { - t.Fatalf("SetDomainDMARCUser: %v", err) + typed, _ := st.AddDomain("typed.example", "sel") + none, _ := st.AddDomain("none.example", "sel") + if err := st.SetDomainDMARCAddress(typed.ID, "second@hub.example"); err != nil { + t.Fatalf("SetDomainDMARCAddress: %v", err) } if err := st.DeleteUser(id); err != nil { t.Fatalf("DeleteUser: %v", err) } - got, err := st.GetDomain(d.ID) - if err != nil { - t.Fatalf("GetDomain: %v", err) + if got, _ := st.GetDomain(typed.ID); got.DMARCRua != "second@hub.example" { + t.Errorf("typed address after deleting the user = %q", got.DMARCRua) } - if got.DMARCRuaUserID.Valid || got.DMARCRua != "" { - t.Fatalf("after deleting the followed user: %q / %v, want no reports", got.DMARCRua, got.DMARCRuaUserID) - } - if rua, err := st.DomainDMARCRua(got, hostedForTest); err != nil || rua != "" { - t.Fatalf("resolved %q, %v; want no reports", rua, err) + if got, _ := st.GetDomain(none.ID); got.DMARCRua != "" { + t.Errorf("no-reports domain after deleting the user = %q", got.DMARCRua) } } diff --git a/internal/store/migrations/0001_init.sql b/internal/store/migrations/0001_init.sql index e6c5785..693c83d 100644 --- a/internal/store/migrations/0001_init.sql +++ b/internal/store/migrations/0001_init.sql @@ -24,27 +24,21 @@ CREATE TABLE settings ( -- exists. role is a reach, not a rank: 'global' sees the whole instance, -- 'domain' only what is assigned below. -- --- email is the user's own address — what the panel writes to. The DMARC --- default is a separate choice that may use it: dmarc_default_mode says where --- aggregate reports go for a domain that follows this user --- (domains.dmarc_rua_user_id): 'hosted' (SelfPost's own mailbox for that --- domain), 'account' (email above), 'custom' (dmarc_default_address) or 'none'. +-- email is the user's own address — what the panel writes to, and what the +-- button beside a domain's typed report address fills it with. -- -- all_domains / all_inbound_domains widen a 'domain' user's list to every -- domain of that direction, including the ones added later; with a flag set -- the rows of the matching assignment table are ignored. CREATE TABLE users ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - username TEXT NOT NULL UNIQUE, - password_hash TEXT NOT NULL, - role TEXT NOT NULL CHECK (role IN ('global', 'domain')), - email TEXT NOT NULL DEFAULT '', - dmarc_default_mode TEXT NOT NULL DEFAULT 'none' - CHECK (dmarc_default_mode IN ('hosted', 'account', 'custom', 'none')), - dmarc_default_address TEXT NOT NULL DEFAULT '', - all_domains INTEGER NOT NULL DEFAULT 0 CHECK (all_domains IN (0, 1)), - all_inbound_domains INTEGER NOT NULL DEFAULT 0 CHECK (all_inbound_domains IN (0, 1)), - created_at TEXT NOT NULL + id INTEGER PRIMARY KEY AUTOINCREMENT, + username TEXT NOT NULL UNIQUE, + password_hash TEXT NOT NULL, + role TEXT NOT NULL CHECK (role IN ('global', 'domain')), + email TEXT NOT NULL DEFAULT '', + all_domains INTEGER NOT NULL DEFAULT 0 CHECK (all_domains IN (0, 1)), + all_inbound_domains INTEGER NOT NULL DEFAULT 0 CHECK (all_inbound_domains IN (0, 1)), + created_at TEXT NOT NULL ); -- Panel login sessions. Persisted so a login survives a container restart or @@ -65,18 +59,17 @@ CREATE INDEX idx_sessions_expires_at ON sessions (expires_at); -- Sending (outbound) domains. DKIM keys themselves live on disk under /data; -- this row records the selector and metadata. -- --- Where the domain's DMARC aggregate reports go (rua=): with dmarc_rua_user_id --- set, the domain follows that user's default and dmarc_rua is ignored; --- otherwise dmarc_rua is the address itself, '' meaning no reports. Deleting --- the user leaves the domain at "none" rather than silently handing its --- reports to someone else. +-- Where the domain's DMARC aggregate reports go (rua=): dmarc_rua is the +-- address itself, '' meaning no reports — which is what a new domain has. The +-- three choices (no reports, SelfPost hosted, an address typed for the domain) +-- are told apart by the value: the hosted one is the address SelfPost derives +-- for the domain and the server's hostname. No user is involved. CREATE TABLE domains ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT NOT NULL UNIQUE, - dkim_selector TEXT NOT NULL, - dmarc_rua TEXT NOT NULL DEFAULT '', - dmarc_rua_user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, - created_at TEXT NOT NULL + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + dkim_selector TEXT NOT NULL, + dmarc_rua TEXT NOT NULL DEFAULT '', + created_at TEXT NOT NULL ); -- Outbound domains assigned to a 'domain' user. diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 1d07052..e602462 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -40,9 +40,9 @@ func TestBaselineSchema(t *testing.T) { st := openTestStore(t) want := map[string]string{ "settings": "key value", - "users": "id username password_hash role email dmarc_default_mode dmarc_default_address all_domains all_inbound_domains created_at", + "users": "id username password_hash role email all_domains all_inbound_domains created_at", "sessions": "token_hash user_id created_at expires_at", - "domains": "id name dkim_selector dmarc_rua dmarc_rua_user_id created_at", + "domains": "id name dkim_selector dmarc_rua created_at", "user_domains": "user_id domain_id", "applications": "id domain_id login address_mode auth_ip_restrict auth_allowed_ips created_at", "application_addresses": "id application_id address", @@ -223,33 +223,30 @@ func TestOpenRefusesANewerSchema(t *testing.T) { // table does when either side goes away. func TestBaselineConstraints(t *testing.T) { st := openTestStore(t) - insert := func(role, mode string, all int) error { + insert := func(name, role string, all int) error { _, err := st.db.Exec( - "INSERT INTO users (username, password_hash, role, dmarc_default_mode, all_domains, created_at) VALUES (?, 'h', ?, ?, ?, 'now')", - role+mode, role, mode, all) + "INSERT INTO users (username, password_hash, role, all_domains, created_at) VALUES (?, 'h', ?, ?, 'now')", + name, role, all) return err } - if err := insert("global", "none", 0); err != nil { + if err := insert("plain", "global", 0); err != nil { t.Fatalf("a plain global user: %v", err) } - if err := insert("domain", "hosted", 1); err != nil { + if err := insert("everything", "domain", 1); err != nil { t.Fatalf("a domain user with All: %v", err) } - if err := insert("domain_admin", "none", 0); err == nil { + if err := insert("old-role", "domain_admin", 0); err == nil { t.Error("the 1.x role name domain_admin was accepted") } - if err := insert("global", "profile", 0); err == nil { - t.Error("an unknown dmarc_default_mode was accepted") - } - if err := insert("domain", "custom", 2); err == nil { + if err := insert("two", "domain", 2); err == nil { t.Error("all_domains = 2 was accepted") } - u, err := st.GetUserByUsername("domainhosted") + u, err := st.GetUserByUsername("everything") if err != nil { t.Fatalf("GetUserByUsername: %v", err) } - if !u.AllDomains || u.AllInboundDomains || u.DMARCDefaultMode != DMARCDefaultHosted { + if !u.AllDomains || u.AllInboundDomains { t.Errorf("flags read back as %+v", u) } diff --git a/internal/store/users.go b/internal/store/users.go index 898a597..3526f61 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -29,59 +29,15 @@ const ( RoleDomain Role = "domain" ) -// How a user's default DMARC report address is chosen (users.dmarc_default_mode). -const ( - DMARCDefaultHosted = "hosted" // SelfPost's own mailbox for the domain - DMARCDefaultAccount = "account" // the user's account e-mail - DMARCDefaultCustom = "custom" // DMARCDefault.Address - DMARCDefaultNone = "none" // no aggregate reports -) - -// DMARCDefault is a user's default report address: the choice a domain takes -// over when it follows that user (Domain.DMARCRuaUserID). -type DMARCDefault struct { - UserID int64 - Username string - Mode string - Address string // for DMARCDefaultCustom - Email string // the account e-mail, for DMARCDefaultAccount -} - -// Resolve returns the address the default stands for on one domain. hosted is -// the SelfPost-hosted mailbox for that domain, "" when report ingest is off — -// a hosted default then resolves to no reports. -func (d DMARCDefault) Resolve(hosted string) string { - switch d.Mode { - case DMARCDefaultHosted: - return hosted - case DMARCDefaultAccount: - return d.Email - case DMARCDefaultCustom: - return d.Address - } - return "" -} - -// ValidDMARCDefaultMode reports whether mode is one of the stored choices. -func ValidDMARCDefaultMode(mode string) bool { - switch mode { - case DMARCDefaultHosted, DMARCDefaultAccount, DMARCDefaultCustom, DMARCDefaultNone: - return true - } - return false -} - // User is a panel login (not an application SASL account). type User struct { ID int64 Username string PasswordHash string Role Role - // Email is the user's own address — what the panel writes to. DMARC may - // use it (DMARCDefaultAccount) but it is not a DMARC field. - Email string - DMARCDefaultMode string - DMARCDefaultAddress string + // Email is the user's own address — what the panel writes to, and what the + // button beside a domain's typed report address fills it with. + Email string // AllDomains / AllInboundDomains widen a domain user's list to every // domain of that direction, including those added later; the id lists // are then ignored. @@ -92,19 +48,14 @@ type User struct { InboundDomainIDs []int64 } -// DMARCDefault returns the user's default report address choice. -func (u User) DMARCDefault() DMARCDefault { - return DMARCDefault{UserID: u.ID, Username: u.Username, Mode: u.DMARCDefaultMode, Address: u.DMARCDefaultAddress, Email: u.Email} -} - -const userColumns = "id, username, password_hash, role, email, dmarc_default_mode, dmarc_default_address, all_domains, all_inbound_domains, created_at" +const userColumns = "id, username, password_hash, role, email, all_domains, all_inbound_domains, created_at" func scanUser(r scanRow) (User, error) { var ( u User createdAt string ) - err := r.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.Role, &u.Email, &u.DMARCDefaultMode, &u.DMARCDefaultAddress, + err := r.Scan(&u.ID, &u.Username, &u.PasswordHash, &u.Role, &u.Email, &u.AllDomains, &u.AllInboundDomains, &createdAt) if err != nil { return User{}, err @@ -318,47 +269,6 @@ func (s *Store) UpdateUser(id int64, username, passwordHash, email string) error return nil } -// SetDMARCDefault stores the user's default report address choice. address is -// kept only for DMARCDefaultCustom. Every domain that follows this user -// resolves to the new choice from the next read on. -func (s *Store) SetDMARCDefault(userID int64, mode, address string) error { - if !ValidDMARCDefaultMode(mode) { - return fmt.Errorf("set dmarc default: unknown mode %q", mode) - } - if mode != DMARCDefaultCustom { - address = "" - } - res, err := s.db.Exec( - "UPDATE users SET dmarc_default_mode = ?, dmarc_default_address = ? WHERE id = ?", mode, address, userID) - if err != nil { - return fmt.Errorf("set dmarc default: %w", err) - } - n, err := res.RowsAffected() - if err != nil { - return fmt.Errorf("set dmarc default: %w", err) - } - if n == 0 { - return ErrUserNotFound - } - return nil -} - -// GetDMARCDefault returns a user's default report address choice, or -// ErrUserNotFound. -func (s *Store) GetDMARCDefault(userID int64) (DMARCDefault, error) { - d := DMARCDefault{UserID: userID} - err := s.db.QueryRow( - "SELECT username, dmarc_default_mode, dmarc_default_address, email FROM users WHERE id = ?", userID, - ).Scan(&d.Username, &d.Mode, &d.Address, &d.Email) - if errors.Is(err, sql.ErrNoRows) { - return DMARCDefault{}, ErrUserNotFound - } - if err != nil { - return DMARCDefault{}, fmt.Errorf("get dmarc default: %w", err) - } - return d, nil -} - // SetUserRole updates a user's role. func (s *Store) SetUserRole(userID int64, role Role) error { res, err := s.db.Exec("UPDATE users SET role = ? WHERE id = ?", role, userID) diff --git a/internal/store/users_test.go b/internal/store/users_test.go index 3e3acbf..69367f7 100644 --- a/internal/store/users_test.go +++ b/internal/store/users_test.go @@ -26,11 +26,8 @@ func TestUpdateUser(t *testing.T) { if got.Email != "reports@hub.example" { t.Fatalf("email = %q", got.Email) } - // The account e-mail is the user's own: saving it does not make it anyone's - // report address, and nothing is mirrored into the instance settings. - if got.DMARCDefaultMode != DMARCDefaultNone { - t.Fatalf("dmarc default mode = %q after setting the e-mail, want %q", got.DMARCDefaultMode, DMARCDefaultNone) - } + // The account e-mail is the user's own: nothing is mirrored into the + // instance settings. if v, err := st.GetSetting("dmarc_report_email"); err == nil && v != "" { t.Fatalf("the e-mail leaked into the instance settings: %q", v) } diff --git a/internal/web/handlers/authz.go b/internal/web/handlers/authz.go index 4d3c337..327083f 100644 --- a/internal/web/handlers/authz.go +++ b/internal/web/handlers/authz.go @@ -50,13 +50,6 @@ func (h *Handlers) hostedDMARCAddress(domainName string) string { return dmarc.HostedReportAddress(h.cfg.Hostname, domainName) } -// domainReportAddress resolves where a domain's DMARC aggregate reports go: -// its own address, or the default of the user it follows. "" is a policy-only -// record. -func (h *Handlers) domainReportAddress(d store.Domain) (string, error) { - return h.store.DomainDMARCRua(d, h.hostedDMARCAddress(d.Name)) -} - func domainNameSet(domains []store.Domain) map[string]bool { m := make(map[string]bool, len(domains)) for _, d := range domains { diff --git a/internal/web/handlers/handlers_account.go b/internal/web/handlers/handlers_account.go index 3fba3e2..c06c4c7 100644 --- a/internal/web/handlers/handlers_account.go +++ b/internal/web/handlers/handlers_account.go @@ -1,13 +1,10 @@ package handlers import ( - "context" "errors" "net/http" "strings" - "time" - "github.com/mixeme/selfpost/internal/dnscheck" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" "github.com/mixeme/selfpost/internal/web/view" @@ -15,22 +12,20 @@ import ( ) // Account is the signed-in user's own half of what used to be one Settings -// page: who they are to the panel (username, e-mail), their password, and their -// default DMARC report address. Every role has it. Each part is its own form -// and its own POST, so changing one never asks for the others. +// page: who they are to the panel (username, e-mail) and their password. Every +// role has it. Each part is its own form and its own POST, so changing one +// never asks for the others. // -// The e-mail is the user's — what the panel writes to. The DMARC default is a -// separate choice that may use it: a domain whose report address is "this -// user's default" resolves to whatever is chosen here (store.DMARCDefault). +// The e-mail is the user's — what the panel writes to. A domain's DMARC report +// address is set in the domain's settings, where a button fills it with this +// e-mail; nothing here refers to a domain. // accountForm is what the page shows back: the stored values, or the ones just // submitted when a form is re-rendered with an error. type accountForm struct { - Err string - Username string - Email string - DMARCMode string - DMARCAddress string + Err string + Username string + Email string } func (h *Handlers) accountUser(w http.ResponseWriter, r *http.Request) (store.User, bool) { @@ -49,7 +44,7 @@ func (h *Handlers) accountUser(w http.ResponseWriter, r *http.Request) (store.Us } func formOf(u store.User) accountForm { - return accountForm{Username: u.Username, Email: u.Email, DMARCMode: u.DMARCDefaultMode, DMARCAddress: u.DMARCDefaultAddress} + return accountForm{Username: u.Username, Email: u.Email} } // HandleAccount shows the Account page. @@ -62,56 +57,18 @@ func (h *Handlers) HandleAccount(w http.ResponseWriter, r *http.Request) { } func (h *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, status int, u store.User, f accountForm) { - hosted := h.dmarc != nil && h.cfg.DMARCEnabled - page := view.NewAccount(h.shellMeta(r), string(u.Role), hosted, u.Email) + page := view.NewAccount(h.shellMeta(r), string(u.Role)) page.Username, page.Email = f.Username, f.Email - page.DMARCSelected, page.DMARCAddress = f.DMARCMode, f.DMARCAddress page.WithResult(accountFlash(r), f.Err) - page.WithDomainUse(h.domainsFollowing(r, u)) - - // The stored default, not the one being typed: the authorization record - // belongs to the address domains are actually told to publish. - def := u.DMARCDefault() - if addr := def.Resolve(""); addr != "" && def.Mode != store.DMARCDefaultHosted { - if hub := dnscheck.EmailDomain(addr); hub != "" { - ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) - ra := h.dns.ReportAuth(ctx, hub) - cancel() - page.WithAuthorization(dnscheck.ReportAuthRecordName(hub), dnscheck.ReportAuthExample(), - string(ra.Status), ra.Detail, ra.Records) - } - } h.view.Render(w, status, "account", page) } -// domainsFollowing counts the outbound domains the user reaches and how many of -// them take their report address from this user's default. -func (h *Handlers) domainsFollowing(r *http.Request, u store.User) (following, total int) { - p, ok := h.principal(r) - if !ok { - return 0, 0 - } - domains, err := h.assignedDomains(p) - if err != nil { - logf("panel: account: list domains: %v", err) - return 0, 0 - } - for _, d := range domains { - if d.DMARCRuaUserID.Valid && d.DMARCRuaUserID.Int64 == u.ID { - following++ - } - } - return following, len(domains) -} - func accountFlash(r *http.Request) string { switch r.URL.Query().Get("done") { case "profile": return "Profile saved." case "password": return "Password changed. Any other signed-in sessions were signed out." - case "dmarc": - return "Default report address saved." } return "" } @@ -155,9 +112,6 @@ func (h *Handlers) HandleAccountProfile(w http.ResponseWriter, r *http.Request) fail(http.StatusInternalServerError, "Could not save the profile. Please check the logs and try again.") return } - // The e-mail is the report address of every domain that follows this - // user's "my account e-mail" default. - h.resyncAfterEmailChange(u, f.Email) logf("panel: user %d profile updated (username: %t, e-mail: %t)", u.ID, f.Username != u.Username, f.Email != u.Email) http.Redirect(w, r, "/account?done=profile", http.StatusSeeOther) } @@ -215,78 +169,3 @@ func (h *Handlers) HandleAccountPassword(w http.ResponseWriter, r *http.Request) logf("panel: user %d changed their password", u.ID) http.Redirect(w, r, "/account?done=password", http.StatusSeeOther) } - -// HandleAccountDMARC saves the user's default report address: SelfPost hosted, -// the account e-mail, another address, or none. -func (h *Handlers) HandleAccountDMARC(w http.ResponseWriter, r *http.Request) { - u, ok := h.accountUser(w, r) - if !ok { - return - } - f := formOf(u) - fail := func(status int, msg string) { - f.Err = msg - h.renderAccount(w, r, status, u, f) - } - if err := r.ParseForm(); err != nil { - fail(http.StatusBadRequest, "Invalid form submission.") - return - } - f.DMARCMode = strings.TrimSpace(r.PostFormValue("dmarc_default")) - f.DMARCAddress = strings.TrimSpace(r.PostFormValue("dmarc_default_address")) - switch f.DMARCMode { - case store.DMARCDefaultNone: - case store.DMARCDefaultHosted: - if h.dmarc == nil || !h.dmarc.Enabled() { - fail(http.StatusBadRequest, "SelfPost-hosted reports are not enabled on this server.") - return - } - case store.DMARCDefaultAccount: - if u.Email == "" { - fail(http.StatusBadRequest, "Your account has no e-mail yet. Save one under Profile first, or choose another address.") - return - } - case store.DMARCDefaultCustom: - if f.DMARCAddress == "" { - fail(http.StatusBadRequest, "Enter the report address or choose another option.") - return - } - if err := validate.Email(f.DMARCAddress); err != nil { - fail(http.StatusBadRequest, err.Error()) - return - } - default: - fail(http.StatusBadRequest, "Choose where your DMARC reports go.") - return - } - if err := h.store.SetDMARCDefault(u.ID, f.DMARCMode, f.DMARCAddress); err != nil { - logf("panel: account: set dmarc default of user %d: %v", u.ID, err) - fail(http.StatusInternalServerError, "Could not save the report address. Please check the logs and try again.") - return - } - h.resyncDMARC("default report address") - logf("panel: user %d set their default report address to %q", u.ID, f.DMARCMode) - http.Redirect(w, r, "/account?done=dmarc", http.StatusSeeOther) -} - -// resyncAfterEmailChange is what changing a user's account e-mail does for -// DMARC: the address is the report address of every domain that follows a user -// whose default is "my account e-mail", so the ingest allow-list is rebuilt. It -// is the one path both the Account page and the user form take. -func (h *Handlers) resyncAfterEmailChange(u store.User, newEmail string) { - if newEmail != u.Email && u.DMARCDefaultMode == store.DMARCDefaultAccount { - h.resyncDMARC("account e-mail") - } -} - -// resyncDMARC rebuilds the ingest allow-list after something that changes -// where a domain's reports go. A failure is logged, not shown: the setting is -// saved, and the next resync picks it up. -func (h *Handlers) resyncDMARC(why string) { - if h.dmarc == nil || !h.dmarc.Enabled() { - return - } - if err := h.dmarc.Resync(); err != nil { - logf("panel: dmarc resync after %s change: %v", why, err) - } -} diff --git a/internal/web/handlers/handlers_account_test.go b/internal/web/handlers/handlers_account_test.go index f266f02..a285305 100644 --- a/internal/web/handlers/handlers_account_test.go +++ b/internal/web/handlers/handlers_account_test.go @@ -12,30 +12,34 @@ import ( // Account is every user's own page: each part is its own form and its own // POST, and none of them touches what another saves. -func TestAccountPageShowsThreeForms(t *testing.T) { +func TestAccountPageShowsTwoForms(t *testing.T) { h, _ := settingsServer(t) out := getBody(t, h.HandleAccount, "/account") for _, want := range []string{ `action="/account/profile"`, `name="email"`, `action="/account/password"`, `name="current_password"`, - `action="/account/dmarc"`, `name="dmarc_default"`, } { if !strings.Contains(out, want) { t.Errorf("account page missing %q", want) } } - for _, gone := range []string{`name="send_log_retention_days"`, `value="hosted"`} { + // A domain's report address is not an Account setting any more. + for _, gone := range []string{`name="send_log_retention_days"`, `/account/dmarc`, `name="dmarc_default"`, `

DMARC reports

`, `Report authorization`} { if strings.Contains(out, gone) { - t.Errorf("account page shows %q (instance setting, or hosted reports with ingest off)", gone) + t.Errorf("account page shows %q", gone) } } } // The profile form saves the name and the e-mail without asking for the -// password and without changing the password or the DMARC default. +// password and without changing the password or any domain's report address. func TestAccountProfile(t *testing.T) { h, _ := settingsServer(t) before, _ := h.store.GetUser(globalPrincipal.ID) + d, _ := h.store.AddDomain("example.com", "mail") + if err := h.store.SetDomainDMARCAddress(d.ID, "reports@hub.example"); err != nil { + t.Fatal(err) + } rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, url.Values{"username": {"operator"}, "email": {"mix@example.org"}}) @@ -46,8 +50,11 @@ func TestAccountProfile(t *testing.T) { if u.Username != "operator" || u.Email != "mix@example.org" { t.Errorf("profile = %q / %q", u.Username, u.Email) } - if u.PasswordHash != before.PasswordHash || u.DMARCDefaultMode != store.DMARCDefaultNone { - t.Errorf("saving the profile changed the password or the DMARC default (%q)", u.DMARCDefaultMode) + if u.PasswordHash != before.PasswordHash { + t.Error("saving the profile changed the password") + } + if got, _ := h.store.GetDomain(d.ID); got.DMARCRua != "reports@hub.example" { + t.Errorf("saving the profile moved a domain's report address to %q", got.DMARCRua) } for _, bad := range []url.Values{ @@ -97,62 +104,6 @@ func TestAccountPassword(t *testing.T) { } } -// The default report address is the user's choice between four modes; the -// address field belongs to "another address" alone. -func TestAccountDMARCDefault(t *testing.T) { - h, _ := settingsServer(t) - save := func(form url.Values) (int, store.User) { - rec := postFormAs(h.HandleAccountDMARC, globalPrincipal, "/account/dmarc", nil, form) - u, _ := h.store.GetUser(globalPrincipal.ID) - return rec.Code, u - } - - // "My account e-mail" needs an e-mail. - if code, u := save(url.Values{"dmarc_default": {"account"}}); code != http.StatusBadRequest || u.DMARCDefaultMode != store.DMARCDefaultNone { - t.Errorf("account e-mail without an e-mail = %d, mode %q", code, u.DMARCDefaultMode) - } - // Hosted needs report ingest, which this server has off. - if code, u := save(url.Values{"dmarc_default": {"hosted"}}); code != http.StatusBadRequest || u.DMARCDefaultMode != store.DMARCDefaultNone { - t.Errorf("hosted with ingest off = %d, mode %q", code, u.DMARCDefaultMode) - } - for _, bad := range []url.Values{ - {"dmarc_default": {"custom"}}, - {"dmarc_default": {"custom"}, "dmarc_default_address": {"nope"}}, - {"dmarc_default": {"profile"}}, - {}, - } { - if code, _ := save(bad); code != http.StatusBadRequest { - t.Errorf("%v = %d, want 400", bad, code) - } - } - - code, u := save(url.Values{"dmarc_default": {"custom"}, "dmarc_default_address": {"dmarc@hub.example"}}) - if code != http.StatusSeeOther || u.DMARCDefaultMode != store.DMARCDefaultCustom || u.DMARCDefaultAddress != "dmarc@hub.example" { - t.Errorf("another address = %d, %q %q", code, u.DMARCDefaultMode, u.DMARCDefaultAddress) - } - // A domain that follows this user now resolves to it. - d, _ := h.store.AddDomain("example.com", "mail") - h.followCreator(d.ID, globalPrincipal.ID) - d, _ = h.store.GetDomain(d.ID) - if rua, err := h.domainReportAddress(d); err != nil || rua != "dmarc@hub.example" { - t.Errorf("a following domain reports to %q, %v", rua, err) - } - - if rec := postFormAs(h.HandleAccountProfile, globalPrincipal, "/account/profile", nil, url.Values{"username": {"admin"}, "email": {"mix@example.org"}}); rec.Code != http.StatusSeeOther { - t.Fatalf("save e-mail = %d", rec.Code) - } - code, u = save(url.Values{"dmarc_default": {"account"}, "dmarc_default_address": {"left-over@hub.example"}}) - if code != http.StatusSeeOther || u.DMARCDefaultMode != store.DMARCDefaultAccount || u.DMARCDefaultAddress != "" { - t.Errorf("my account e-mail = %d, %q %q", code, u.DMARCDefaultMode, u.DMARCDefaultAddress) - } - if rua, _ := h.domainReportAddress(d); rua != "mix@example.org" { - t.Errorf("the following domain reports to %q, want the account e-mail", rua) - } - if code, u = save(url.Values{"dmarc_default": {"none"}}); code != http.StatusSeeOther || u.DMARCDefaultMode != store.DMARCDefaultNone { - t.Errorf("none = %d, %q", code, u.DMARCDefaultMode) - } -} - // A domain administrator has the same Account page; it is not a Server page. func TestAccountIsOpenToADomainAdministrator(t *testing.T) { h, _ := settingsServer(t) @@ -178,7 +129,7 @@ func TestAccountPageIsDrawnInTheShell(t *testing.T) { out := getBody(t, h.HandleAccount, "/account") for _, want := range []string{ `class="navbar is-primary"`, `Signed in as admin · global`, `

Account

`, - `id="dmarc"`, `href="/static/panel.css"`, + `href="/static/panel.css"`, } { if !strings.Contains(out, want) { t.Errorf("account page missing %q", want) @@ -222,7 +173,6 @@ func TestAccountResultsAreTheFlash(t *testing.T) { for done, want := range map[string]string{ "profile": "Profile saved.", "password": "Password changed.", - "dmarc": "Default report address saved.", } { out := getBody(t, h.HandleAccount, "/account?done="+done) if !strings.Contains(out, `notification is-success is-light`) || !strings.Contains(out, want) { @@ -247,22 +197,3 @@ func TestAccountPageForADomainAdministrator(t *testing.T) { } } } - -// The default address needs an authorization record when it is on another -// domain; with "no reports" nothing is asked of DNS and no record is drawn. The -// count of following domains is the user's own. -func TestAccountReportAuthorizationFollowsTheStoredDefault(t *testing.T) { - h, _ := settingsServer(t) - d, _ := h.store.AddDomain("example.com", "mail") - h.followCreator(d.ID, globalPrincipal.ID) - if _, err := h.store.AddDomain("other.example", "mail"); err != nil { - t.Fatal(err) - } - out := getBody(t, h.HandleAccount, "/account") - if strings.Contains(out, "Report authorization") { - t.Error("an authorization record for a default of no reports") - } - if !strings.Contains(out, "1 of 2 now") { - t.Errorf("the domains following the default are not counted:\n%s", out) - } -} diff --git a/internal/web/handlers/handlers_apps.go b/internal/web/handlers/handlers_apps.go index 0937dbb..c6efebd 100644 --- a/internal/web/handlers/handlers_apps.go +++ b/internal/web/handlers/handlers_apps.go @@ -71,12 +71,7 @@ func (h *Handlers) renderDomain(w http.ResponseWriter, r *http.Request, status i // What DNS actually publishes for the domain today, checked against the key // this server signs with. - reportEmail, err := h.domainReportAddress(d) - if err != nil { - logf("panel: domain %d: dmarc report address: %v", d.ID, err) - http.Error(w, "internal error", http.StatusInternalServerError) - return - } + reportEmail := d.DMARCRua dns, srv := h.domainDNS(d, record, reportEmail, false) records := []view.Record{ view.DKIMRecord(d.DKIMSelector, record.Name, record.Value, dnsCheck(dns.DKIM)), @@ -88,7 +83,7 @@ func (h *Handlers) renderDomain(w http.ResponseWriter, r *http.Request, status i } records = append(records, view.DMARCRecord(view.DMARCRecordInput{ Host: dnscheck.DMARCRecordName(d.Name), Value: dnscheck.DMARCExample(reportEmail), Check: dnsCheck(dns.DMARC), - Source: h.reportSource(d, reportEmail), SettingsHref: view.DomainHref(d.ID) + "/settings", ReportsHref: reports, + Source: h.reportSource(d), SettingsHref: view.DomainHref(d.ID) + "/settings", ReportsHref: reports, SameDomain: reportEmail != "" && strings.EqualFold(dnscheck.EmailDomain(reportEmail), d.Name) && !(h.cfg.DMARCEnabled && dmarc.IsHostedOnHostname(reportEmail, h.cfg.Hostname)), })) @@ -112,21 +107,14 @@ func dnsCheck(r dnscheck.Result) view.DNSCheck { return view.DNSCheck{Status: string(r.Status), Detail: r.Detail, Found: r.Records} } -// reportSource says in words where a domain's DMARC reports go: the default of -// the user it follows, the hosted address, an address of its own, or nowhere. -func (h *Handlers) reportSource(d store.Domain, reportEmail string) string { - switch { - case d.DMARCRuaUserID.Valid: - if def, err := h.store.GetDMARCDefault(d.DMARCRuaUserID.Int64); err == nil && def.Username != "" { - return def.Username + "'s default" - } - return "a user's default" - case reportEmail == "": - return "none" - case strings.EqualFold(reportEmail, h.hostedDMARCAddress(d.Name)): - return "SelfPost hosted" +// reportSource says in words where a domain's DMARC reports go, for the note +// of its DMARC record: the hosted address or the one typed for the domain, +// written out, or "no reports". +func (h *Handlers) reportSource(d store.Domain) string { + if d.DMARCRua == "" { + return "no reports" } - return "a custom address" + return d.DMARCRua } // outAppRow is one line of the Applications table of a domain. @@ -160,7 +148,8 @@ func (h *Handlers) outAppRow(d store.Domain, a store.Application, retention int) } return view.OutAppRow{ Login: a.Login, Senders: senders, Activity: view.FormatActivity(stats.Total, stats.PeakPerHour), Limits: limits, - Edit: fmt.Sprintf("%s/applications/%d", view.DomainHref(d.ID), a.ID), + Edit: fmt.Sprintf("%s/applications/%d", view.DomainHref(d.ID), a.ID), + Actions: view.AppRowActions(d.ID, a.ID, a.Login), }, nil } @@ -214,76 +203,40 @@ func (h *Handlers) renderDomainSettings(w http.ResponseWriter, r *http.Request, rate.Computed = intOrBlank(rl.MaxMessages) } - options, selected, custom, help := h.reportAddressForm(r, d) + options, selected, custom, mine := h.reportAddressForm(r, d) if sv.RuaMode != "" { selected, custom = sv.RuaMode, sv.RuaCustom } page := view.NewOutDomainSettings(h.shellMeta(r), d.ID, d.Name, d.AppCount, p.IsGlobal()). - WithReportAddress(options, selected, custom, help). + WithReportAddress(options, selected, custom, mine). WithExport(validate.MinSecretFilePasswordLen). WithRateLimit(rate). WithResult("", sv.Err) h.view.Render(w, status, "out-domain-settings", page) } -// reportAddressForm is the choices of the report-address select for a domain -// and the signed-in user. "inherit" makes the domain follow the default of the -// user who saves the form (HandleDomainDMARC), so what the option says depends -// on whom the domain follows now: the user themselves, someone else, or nobody. -// A domain that follows someone else also offers "keep" — that user's default, -// selected — so saving the form untouched does not take the domain over. -func (h *Handlers) reportAddressForm(r *http.Request, d store.Domain) (options []view.Option, selected, custom string, help view.Text) { - hosted := h.hostedDMARCAddress(d.Name) - me, _ := h.principal(r) - mine, _ := h.store.GetDMARCDefault(me.ID) - address := func(a string) string { - if a == "" { - return "no report address yet" - } - return a +// reportAddressForm is the report-address form of a domain: the three choices +// of the select, the one the stored address stands for, the address typed for +// the domain (only in the typed choice) and the button that fills it with the +// signed-in user's profile e-mail ("" when the profile has none). SelfPost +// hosted is offered only while hosted reports are enabled. +func (h *Handlers) reportAddressForm(r *http.Request, d store.Domain) (options []view.Option, selected, custom, mine string) { + options = append(options, view.Option{Value: ruaNone, Label: "No reports"}) + if hosted := h.hostedDMARCAddress(d.Name); hosted != "" { + options = append(options, view.Option{Value: ruaHosted, Label: "SelfPost hosted — " + hosted}) } - myAddr := address(mine.Resolve(hosted)) + options = append(options, view.Option{Value: ruaCustom, Label: "A specific address"}) - help = view.Rich("A default belongs to a user and is set under their ", view.Link("/account#dmarc", "Account"), - "; a domain follows one named user, so two people sharing a domain never pull it two ways. Changing this changes the DMARC record to publish.") - inherit := view.Option{Value: "inherit", Label: "My default — " + myAddr} - followsOther := d.DMARCRuaUserID.Valid && d.DMARCRuaUserID.Int64 != me.ID - switch { - case followsOther: - inherit.Label = "My default instead — " + myAddr + " (you are " + me.Username + ")" - keep := view.Option{Value: "keep", Label: "Another user's default"} - if other, err := h.store.GetDMARCDefault(d.DMARCRuaUserID.Int64); err == nil { - keep.Label = other.Username + "'s default — " + address(other.Resolve(hosted)) + selected = h.reportChoice(d) + if selected == ruaCustom { + custom = d.DMARCRua + } + if p, ok := h.principal(r); ok { + if u, err := h.store.GetUser(p.ID); err == nil { + mine = u.Email } - options = append(options, keep) - case d.DMARCRuaUserID.Valid: - inherit.Label = me.Username + "'s default — " + myAddr - options = append(options, inherit) - default: - options = append(options, inherit) } - if hosted != "" { - options = append(options, view.Option{Value: "hosted", Label: "SelfPost hosted (" + hosted + ")"}) - } - // The mockup's order: whose default is followed now, hosted, "mine instead". - if followsOther { - options = append(options, inherit) - } - options = append(options, view.Option{Value: "none", Label: "No aggregate reports"}, view.Option{Value: "custom", Label: "Custom address"}) - - switch { - case followsOther: - selected = "keep" - case d.DMARCRuaUserID.Valid: - selected = "inherit" - case d.DMARCRua == "": - selected = "none" - case hosted != "" && strings.EqualFold(d.DMARCRua, hosted): - selected = "hosted" - default: - selected, custom = "custom", d.DMARCRua - } - return options, selected, custom, help + return options, selected, custom, mine } // formatBound writes a multiplier bound the short way: "1.5", "5". @@ -320,13 +273,7 @@ func (h *Handlers) HandleDomainDNSRecheck(w http.ResponseWriter, r *http.Request http.Error(w, "internal error", http.StatusInternalServerError) return } - reportEmail, err := h.domainReportAddress(d) - if err != nil { - logf("panel: domain %d: dmarc report address: %v", d.ID, err) - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - h.domainDNS(d, record, reportEmail, true) + h.domainDNS(d, record, d.DMARCRua, true) http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?rechecked=1", d.ID), http.StatusSeeOther) } diff --git a/internal/web/handlers/handlers_apps_test.go b/internal/web/handlers/handlers_apps_test.go index 5ac94ac..6cac4e1 100644 --- a/internal/web/handlers/handlers_apps_test.go +++ b/internal/web/handlers/handlers_apps_test.go @@ -263,3 +263,42 @@ func TestRegeneratedPasswordIsNotCached(t *testing.T) { t.Error("the response does not show the new password") } } + +// New password and Delete are in the application's row of the domain page, for +// whoever may edit the application: each is a POST form aimed at the +// application's own handler and asks first. A domain administrator of another +// domain gets a 404 from the same targets. +func TestApplicationRowCarriesNewPasswordAndDelete(t *testing.T) { + s := newAppStand(t) + a, _, err := s.h.apps.CreateWithSettings(s.d.ID, "prod-server", app.Settings{Mode: store.AddressModeWildcard}, nil) + if err != nil { + t.Fatal(err) + } + row := `Edit` + + `
` + + `
` + + has(t, "domain page", pageOf(t, s.h.HandleDomainDetail, "/outbound/domains/1"), row, + `New password and Delete ask for confirmation; the old password stops working at once.`) + ops := domainAdmin(t, s.h.store, "ops", s.d.ID) + has(t, "domain page for its administrator", send(s.h.HandleDomainDetail, &ops, "GET", "/outbound/domains/1", s.paths, nil).Body.String(), row) + + // The row's targets are the application page's own handlers. + paths := map[string]string{"id": idStr(s.d.ID), "aid": idStr(a.ID)} + if rec := postFormAs(s.h.HandleRegenPassword, ops, "/outbound/domains/1/applications/1/password", paths, url.Values{}); rec.Code != http.StatusOK || + rec.Header().Get("Cache-Control") != "no-store" || !strings.Contains(rec.Body.String(), s.sasl["prod-server"]) { + t.Errorf("the row's New password as a domain administrator = %d, Cache-Control %q", rec.Code, rec.Header().Get("Cache-Control")) + } + stranger := domainAdmin(t, s.h.store, "stranger") + for name, h := range map[string]http.HandlerFunc{"password": s.h.HandleRegenPassword, "delete": s.h.HandleDeleteApplication} { + if rec := postFormAs(h, stranger, "/outbound/domains/1/applications/1/"+name, paths, url.Values{}); rec.Code != http.StatusNotFound { + t.Errorf("%s as the administrator of another domain = %d, want 404", name, rec.Code) + } + } + if rec := postFormAs(s.h.HandleDeleteApplication, ops, "/outbound/domains/1/applications/1/delete", paths, url.Values{}); rec.Code != http.StatusSeeOther { + t.Fatalf("the row's Delete as a domain administrator = %d", rec.Code) + } + if _, err := s.h.store.GetApplicationByLogin("prod-server"); err == nil { + t.Error("the application is still there after Delete") + } +} diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index b706361..98e15a4 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -273,12 +273,8 @@ func (h *Handlers) HandleImportDomain(w http.ResponseWriter, r *http.Request) { h.renderBackupPage(w, r, status, msg) return } - // An export without an address of its own followed someone's default on the - // server it came from; here it follows whoever imports it. - if exp.DMARCRua == nil { - if p, ok := h.principal(r); ok { - h.followCreator(d.ID, p.ID) - } + if exp.DMARCRua != "" { + h.resyncDMARC("domain import") } http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?imported=1", d.ID), http.StatusSeeOther) } diff --git a/internal/web/handlers/handlers_dmarc.go b/internal/web/handlers/handlers_dmarc.go index cf16340..60794bd 100644 --- a/internal/web/handlers/handlers_dmarc.go +++ b/internal/web/handlers/handlers_dmarc.go @@ -6,10 +6,16 @@ import ( "strings" "github.com/mixeme/selfpost/internal/dmarc" + "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" + "github.com/mixeme/selfpost/internal/web/view" ) -// HandleDomainDMARC saves per-domain DMARC rua= settings. +// HandleDomainDMARC saves where a domain's aggregate reports (rua=) go. The +// form offers exactly three choices: "none", "hosted" (the address SelfPost +// derives for the domain, offered only while hosted reports are enabled) and +// "custom" (an address typed for the domain). What is stored is the address +// alone; "" is no reports. func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { w.Header().Set("Allow", "POST") @@ -25,9 +31,6 @@ func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { return } - // "inherit" makes the domain follow the default of the user who chose it; - // every other mode gives the domain an address of its own ("" = none). - follow := false rua := "" mode := strings.TrimSpace(r.PostFormValue("dmarc_rua_mode")) // A refusal shows the form again with the choice that was made. @@ -35,30 +38,19 @@ func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { h.renderDomainSettings(w, r, status, d, settingsView{Err: msg, RuaMode: mode, RuaCustom: strings.TrimSpace(r.PostFormValue("dmarc_rua_email"))}) } switch mode { - case "keep": - // The domain follows another user's default and the form left it so. - // Anything else under this value is a stale or forged form. - if p, ok := h.principal(r); !ok || !d.DMARCRuaUserID.Valid || d.DMARCRuaUserID.Int64 == p.ID { - refuse(http.StatusBadRequest, "Choose how aggregate reports are addressed for this domain.") - return - } - http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?dmarc=1", d.ID), http.StatusSeeOther) - return - case "inherit": - follow = true - case "none": - case "custom": + case ruaNone: + case ruaCustom: email := strings.TrimSpace(r.PostFormValue("dmarc_rua_email")) if err := validate.Email(email); err != nil { refuse(http.StatusBadRequest, err.Error()) return } if email == "" { - refuse(http.StatusBadRequest, "Enter a custom report address or choose another mode.") + refuse(http.StatusBadRequest, "Enter the report address or choose another option.") return } rua = email - case "hosted": + case ruaHosted: if h.dmarc == nil || !h.dmarc.Enabled() { refuse(http.StatusBadRequest, "SelfPost-hosted reports are not enabled on this server.") return @@ -69,22 +61,46 @@ func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { return } - var err error - if p, ok := h.principal(r); follow && ok { - err = h.store.SetDomainDMARCUser(d.ID, p.ID) - } else { - err = h.store.SetDomainDMARCAddress(d.ID, rua) - } - if err != nil { + if err := h.store.SetDomainDMARCAddress(d.ID, rua); err != nil { logf("panel: domain %d: save dmarc rua: %v", d.ID, err) refuse(http.StatusInternalServerError, "Could not save DMARC settings. Please check the logs and try again.") return } - if h.dmarc != nil && h.dmarc.Enabled() { - if err := h.dmarc.Resync(); err != nil { - logf("panel: domain %d: dmarc resync: %v", d.ID, err) - } - } + h.resyncDMARC("domain report address") h.dns.Forget(d.Name) http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?dmarc=1", d.ID), http.StatusSeeOther) } + +// The three choices of a domain's report address, the values of the select. +const ( + ruaNone = view.ReportNone + ruaHosted = view.ReportHosted + ruaCustom = view.ReportCustom +) + +// reportChoice says which of the three choices a domain's stored address is: +// none for "", hosted for the address SelfPost derives for the domain (only +// while hosted reports are enabled — otherwise it is just an address), custom +// for anything else. +func (h *Handlers) reportChoice(d store.Domain) string { + hosted := h.hostedDMARCAddress(d.Name) + switch { + case d.DMARCRua == "": + return ruaNone + case hosted != "" && strings.EqualFold(d.DMARCRua, hosted): + return ruaHosted + } + return ruaCustom +} + +// resyncDMARC rebuilds the ingest allow-list after something that changes +// where a domain's reports go. A failure is logged, not shown: the setting is +// saved, and the next resync picks it up. +func (h *Handlers) resyncDMARC(why string) { + if h.dmarc == nil || !h.dmarc.Enabled() { + return + } + if err := h.dmarc.Resync(); err != nil { + logf("panel: dmarc resync after %s change: %v", why, err) + } +} diff --git a/internal/web/handlers/handlers_dmarc_test.go b/internal/web/handlers/handlers_dmarc_test.go index 75018d4..7c10c10 100644 --- a/internal/web/handlers/handlers_dmarc_test.go +++ b/internal/web/handlers/handlers_dmarc_test.go @@ -4,10 +4,13 @@ import ( "net/http" "net/http/httptest" "net/url" + "regexp" "strings" "testing" + "github.com/mixeme/selfpost/internal/dmarc" "github.com/mixeme/selfpost/internal/domain" + "github.com/mixeme/selfpost/internal/postfix" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/auth" ) @@ -24,78 +27,269 @@ func postFormAs(h http.HandlerFunc, p auth.Principal, target string, pathValues return rec } -// The domain form's "Same as Settings" is, on the 2.0 schema, a domain that -// follows the default of the user who chose it; every other choice gives the -// domain an address of its own and stops it following anyone. -func TestDomainDMARCFollowsTheUserWhoChoseInherit(t *testing.T) { - h, _ := settingsServer(t) - h.domains = domain.NewService(h.store, nil, nil, "selfpost") - d, err := h.store.AddDomain("example.com", "selfpost") - if err != nil { - t.Fatalf("AddDomain: %v", err) - } - admin, err := h.store.GetUserByUsername("admin") - if err != nil { - t.Fatalf("GetUserByUsername: %v", err) - } - p := auth.Principal{ID: admin.ID, Username: "admin", Role: auth.RoleGlobal} - if err := h.store.UpdateUser(admin.ID, "admin", admin.PasswordHash, "mix@example.org"); err != nil { - t.Fatalf("UpdateUser: %v", err) - } - if err := h.store.SetDMARCDefault(admin.ID, store.DMARCDefaultAccount, ""); err != nil { - t.Fatalf("SetDMARCDefault: %v", err) - } - save := func(values url.Values) store.Domain { - t.Helper() - rec := postFormAs(h.HandleDomainDMARC, p, "/outbound/domains/1/settings/reports", map[string]string{"id": "1"}, values) - if rec.Code != http.StatusSeeOther { - t.Fatalf("POST %v = %d, want 303:\n%s", values, rec.Code, rec.Body.String()) - } - got, err := h.store.GetDomain(d.ID) - if err != nil { - t.Fatalf("GetDomain: %v", err) - } - return got - } - - got := save(url.Values{"dmarc_rua_mode": {"inherit"}}) - if !got.DMARCRuaUserID.Valid || got.DMARCRuaUserID.Int64 != admin.ID { - t.Fatalf("inherit: the domain follows %v, want user %d", got.DMARCRuaUserID, admin.ID) - } - if rua, err := h.domainReportAddress(got); err != nil || rua != "mix@example.org" { - t.Fatalf("inherit resolves to %q, %v; want the user's account e-mail", rua, err) - } - - got = save(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"dmarc@hub.example"}}) - if got.DMARCRuaUserID.Valid || got.DMARCRua != "dmarc@hub.example" { - t.Fatalf("custom: %q / follows %v", got.DMARCRua, got.DMARCRuaUserID) - } - - got = save(url.Values{"dmarc_rua_mode": {"none"}}) - if got.DMARCRuaUserID.Valid || got.DMARCRua != "" { - t.Fatalf("none: %q / follows %v", got.DMARCRua, got.DMARCRuaUserID) - } +// dmarcRecordingMaps stands in for Postfix: the allow-lists a resync would write. +type dmarcRecordingMaps struct{ got []postfix.DMARCMapsConfig } +func (m *dmarcRecordingMaps) RebuildDMARCMaps(cfg postfix.DMARCMapsConfig) error { + m.got = append(m.got, cfg) + return nil } -// A new domain takes its report address from whoever created it. -func TestFollowCreator(t *testing.T) { - h, _ := settingsServer(t) - d, err := h.store.AddDomain("example.com", "selfpost") +// hostedStand is an app stand whose server receives hosted DMARC reports. +func hostedStand(t *testing.T) (appStand, *dmarcRecordingMaps) { + t.Helper() + s := newAppStand(t) + maps := &dmarcRecordingMaps{} + s.h.cfg.DMARCEnabled, s.h.cfg.Hostname = true, "mail.example.org" + s.h.dmarc = dmarc.NewService(s.h.store, maps, "mail.example.org", true) + return s, maps +} + +const hostedForExample = "dmarc-reports+example.org@mail.example.org" + +// hostedEscaped is that address as html/template writes it into a page. +const hostedEscaped = "dmarc-reports+example.org@mail.example.org" + +// saveReports posts the report-address form of domain 1 as the global +// administrator. +func (s appStand) saveReports(form url.Values) *httptest.ResponseRecorder { + return postFormAs(s.h.HandleDomainDMARC, globalPrincipal, "/outbound/domains/1/settings/reports", s.paths, form) +} + +func (s appStand) reportAddress(t *testing.T) string { + t.Helper() + d, err := s.h.store.GetDomain(s.d.ID) if err != nil { - t.Fatalf("AddDomain: %v", err) + t.Fatal(err) } - h.followCreator(d.ID, globalPrincipal.ID) - got, err := h.store.GetDomain(d.ID) - if err != nil { - t.Fatalf("GetDomain: %v", err) + return d.DMARCRua +} + +func contains(list []string, want string) bool { + for _, s := range list { + if s == want { + return true + } } - if !got.DMARCRuaUserID.Valid || got.DMARCRuaUserID.Int64 != globalPrincipal.ID { - t.Fatalf("the domain follows %v, want its creator (user %d)", got.DMARCRuaUserID, globalPrincipal.ID) + return false +} + +// A new domain has no report address: its settings show "No reports" selected, +// and its DMARC record says so. +func TestNewDomainHasNoReports(t *testing.T) { + s, _ := hostedStand(t) + // A second OpenDKIM that reloads nothing: the stand's has no supervisor. + odk := domain.NewOpenDKIM(t.TempDir()) + odk.SetReloadHook(func() error { return nil }) + if _, err := odk.EnsureKey("example.org", "mail"); err != nil { + t.Fatal(err) } - // The creator's default is "none" until they choose one, so the new - // domain publishes a policy-only record — as a fresh 1.x install did. - if rua, err := h.domainReportAddress(got); err != nil || rua != "" { - t.Fatalf("resolved %q, %v; want no report address yet", rua, err) + s.h.domains = domain.NewService(s.h.store, odk, s.h.apps, "mail") + rec := postFormAs(s.h.HandleAddDomain, globalPrincipal, "/outbound/domains", nil, url.Values{"name": {"new.example"}}) + if rec.Code != http.StatusSeeOther { + t.Fatalf("adding a domain = %d:\n%s", rec.Code, rec.Body.String()) + } + domains, _ := s.h.store.ListDomains() + var added store.Domain + for _, d := range domains { + if d.Name == "new.example" { + added = d + } + } + if added.ID == 0 || added.DMARCRua != "" { + t.Fatalf("a new domain reports to %q (%+v), want no reports", added.DMARCRua, added) + } + body := send(s.h.HandleDomainSettings, &globalPrincipal, "GET", "/outbound/domains/2/settings", map[string]string{"id": idStr(added.ID)}, nil).Body.String() + has(t, "settings of a new domain", body, ``) + lacks(t, "settings of a new domain", body, ``, + ``, + ``) + + if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"hosted"}}); rec.Code != http.StatusSeeOther { + t.Fatalf("hosted = %d:\n%s", rec.Code, rec.Body.String()) + } + if got := s.reportAddress(t); got != hostedForExample { + t.Fatalf("hosted stored %q, want %q", got, hostedForExample) + } + has(t, "settings after hosted", settings(), ``, ``) + + if err := s.h.store.SetDomainDMARCAddress(s.d.ID, "reports@hub.example"); err != nil { + t.Fatal(err) + } + for _, form := range []url.Values{ + {"dmarc_rua_mode": {"hosted"}}, + {"dmarc_rua_mode": {"inherit"}}, + {"dmarc_rua_mode": {"keep"}}, + {"dmarc_rua_mode": {"custom"}}, + {"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"not an address"}}, + {}, + } { + if rec := s.saveReports(form); rec.Code != http.StatusBadRequest { + t.Errorf("%v = %d, want 400", form, rec.Code) + } + if got := s.reportAddress(t); got != "reports@hub.example" { + t.Fatalf("%v changed the address to %q", form, got) + } + } + + // A refused address comes back in the field, escaped, with its choice. + rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {`x">@no`}}) + has(t, "refusal", rec.Body.String(), `