diff --git a/CHANGELOG.md b/CHANGELOG.md index c933d14..87e4a36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,15 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version panel from a stopped one. Seen on the e2e stand with the panel frozen, and now counted, once per message (audit 2026-10-12, M13). +- **Application logins are one name whatever their case.** Postfix compares + the SASL login with the sender map without regard to case, the panel did + not: `Ops` in one domain and `ops` in another were two applications to the + panel and one sender to Postfix, so a tenant could send with another + tenant's envelope sender. A new login is now stored in lower case, and + migration `0008` refuses two logins that differ only in case. A login + stored earlier, or imported from another instance, keeps its case, since + its client uses it (audit 2026-10-12, H4). + ### Added - **The delivery log measures its own guesses** (migration `0007`). A table diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index a157f6d..a0b9af7 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -642,7 +642,7 @@ has already run is never edited — the next file is added; the published ### Stage 2 — the tenant boundary and the registry (H4, M7, M9, M10) → in `2.5.0` (migration `0008`) -- [ ] **Logins lower-cased** at validation (owner, 2026-10-12: fold, do not +- [x] **Logins lower-cased** at validation (owner, 2026-10-12: fold, do not refuse) and `applications.login COLLATE NOCASE` (migration `0008` rebuilds the unique index). Before the migration a check that no two existing logins differ only in case (there is one diff --git a/docs/roadmap.md b/docs/roadmap.md index 435b3e8..1e8548f 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 14/61 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 15/61 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and diff --git a/internal/app/service.go b/internal/app/service.go index 81978df..f3b8bbe 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -55,6 +55,7 @@ func (s *Service) Get(id int64) (store.Application, error) { // the map rebuild fails, everything is rolled back so we never leave an // application the panel cannot fully account for. func (s *Service) Create(domainID int64, login, mode string, rawAddresses []string) (store.Application, string, error) { + login = foldLogin(login) addresses, err := s.validateForDomain(domainID, login, mode, rawAddresses) if err != nil { return store.Application{}, "", err @@ -313,7 +314,7 @@ func (s *Service) CreateWithSettings(domainID int64, login string, set Settings, } if err := s.applyAccess(a.ID, set, recalc); err != nil { _ = s.ClearRateLimit(a.ID) - s.rollbackCreate(a.ID, login) + s.rollbackCreate(a.ID, a.Login) // the login as stored, folded _ = s.Resync() return store.Application{}, "", err } diff --git a/internal/app/service_test.go b/internal/app/service_test.go index 7a5e69f..39aa4d7 100644 --- a/internal/app/service_test.go +++ b/internal/app/service_test.go @@ -113,6 +113,40 @@ func TestServiceCreateListValidatesDomain(t *testing.T) { } } +// A new login is stored in lower case, and the sender map names it so: to +// Postfix "Ops" and "ops" are one login (audit 2026-10-12, H4). A second +// application that differs only in case is refused, created or imported, +// whichever domain it is for. +func TestServiceCreateFoldsTheLogin(t *testing.T) { + svc, st, rec, maps := newServiceHarness(t) + a1 := addDomain(t, st, "a.example") + b := addDomain(t, st, "b.example") + + a, _, err := svc.Create(a1.ID, "Ops", store.AddressModeWildcard, nil) + if err != nil { + t.Fatal(err) + } + if a.Login != "ops" { + t.Fatalf("login stored as %q, want ops", a.Login) + } + if _, ok := rec.set["ops"]; !ok { + t.Fatalf("SASL account written as %v, want ops", rec.set) + } + if len(maps.last) != 1 || maps.last[0].Login != "ops" { + t.Fatalf("map bindings = %+v, want the login in lower case", maps.last) + } + + if _, _, err := svc.Create(b.ID, "OPS", store.AddressModeWildcard, nil); !errors.Is(err, store.ErrLoginExists) { + t.Fatalf("OPS in another domain = %v, want ErrLoginExists", err) + } + if err := svc.ImportApplication(b.ID, "oPs", store.AddressModeWildcard, nil, "imported-password"); !errors.Is(err, store.ErrLoginExists) { + t.Fatalf("an imported oPs = %v, want ErrLoginExists", err) + } + if _, ok := rec.set["OPS"]; ok { + t.Fatal("the refused OPS got a SASL account") + } +} + func TestServiceCreateDuplicateLogin(t *testing.T) { svc, st, _, _ := newServiceHarness(t) d := addDomain(t, st, "example.com") diff --git a/internal/app/validate.go b/internal/app/validate.go index 8c72090..1cb368a 100644 --- a/internal/app/validate.go +++ b/internal/app/validate.go @@ -24,6 +24,17 @@ func invalid(format string, args ...any) error { return &ValidationError{msg: fmt.Sprintf(format, args...)} } +// foldLogin is a new application's login as it is stored: in lower case +// (audit 2026-10-12, H4; owner: fold, do not refuse). Postfix compares the +// SASL login with the sender map's without regard to case, so two logins +// that differ only in case would be one sender to Postfix and two +// applications to the panel. The store refuses such a pair as well +// (idx_applications_login_nocase); a login imported from another instance is +// not folded, since its SASL account and its client use it as it is. +func foldLogin(login string) string { + return strings.ToLower(login) +} + // validateLogin enforces a strict server-side whitelist for the SASL login // (security.md). It intentionally excludes '@': the login is stored in sasldb2, // where '@' separates the user from the realm, so allowing it would change the diff --git a/internal/store/applications_test.go b/internal/store/applications_test.go index 5091e0c..d934102 100644 --- a/internal/store/applications_test.go +++ b/internal/store/applications_test.go @@ -38,6 +38,20 @@ func TestAddApplicationWildcard(t *testing.T) { } } +// Logins are unique without regard to case, whatever the caller passed +// (idx_applications_login_nocase, migration 0008). +func TestAddApplicationRefusesALoginDifferingOnlyInCase(t *testing.T) { + st := openTestStore(t) + a := addTestDomain(t, st, "a.example") + b := addTestDomain(t, st, "b.example") + if _, err := st.AddApplication(a.ID, "Ops", AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } + if _, err := st.AddApplication(b.ID, "ops", AddressModeWildcard, nil); !errors.Is(err, ErrLoginExists) { + t.Fatalf("ops beside Ops = %v, want ErrLoginExists", err) + } +} + func TestAddApplicationListStoresAddresses(t *testing.T) { st := openTestStore(t) d := addTestDomain(t, st, "example.com") diff --git a/internal/store/migrations/0008_tenant_boundary.sql b/internal/store/migrations/0008_tenant_boundary.sql new file mode 100644 index 0000000..336988e --- /dev/null +++ b/internal/store/migrations/0008_tenant_boundary.sql @@ -0,0 +1,11 @@ +-- The tenant boundary (audit 2026-10-12, stage 2). + +-- An application login is unique without regard to case. Postfix compares +-- the SASL login with the sender map's without regard to case, so "Ops" and +-- "ops" were two applications to the panel and one sender to Postfix: a +-- tenant could take another's envelope sender (audit H4). New logins are +-- folded to lower case by the panel; a login stored earlier keeps its case, +-- since its SASL account and its client use it. The column's own UNIQUE +-- (case-sensitive) stays: SQLite cannot change a column's collation without +-- rebuilding the table, and this index is the stricter rule. +CREATE UNIQUE INDEX idx_applications_login_nocase ON applications (login COLLATE NOCASE); diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 820cb64..6d9cfd8 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -104,7 +104,7 @@ func TestBaselineSchema(t *testing.T) { } rows.Close() wantIdx := []string{ - "idx_dmarc_report_records_report", "idx_dmarc_reports_domain", "idx_dmarc_reports_received", + "idx_applications_login_nocase", "idx_dmarc_report_records_report", "idx_dmarc_reports_domain", "idx_dmarc_reports_received", "idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at", "idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_lines_queue_id", "idx_send_log_queue_id", "idx_send_log_rejected", "idx_sessions_expires_at", @@ -115,8 +115,8 @@ func TestBaselineSchema(t *testing.T) { } var version, appID int64 - if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 7 { - t.Errorf("user_version = %d, %v; want 7", version, err) + if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 8 { + t.Errorf("user_version = %d, %v; want 8", version, err) } if err := st.db.QueryRow("PRAGMA application_id").Scan(&appID); err != nil || appID != applicationID { t.Errorf("application_id = %#x, %v; want %#x", appID, err, applicationID) @@ -192,8 +192,8 @@ func TestOpenMigratesA20Database(t *testing.T) { } defer st.Close() var version int - if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 7 { - t.Errorf("user_version = %d, %v; want 7", version, err) + if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 8 { + t.Errorf("user_version = %d, %v; want 8", version, err) } if ok, err := st.UserExists(); err != nil || !ok { t.Errorf("the user of the 2.0 database: exists = %v, %v", ok, err)