diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d5689d..436923e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version (username, e-mail), password, and the default DMARC report address (SelfPost hosted, my account e-mail, another address, none). Saving the profile no longer asks for the password; changing the password still does. +- panel-redesign (stage 1): the application form is one POST. Who the + application may send as, its client-IP allow-list and its rate limit + ("use the domain limit", manual or auto) are validated together and saved + together or not at all — also at creation, so an application can be added + with its limit already in place. The page that shows a generated password + is the response to its POST only and is sent with `Cache-Control: no-store`. ### Changed @@ -62,8 +68,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version old paths are removed, not redirected — see *Removed*. Polled fragments all answer `/fragment`. Everything under `/server/` is guarded for the global role where the routes are registered, in addition to each handler's - own check. The application routes and Server › Health follow in the next - two steps. + own check. An application is addressed under its domain + (`/outbound/domains/{id}/applications/{aid}`), and an application id that + does not belong to the domain in the path is a 404. - **Breaking — the database starts from zero.** The 1.x migration chain (`0001`–`0010`) is replaced by one baseline, `0001_init.sql`. A 1.x `/data/selfpost.db` is refused at start-up with an explicit error and left @@ -89,7 +96,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version `/deliveries`, `/deliveries/rows`, `/deliveries/{id}`, `/mail-queue`, `/mail-queue/body`, `/system-log`, `/system-log/body`, `/backup`, `/users` and everything under it, `/settings`, and the `/account` → `/settings` - redirect. + redirect; `POST /domains/{id}/applications` and `/applications/{aid}/mode`, + `/authips`, `/ratelimit`, `/ratelimit/recalc`, `/password`, `/delete` — the + three separate saves of an application are one form now. - The instance setting `dmarc_report_email` (a mirror of the global user's profile field) and the unused `rate_limits.allowed_ips` column. diff --git a/docs/plans/panel-redesign.md b/docs/plans/panel-redesign.md index 82e5c79..d555cb2 100644 --- a/docs/plans/panel-redesign.md +++ b/docs/plans/panel-redesign.md @@ -508,7 +508,7 @@ only when both are done. Every stage-2 step ends with the evidence of - [x] Schema from zero: single `0001_init.sql` baseline with `users.email`, the DMARC default fields, `user_inbound_domains`, the two `all_*` flags; store tests; `schema-migrations.md` rewritten — **Opus** - [x] Inbound delegation: `requireInboundDomain`, lists and log filtered in the query, two-list user form data, per-route 404 tests for both roles including another tenant's id — **Opus** - [x] Routes renamed per § Routes: new paths, one fragment name, old paths removed, subtree `requireGlobal()` for `/server/`; links in the old templates retargeted so the panel works on every commit — **Opus** -- [ ] The application form as one POST; `out-app-created` as the response with `Cache-Control: no-store` — **Opus** +- [x] The application form as one POST; `out-app-created` as the response with `Cache-Control: no-store` — **Opus** - [ ] Server › Health route and handler (tables and *Reload configuration* from `/status`); Overview handler reduced to verdicts — **Opus** **Stage 2 — pages, in groups** (each step: partial calls only, page off the ratchet list, evidence pairs) diff --git a/docs/roadmap.md b/docs/roadmap.md index 10e9616..bfc67b7 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -39,7 +39,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | panel-notifications | E-mail notifications about important events | candidate | — | — | | delivery-log-storage | Keep a message's `mail.log` lines with its send-log row | candidate | — | — | | password-reset | Password reset by e-mail | candidate | — | — | -| panel-redesign | Panel redesign (accepted mockups, design contract) | agreed | 13/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | +| panel-redesign | Panel redesign (accepted mockups, design contract) | agreed | 14/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows diff --git a/internal/app/sasl.go b/internal/app/sasl.go index 4639557..e029178 100644 --- a/internal/app/sasl.go +++ b/internal/app/sasl.go @@ -183,3 +183,11 @@ func dumpSASLDB(path string) ([]byte, error) { } return out, nil } + +// WithRunner replaces the command that runs saslpasswd2 and returns the same +// manager. It exists for tests outside this package that need an application +// service without the real binary; the panel never calls it. +func (s *SASLDB) WithRunner(run func(args []string, stdin []byte) error) *SASLDB { + s.run = run + return s +} diff --git a/internal/app/service.go b/internal/app/service.go index 43227f7..81978df 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -275,3 +275,117 @@ func (s *Service) validateForDomain(domainID int64, login, mode string, rawAddre } return parseAddresses(rawAddresses, d.Name) } + +// Limit is an application's level-2 rate-limit choice on the application form. +// An empty Mode means "use the domain limit": the application has no limit of +// its own. +type Limit struct { + Mode string // "", store.RateLimitModeManual or store.RateLimitModeAuto + MaxMessages int + WindowSeconds int + AutoMultiplier float64 +} + +// Settings is everything the application form holds besides the login: who the +// application may send as, which client IPs may use it, and its rate limit. +// The form is one POST, so the three are validated and saved together. +type Settings struct { + Mode string + Addresses []string // raw; list mode only + AuthIPRestrict bool + AuthAllowedIPs []string + Limit Limit +} + +// Recalc derives an auto limit's ceiling from send statistics. The numbers it +// needs (retention, the level-1 backstop) are the panel's configuration, so +// the caller supplies it. +type Recalc func(appID int64) error + +// CreateWithSettings adds an application with all of its settings in one step +// and returns the password to show once. If anything after the creation fails +// the application is removed again: the form either produces a complete +// application or none. +func (s *Service) CreateWithSettings(domainID int64, login string, set Settings, recalc Recalc) (store.Application, string, error) { + a, password, err := s.Create(domainID, login, set.Mode, set.Addresses) + if err != nil { + return store.Application{}, "", err + } + if err := s.applyAccess(a.ID, set, recalc); err != nil { + _ = s.ClearRateLimit(a.ID) + s.rollbackCreate(a.ID, login) + _ = s.Resync() + return store.Application{}, "", err + } + a, err = s.store.GetApplication(a.ID) + if err != nil { + return store.Application{}, "", err + } + return a, password, nil +} + +// SaveSettings replaces an application's settings in one step. The sender +// rules are validated before anything is written; if a later write fails, what +// was already changed is put back, so the application is left as it was rather +// than half-saved. The login and the password are untouched. +func (s *Service) SaveSettings(id int64, set Settings, recalc Recalc) error { + before, err := s.store.GetApplication(id) + if err != nil { + return err + } + addresses, err := s.validateForDomain(before.DomainID, before.Login, set.Mode, set.Addresses) + if err != nil { + return err + } + limitBefore, hadLimit, err := s.RateLimit(id) + if err != nil { + return err + } + restore := func() { + _ = s.store.UpdateApplicationMode(id, before.AddressMode, before.Addresses) + _ = s.store.UpdateApplicationAuthIPs(id, before.AuthIPRestrict, before.AuthAllowedIPs) + if hadLimit { + _ = s.SaveRateLimit(id, limitBefore) + } else { + _ = s.ClearRateLimit(id) + } + _ = s.Resync() + } + if err := s.store.UpdateApplicationMode(id, set.Mode, addresses); err != nil { + restore() + return err + } + if err := s.applyAccess(id, set, recalc); err != nil { + restore() + return err + } + if err := s.Resync(); err != nil { + restore() + return err + } + return nil +} + +// applyAccess writes the client-IP restriction and the rate limit. +func (s *Service) applyAccess(id int64, set Settings, recalc Recalc) error { + if err := s.UpdateAuthIPs(id, set.AuthIPRestrict, set.AuthAllowedIPs); err != nil { + return err + } + switch set.Limit.Mode { + case "": + return s.ClearRateLimit(id) + case store.RateLimitModeAuto: + rl := store.RateLimit{Mode: store.RateLimitModeAuto, AutoMultiplier: set.Limit.AutoMultiplier, WindowSeconds: set.Limit.WindowSeconds} + if err := s.SaveRateLimit(id, rl); err != nil { + return err + } + if recalc == nil { + return nil + } + return recalc(id) + default: + return s.SaveRateLimit(id, store.RateLimit{ + Mode: store.RateLimitModeManual, MaxMessages: set.Limit.MaxMessages, WindowSeconds: set.Limit.WindowSeconds, + }) + } +} diff --git a/internal/app/service_test.go b/internal/app/service_test.go index e598c76..7a5e69f 100644 --- a/internal/app/service_test.go +++ b/internal/app/service_test.go @@ -318,3 +318,175 @@ func TestServicePurgeDomainSASL(t *testing.T) { t.Errorf("deleted %d logins, want 2", len(rec.deleted)) } } + +// The application form is one POST: sender, client IPs and rate limit arrive +// together and are saved together. +func TestCreateWithSettings(t *testing.T) { + svc, st, rec, _ := newServiceHarness(t) + d := addDomain(t, st, "example.com") + recalced := 0 + + a, pw, err := svc.CreateWithSettings(d.ID, "prod", Settings{ + Mode: store.AddressModeList, Addresses: []string{"alerts@example.com"}, + AuthIPRestrict: true, AuthAllowedIPs: []string{"203.0.113.10"}, + Limit: Limit{Mode: store.RateLimitModeManual, MaxMessages: 200, WindowSeconds: 3600}, + }, func(int64) error { recalced++; return nil }) + if err != nil { + t.Fatalf("CreateWithSettings: %v", err) + } + if rec.set["prod"] != pw || pw == "" { + t.Errorf("sasl password %q != returned %q", rec.set["prod"], pw) + } + if a.AddressMode != store.AddressModeList || len(a.Addresses) != 1 || !a.AuthIPRestrict || len(a.AuthAllowedIPs) != 1 { + t.Errorf("application = %+v", a) + } + rl, ok, err := svc.RateLimit(a.ID) + if err != nil || !ok || rl.MaxMessages != 200 || rl.WindowSeconds != 3600 || rl.IsAuto() { + t.Errorf("rate limit = %+v, %v, %v", rl, ok, err) + } + if recalced != 0 { + t.Error("a manual limit was recalculated") + } + + // Auto: the ceiling comes from the caller's recalculation. + b, _, err := svc.CreateWithSettings(d.ID, "auto", Settings{ + Mode: store.AddressModeWildcard, Limit: Limit{Mode: store.RateLimitModeAuto, AutoMultiplier: 2.5, WindowSeconds: 3600}, + }, func(int64) error { recalced++; return nil }) + if err != nil { + t.Fatalf("CreateWithSettings auto: %v", err) + } + if rl, ok, _ := svc.RateLimit(b.ID); !ok || !rl.IsAuto() || rl.AutoMultiplier != 2.5 || recalced != 1 { + t.Errorf("auto limit = %+v, %v; recalculated %d time(s)", rl, ok, recalced) + } + + // No limit of its own: the domain limit applies. + c, _, err := svc.CreateWithSettings(d.ID, "plain", Settings{Mode: store.AddressModeWildcard}, nil) + if err != nil { + t.Fatalf("CreateWithSettings plain: %v", err) + } + if _, ok, _ := svc.RateLimit(c.ID); ok { + t.Error("an application created without a limit has one") + } +} + +// A failure after the application row exists must leave no application: not in +// the registry, not in sasldb2, and no orphaned limit. +func TestCreateWithSettingsIsAllOrNothing(t *testing.T) { + svc, st, rec, _ := newServiceHarness(t) + d := addDomain(t, st, "example.com") + + _, _, err := svc.CreateWithSettings(d.ID, "prod", Settings{ + Mode: store.AddressModeWildcard, AuthIPRestrict: true, AuthAllowedIPs: []string{"203.0.113.10"}, + Limit: Limit{Mode: store.RateLimitModeAuto, AutoMultiplier: 2, WindowSeconds: 3600}, + }, func(int64) error { return errors.New("no statistics") }) + if err == nil { + t.Fatal("CreateWithSettings succeeded although the recalculation failed") + } + if apps, _ := st.ListApplicationsByDomain(d.ID); len(apps) != 0 { + t.Errorf("%d application(s) left behind", len(apps)) + } + if _, ok := rec.set["prod"]; ok { + t.Error("the SASL account was left behind") + } + // The id is free again and carries nothing over. + a, _, err := svc.CreateWithSettings(d.ID, "prod", Settings{Mode: store.AddressModeWildcard}, nil) + if err != nil { + t.Fatalf("create again: %v", err) + } + if _, ok, _ := svc.RateLimit(a.ID); ok || a.AuthIPRestrict { + t.Errorf("the new application inherited settings of the failed one: %+v", a) + } + + // A sender rule that does not validate creates nothing either. + if _, _, err := svc.CreateWithSettings(d.ID, "bad", Settings{Mode: store.AddressModeList, Addresses: []string{"a@evil.com"}}, nil); err == nil { + t.Error("a cross-domain address was accepted") + } + if _, ok := rec.set["bad"]; ok { + t.Error("a SASL account was written for a refused application") + } +} + +func TestSaveSettings(t *testing.T) { + svc, st, rec, maps := newServiceHarness(t) + d := addDomain(t, st, "example.com") + a, pw, err := svc.CreateWithSettings(d.ID, "prod", Settings{ + Mode: store.AddressModeWildcard, Limit: Limit{Mode: store.RateLimitModeManual, MaxMessages: 100, WindowSeconds: 3600}, + }, nil) + if err != nil { + t.Fatalf("create: %v", err) + } + + err = svc.SaveSettings(a.ID, Settings{ + Mode: store.AddressModeList, Addresses: []string{"alerts@example.com", "noreply@example.com"}, + AuthIPRestrict: true, AuthAllowedIPs: []string{"203.0.113.10", "203.0.113.11"}, + }, nil) + if err != nil { + t.Fatalf("SaveSettings: %v", err) + } + got, _ := st.GetApplication(a.ID) + if got.AddressMode != store.AddressModeList || len(got.Addresses) != 2 || !got.AuthIPRestrict || len(got.AuthAllowedIPs) != 2 { + t.Errorf("application = %+v", got) + } + if _, ok, _ := svc.RateLimit(a.ID); ok { + t.Error("choosing the domain limit left the application's own limit in place") + } + if len(maps.last) != 2 { + t.Errorf("sender map has %d binding(s), want the two listed addresses", len(maps.last)) + } + if rec.set["prod"] != pw { + t.Error("saving the settings touched the password") + } +} + +// Nothing is half-saved: a sender rule that does not validate changes nothing, +// and a failure in a later write puts the earlier ones back. +func TestSaveSettingsIsAllOrNothing(t *testing.T) { + svc, st, _, maps := newServiceHarness(t) + d := addDomain(t, st, "example.com") + a, _, err := svc.CreateWithSettings(d.ID, "prod", Settings{ + Mode: store.AddressModeList, Addresses: []string{"alerts@example.com"}, + AuthIPRestrict: true, AuthAllowedIPs: []string{"203.0.113.10"}, + Limit: Limit{Mode: store.RateLimitModeManual, MaxMessages: 100, WindowSeconds: 3600}, + }, nil) + if err != nil { + t.Fatalf("create: %v", err) + } + unchanged := func(when string) { + t.Helper() + got, _ := st.GetApplication(a.ID) + rl, ok, _ := svc.RateLimit(a.ID) + if got.AddressMode != store.AddressModeList || len(got.Addresses) != 1 || got.Addresses[0] != "alerts@example.com" || + !got.AuthIPRestrict || len(got.AuthAllowedIPs) != 1 || !ok || rl.MaxMessages != 100 || rl.IsAuto() { + t.Errorf("%s: the application changed: %+v, limit %+v (%v)", when, got, rl, ok) + } + } + + change := Settings{ + Mode: store.AddressModeWildcard, AuthIPRestrict: false, + Limit: Limit{Mode: store.RateLimitModeAuto, AutoMultiplier: 3, WindowSeconds: 3600}, + } + bad := change + bad.Mode, bad.Addresses = store.AddressModeList, []string{"a@evil.com"} + if err := svc.SaveSettings(a.ID, bad, nil); err == nil { + t.Fatal("a cross-domain address was accepted") + } + unchanged("after a refused sender rule") + + if err := svc.SaveSettings(a.ID, change, func(int64) error { return errors.New("no statistics") }); err == nil { + t.Fatal("SaveSettings succeeded although the recalculation failed") + } + unchanged("after a failed recalculation") + + maps.failNext = true + if err := svc.SaveSettings(a.ID, change, func(int64) error { return nil }); err == nil { + t.Fatal("SaveSettings succeeded although the sender map could not be rebuilt") + } + unchanged("after a failed sender-map rebuild") + if len(maps.last) != 1 || maps.last[0].Address != "alerts@example.com" { + t.Errorf("the sender map was left at %+v", maps.last) + } + + if err := svc.SaveSettings(a.ID+100, change, nil); !errors.Is(err, store.ErrApplicationNotFound) { + t.Errorf("unknown application = %v, want ErrApplicationNotFound", err) + } +} diff --git a/internal/web/handlers/handlers_apps.go b/internal/web/handlers/handlers_apps.go index d05e343..7e64a6d 100644 --- a/internal/web/handlers/handlers_apps.go +++ b/internal/web/handlers/handlers_apps.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/mixeme/selfpost/internal/app" "github.com/mixeme/selfpost/internal/dmarc" "github.com/mixeme/selfpost/internal/dnscheck" "github.com/mixeme/selfpost/internal/domain" @@ -53,7 +54,10 @@ type appRateLimitView struct { AutoMultiplier string AutoUpdated string IsAuto bool - Stats sendStatsView + // RLMode is the form's limit choice: "domain" (no limit of its own), + // "manual" or "auto". + RLMode string + Stats sendStatsView } // sendStatsView is the template-facing send statistics block. @@ -122,6 +126,7 @@ func (h *Handlers) renderDomainDetail(w http.ResponseWriter, r *http.Request, st Mode: mode, AutoMultiplier: formatMultiplier(mult), IsAuto: ok && rl.IsAuto(), + RLMode: limitChoice(rl, ok), AutoUpdated: formatAutoUpdated(rl.AutoUpdatedAt), Stats: formatSendStats(appStats), }) @@ -293,6 +298,19 @@ func (h *Handlers) HandleDomainDNSRecheck(w http.ResponseWriter, r *http.Request http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?rechecked=1", d.ID), http.StatusSeeOther) } +// limitChoice names which of the form's three limit choices an application has: +// the domain limit (none of its own), manual, or auto. +func limitChoice(rl store.RateLimit, configured bool) string { + switch { + case !configured: + return "domain" + case rl.IsAuto(): + return store.RateLimitModeAuto + default: + return store.RateLimitModeManual + } +} + // intOrBlank renders a non-positive number as an empty string so an unset field // shows blank rather than "0". func intOrBlank(n int) string { @@ -336,12 +354,10 @@ func detailFlash(r *http.Request) string { switch { case r.URL.Query().Get("appdeleted") != "": return "Application deleted." - case r.URL.Query().Get("modeupdated") != "": - return "Application address mode updated." + case r.URL.Query().Get("appsaved") != "": + return "Application saved." case r.URL.Query().Get("ratelimit") != "": return "Rate limit updated." - case r.URL.Query().Get("authips") != "": - return "Client IP restriction updated." case r.URL.Query().Get("recalculated") != "": return "Auto rate limit recalculated." case r.URL.Query().Get("dmarc") != "": @@ -355,11 +371,79 @@ func detailFlash(r *http.Request) string { } } -// HandleAddApplication creates an application on a domain and renders the page -// back with the generated password shown once (product.md, security.md). Because the -// password cannot be recovered later, this deliberately renders inline rather -// than redirecting. -func (h *Handlers) HandleAddApplication(w http.ResponseWriter, r *http.Request) { +// The application form is ONE post (plan § Routes): who the application may +// send as, which client IPs may use it and its rate limit are parsed and +// validated together and saved together, or not at all (app.Service +// CreateWithSettings / SaveSettings). The same form, empty, adds an +// application; filled, it edits one. The login is given once, at creation. + +// parseApplicationForm reads the form's three parts. Every field is checked +// here or by the service before anything is written. +func (h *Handlers) parseApplicationForm(r *http.Request) (app.Settings, error) { + set := app.Settings{ + Mode: r.PostFormValue("mode"), + Addresses: splitAddresses(r.PostFormValue("addresses")), + } + restrict, ips, err := parseAppAuthIPsForm(r) + if err != nil { + return app.Settings{}, err + } + set.AuthIPRestrict, set.AuthAllowedIPs = restrict, ips + + switch mode := strings.TrimSpace(r.PostFormValue("rl_mode")); mode { + case "", "domain": + // Use the domain limit: the application has none of its own. + case store.RateLimitModeAuto: + mult, err := parseAutoMultiplier(r.PostFormValue("auto_multiplier")) + if err != nil { + return app.Settings{}, err + } + set.Limit = app.Limit{Mode: mode, AutoMultiplier: mult, WindowSeconds: h.l1Window()} + case store.RateLimitModeManual: + maxMessages, err := parsePositiveInt(r.PostFormValue("max_messages"), 0) + if err != nil || maxMessages <= 0 { + return app.Settings{}, fmt.Errorf("enter a message limit greater than zero, or use the domain limit") + } + if l1 := h.l1Messages(); maxMessages > l1 { + return app.Settings{}, fmt.Errorf("message limit cannot exceed the level-1 backstop (%d)", l1) + } + window, err := parsePositiveInt(r.PostFormValue("window_seconds"), defaultRateLimitWindowSeconds) + if err != nil || window <= 0 { + return app.Settings{}, fmt.Errorf("enter a time window greater than zero seconds") + } + set.Limit = app.Limit{Mode: mode, MaxMessages: maxMessages, WindowSeconds: window} + default: + return app.Settings{}, fmt.Errorf("choose the domain limit, manual or auto") + } + return set, nil +} + +func (h *Handlers) recalcApp(appID int64) error { + return h.recalcRateLimit(store.RateLimitScopeApp, appID) +} + +// noStore keeps a page that shows a secret out of every cache and out of the +// browser's history copy: the password on it is shown once. +func noStore(w http.ResponseWriter) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Pragma", "no-cache") +} + +// HandleApplicationNew shows the form that adds an application. Until the +// domain page is split (stage 2) the form is a card of that page. +func (h *Handlers) HandleApplicationNew(w http.ResponseWriter, r *http.Request) { + d, ok := h.lookupDomain(w, r) + if !ok { + return + } + h.renderDomainDetail(w, r, http.StatusOK, d, detailView{FormMode: store.AddressModeWildcard}) +} + +// HandleApplicationCreate creates an application with all of its settings and +// answers with the generated password, shown once (product.md, security.md). +// The password cannot be recovered later, so the page is the response to this +// POST — it has no GET path — and is never stored by the browser. +func (h *Handlers) HandleApplicationCreate(w http.ResponseWriter, r *http.Request) { d, ok := h.lookupDomain(w, r) if !ok { return @@ -370,16 +454,18 @@ func (h *Handlers) HandleAddApplication(w http.ResponseWriter, r *http.Request) return } login := strings.TrimSpace(r.PostFormValue("login")) - mode := r.PostFormValue("mode") - addrs := splitAddresses(r.PostFormValue("addresses")) - repopulate := detailView{ FormLogin: login, - FormMode: mode, + FormMode: r.PostFormValue("mode"), FormAddrs: r.PostFormValue("addresses"), } - - a, password, err := h.apps.Create(d.ID, login, mode, addrs) + set, err := h.parseApplicationForm(r) + if err != nil { + repopulate.FormErr = err.Error() + h.renderDomainDetail(w, r, http.StatusBadRequest, d, repopulate) + return + } + a, password, err := h.apps.CreateWithSettings(d.ID, login, set, h.recalcApp) if err != nil { repopulate.FormErr = applicationErrorMessage(err) status := http.StatusBadRequest @@ -389,58 +475,71 @@ func (h *Handlers) HandleAddApplication(w http.ResponseWriter, r *http.Request) h.renderDomainDetail(w, r, status, d, repopulate) return } + noStore(w) h.renderDomainDetail(w, r, http.StatusCreated, d, detailView{ FormMode: store.AddressModeWildcard, NewCred: &newCred{Login: a.Login, Password: password}, }) } -// HandleUpdateAppMode switches an application's address mode / list (product.md). -func (h *Handlers) HandleUpdateAppMode(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) +// HandleApplicationEdit shows an application's form. Until the domain page is +// split (stage 2) that is the application's Edit panel on the domain page. +func (h *Handlers) HandleApplicationEdit(w http.ResponseWriter, r *http.Request) { + _, d, ok := h.lookupDomainApplication(w, r) if !ok { return } - if err := r.ParseForm(); err != nil { - http.Error(w, "invalid form", http.StatusBadRequest) - return - } - mode := r.PostFormValue("mode") - addrs := splitAddresses(r.PostFormValue("addresses")) + h.renderDomainDetail(w, r, http.StatusOK, d, detailView{FormMode: store.AddressModeWildcard}) +} - if err := h.apps.UpdateMode(a.ID, mode, addrs); err != nil { - d, derr := h.domains.Get(a.DomainID) - if derr != nil { - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - h.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{ - FormErr: fmt.Sprintf("Could not update %s: %s", a.Login, applicationErrorMessage(err)), - FormMode: store.AddressModeWildcard, - }) +// HandleApplicationSave saves an application's form: sender, client IPs and +// rate limit in one step. +func (h *Handlers) HandleApplicationSave(w http.ResponseWriter, r *http.Request) { + a, d, ok := h.lookupDomainApplication(w, r) + if !ok { return } - http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?modeupdated=1", a.DomainID), http.StatusSeeOther) + fail := func(msg string) { + h.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{ + FormMode: store.AddressModeWildcard, + RateLimitErr: fmt.Sprintf("%s: %s", a.Login, msg), + }) + } + if err := r.ParseForm(); err != nil { + fail("invalid form submission") + return + } + // The login is the SASL account's name and is not renamed by this form. + if login := strings.TrimSpace(r.PostFormValue("login")); login != "" && login != a.Login { + fail("the login of an application cannot be changed; add a new application instead") + return + } + set, err := h.parseApplicationForm(r) + if err != nil { + fail(err.Error()) + return + } + if err := h.apps.SaveSettings(a.ID, set, h.recalcApp); err != nil { + fail(applicationErrorMessage(err)) + return + } + http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?appsaved=1", d.ID), http.StatusSeeOther) } // HandleRegenPassword issues a new password for an application and shows it once -// (product.md, security.md). Rendered inline, like creation, so the password is visible. +// (product.md, security.md): the response to the POST, never cached. func (h *Handlers) HandleRegenPassword(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) + a, d, ok := h.lookupDomainApplication(w, r) if !ok { return } - d, err := h.domains.Get(a.DomainID) - if err != nil { - http.Error(w, "internal error", http.StatusInternalServerError) - return - } password, err := h.apps.RegeneratePassword(a.ID) if err != nil { logf("panel: regenerate password for application %d: %v", a.ID, err) http.Error(w, "internal error", http.StatusInternalServerError) return } + noStore(w) h.renderDomainDetail(w, r, http.StatusOK, d, detailView{ FormMode: store.AddressModeWildcard, NewCred: &newCred{Login: a.Login, Password: password}, @@ -450,7 +549,7 @@ func (h *Handlers) HandleRegenPassword(w http.ResponseWriter, r *http.Request) { // HandleDeleteApplication removes an application and returns to its domain page // (product.md). func (h *Handlers) HandleDeleteApplication(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) + a, _, ok := h.lookupDomainApplication(w, r) if !ok { return } @@ -462,6 +561,27 @@ func (h *Handlers) HandleDeleteApplication(w http.ResponseWriter, r *http.Reques http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?appdeleted=1", a.DomainID), http.StatusSeeOther) } +// lookupDomainApplication resolves /outbound/domains/{id}/applications/{aid} +// to the application and its domain. The application must belong to the domain +// named in the path: an id from another domain is answered 404 like a missing +// one, whoever asks, so a URL can never address an application through a +// domain it is not in. +func (h *Handlers) lookupDomainApplication(w http.ResponseWriter, r *http.Request) (store.Application, store.Domain, bool) { + d, ok := h.lookupDomain(w, r) + if !ok { + return store.Application{}, store.Domain{}, false + } + a, ok := h.lookupApplication(w, r) + if !ok { + return store.Application{}, store.Domain{}, false + } + if a.DomainID != d.ID { + http.NotFound(w, r) + return store.Application{}, store.Domain{}, false + } + return a, d, true +} + // lookupApplication resolves the {aid} path value to an application, writing a // 404 for a bad id or missing application. func (h *Handlers) lookupApplication(w http.ResponseWriter, r *http.Request) (store.Application, bool) { diff --git a/internal/web/handlers/handlers_apps_test.go b/internal/web/handlers/handlers_apps_test.go new file mode 100644 index 0000000..45c92c4 --- /dev/null +++ b/internal/web/handlers/handlers_apps_test.go @@ -0,0 +1,258 @@ +package handlers + +import ( + "net/http" + "net/url" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/app" + "github.com/mixeme/selfpost/internal/dnscheck" + "github.com/mixeme/selfpost/internal/domain" + "github.com/mixeme/selfpost/internal/postfix" + "github.com/mixeme/selfpost/internal/store" +) + +// offlineDNS answers every check from a resolver nobody listens on. It is +// shared, so the one slow failure is paid once and cached for the other tests. +var offlineDNS = dnscheck.New([]string{"127.0.0.1:9"}) + +type nopSenderMaps struct{} + +func (nopSenderMaps) RebuildSenderLoginMaps([]postfix.Binding) error { return nil } + +// appStand is a panel with one sending domain and a working application +// service whose SASL writes are recorded instead of run. +type appStand struct { + h *Handlers + d store.Domain + sasl map[string]string + paths map[string]string +} + +func newAppStand(t *testing.T) appStand { + t.Helper() + h, _ := settingsServer(t) + s := appStand{h: h, sasl: map[string]string{}} + sasl := app.NewSASLDB("/data/sasl/sasldb2", "mail.example.org").WithRunner(func(args []string, stdin []byte) error { + login := args[len(args)-1] + for _, a := range args { + if a == "-d" { + delete(s.sasl, login) + return nil + } + } + s.sasl[login] = string(stdin) + return nil + }) + h.apps = app.NewService(h.store, sasl, nopSenderMaps{}) + // The domain page is rendered with the domain's DKIM record and its DNS + // state: a real key in a temporary directory, and a resolver nobody + // listens on, so every check answers at once without leaving the machine. + odk := domain.NewOpenDKIM(t.TempDir()) + if _, err := odk.EnsureKey("example.org", "mail"); err != nil { + t.Fatalf("EnsureKey: %v", err) + } + h.domains = domain.NewService(h.store, odk, h.apps, "mail") + h.dns = offlineDNS + h.cfg.RateLimitMessagesPerIP, h.cfg.RateLimitWindowSeconds = 600, 3600 + var err error + if s.d, err = h.store.AddDomain("example.org", "mail"); err != nil { + t.Fatal(err) + } + s.paths = map[string]string{"id": idStr(s.d.ID)} + return s +} + +func (s appStand) app(t *testing.T, login string) (store.Application, store.RateLimit, bool) { + t.Helper() + a, err := s.h.store.GetApplicationByLogin(login) + if err != nil { + return store.Application{}, store.RateLimit{}, false + } + rl, ok, err := s.h.store.GetRateLimit(store.RateLimitScopeApp, a.ID) + if err != nil { + t.Fatal(err) + } + return a, rl, ok +} + +func TestParseApplicationForm(t *testing.T) { + h := &Handlers{cfg: Config{RateLimitMessagesPerIP: 600, RateLimitWindowSeconds: 3600}} + parse := func(v url.Values) (app.Settings, error) { + r, _ := http.NewRequest(http.MethodPost, "/", strings.NewReader(v.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return h.parseApplicationForm(r) + } + + set, err := parse(url.Values{ + "mode": {"list"}, "addresses": {"a@example.org\nb@example.org"}, + "auth_ip_restrict": {"1"}, "auth_allowed_ips": {"203.0.113.10, 203.0.113.11"}, + "rl_mode": {"manual"}, "max_messages": {"200"}, "window_seconds": {"1800"}, + }) + if err != nil { + t.Fatalf("parse: %v", err) + } + if set.Mode != "list" || len(set.Addresses) != 2 || !set.AuthIPRestrict || len(set.AuthAllowedIPs) != 2 || + set.Limit.Mode != store.RateLimitModeManual || set.Limit.MaxMessages != 200 || set.Limit.WindowSeconds != 1800 { + t.Errorf("settings = %+v", set) + } + + // The limit choice: nothing or "domain" means no limit of its own; auto + // takes the level-1 window and a default multiplier. + for _, v := range []url.Values{{"mode": {"wildcard"}}, {"mode": {"wildcard"}, "rl_mode": {"domain"}, "max_messages": {"50"}}} { + if set, err := parse(v); err != nil || set.Limit.Mode != "" || set.Limit.MaxMessages != 0 { + t.Errorf("%v = %+v, %v; want the domain limit", v, set.Limit, err) + } + } + if set, err := parse(url.Values{"rl_mode": {"auto"}}); err != nil || set.Limit.Mode != store.RateLimitModeAuto || + set.Limit.AutoMultiplier != store.DefaultAutoMultiplier || set.Limit.WindowSeconds != 3600 { + t.Errorf("auto = %+v, %v", set.Limit, err) + } + + for name, v := range map[string]url.Values{ + "manual without a number": {"rl_mode": {"manual"}}, + "manual above level 1": {"rl_mode": {"manual"}, "max_messages": {"601"}}, + "manual with a zero window": {"rl_mode": {"manual"}, "max_messages": {"10"}, "window_seconds": {"0"}}, + "an unknown limit mode": {"rl_mode": {"always"}}, + "a multiplier out of range": {"rl_mode": {"auto"}, "auto_multiplier": {"99"}}, + "the allow-list without IPs": {"auth_ip_restrict": {"1"}}, + "something that is not an IP": {"auth_ip_restrict": {"1"}, "auth_allowed_ips": {"not-an-ip"}}, + } { + if _, err := parse(v); err == nil { + t.Errorf("%s was accepted", name) + } + } +} + +// One POST creates the application with its sender rule, client IPs and limit, +// and the response is the page with the password — shown once, never cached. +func TestApplicationCreateIsOnePost(t *testing.T) { + s := newAppStand(t) + rec := postFormAs(s.h.HandleApplicationCreate, globalPrincipal, "/outbound/domains/1/applications/new", s.paths, url.Values{ + "login": {"prod-server"}, "mode": {"list"}, "addresses": {"alerts@example.org"}, + "auth_ip_restrict": {"1"}, "auth_allowed_ips": {"203.0.113.10"}, + "rl_mode": {"manual"}, "max_messages": {"200"}, "window_seconds": {"3600"}, + }) + if rec.Code != http.StatusCreated { + t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String()) + } + if got := rec.Header().Get("Cache-Control"); got != "no-store" { + t.Errorf("Cache-Control = %q; the page shows a password once and must not be stored", got) + } + password := s.sasl["prod-server"] + if password == "" || !strings.Contains(rec.Body.String(), password) { + t.Errorf("the response does not show the generated password") + } + a, rl, ok := s.app(t, "prod-server") + if a.AddressMode != store.AddressModeList || len(a.Addresses) != 1 || !a.AuthIPRestrict || len(a.AuthAllowedIPs) != 1 || + !ok || rl.MaxMessages != 200 || rl.WindowSeconds != 3600 { + t.Errorf("application = %+v, limit %+v (%v)", a, rl, ok) + } + + // The password page has no GET of its own: the form's address, read + // again, shows the form and not the secret. + again := postFormAs(s.h.HandleApplicationEdit, globalPrincipal, "/outbound/domains/1/applications/1", + map[string]string{"id": idStr(s.d.ID), "aid": idStr(a.ID)}, nil) + if strings.Contains(again.Body.String(), password) { + t.Error("a later request shows the password again") + } +} + +// A form that does not validate creates nothing — whichever of its three parts +// is wrong, and although the other two are fine. +func TestApplicationCreateIsAllOrNothing(t *testing.T) { + s := newAppStand(t) + good := url.Values{ + "login": {"prod-server"}, "mode": {"wildcard"}, + "auth_ip_restrict": {"1"}, "auth_allowed_ips": {"203.0.113.10"}, + "rl_mode": {"manual"}, "max_messages": {"200"}, + } + for name, change := range map[string][2]string{ + "a bad client IP": {"auth_allowed_ips", "nope"}, + "a limit above level 1": {"max_messages", "100000"}, + "an address of another domain": {"addresses", "a@evil.example"}, + "a bad login": {"login", "has space"}, + } { + form := url.Values{} + for k, v := range good { + form[k] = v + } + form.Set(change[0], change[1]) + if change[0] == "addresses" { + form.Set("mode", "list") + } + rec := postFormAs(s.h.HandleApplicationCreate, globalPrincipal, "/outbound/domains/1/applications/new", s.paths, form) + if rec.Code != http.StatusBadRequest { + t.Errorf("%s: %d, want 400", name, rec.Code) + } + } + apps, _ := s.h.store.ListApplicationsByDomain(s.d.ID) + if len(apps) != 0 || len(s.sasl) != 0 { + t.Fatalf("the refused forms left %d application(s) and %d SASL account(s)", len(apps), len(s.sasl)) + } +} + +func TestApplicationSaveIsOnePost(t *testing.T) { + s := newAppStand(t) + a, pw, err := s.h.apps.CreateWithSettings(s.d.ID, "prod-server", app.Settings{ + Mode: store.AddressModeWildcard, Limit: app.Limit{Mode: store.RateLimitModeManual, MaxMessages: 100, WindowSeconds: 3600}, + }, nil) + if err != nil { + t.Fatal(err) + } + paths := map[string]string{"id": idStr(s.d.ID), "aid": idStr(a.ID)} + save := func(form url.Values) int { + return postFormAs(s.h.HandleApplicationSave, globalPrincipal, "/outbound/domains/1/applications/1", paths, form).Code + } + + if code := save(url.Values{ + "mode": {"list"}, "addresses": {"alerts@example.org"}, + "auth_ip_restrict": {"1"}, "auth_allowed_ips": {"203.0.113.10"}, "rl_mode": {"domain"}, + }); code != http.StatusSeeOther { + t.Fatalf("save = %d", code) + } + got, _, hasLimit := s.app(t, "prod-server") + if got.AddressMode != store.AddressModeList || !got.AuthIPRestrict || hasLimit { + t.Errorf("after the save: %+v, own limit: %v", got, hasLimit) + } + if s.sasl["prod-server"] != pw { + t.Error("saving the form changed the password") + } + + // One wrong part refuses the whole form and leaves all three as they were. + if code := save(url.Values{ + "mode": {"wildcard"}, "rl_mode": {"manual"}, "max_messages": {"100000"}, + }); code != http.StatusBadRequest { + t.Errorf("a limit above level 1 = %d, want 400", code) + } + if got, _, hasLimit := s.app(t, "prod-server"); got.AddressMode != store.AddressModeList || !got.AuthIPRestrict || hasLimit { + t.Errorf("a refused form changed the application: %+v, own limit: %v", got, hasLimit) + } + + // The login is not renamed through this form. + if code := save(url.Values{"login": {"renamed"}, "mode": {"wildcard"}}); code != http.StatusBadRequest { + t.Errorf("a changed login = %d, want 400", code) + } + if _, err := s.h.store.GetApplicationByLogin("renamed"); err == nil { + t.Error("the application was renamed") + } +} + +// A new password is the response to its POST, shown once and never cached. +func TestRegeneratedPasswordIsNotCached(t *testing.T) { + s := newAppStand(t) + a, old, err := s.h.apps.CreateWithSettings(s.d.ID, "prod-server", app.Settings{Mode: store.AddressModeWildcard}, nil) + if err != nil { + t.Fatal(err) + } + paths := map[string]string{"id": idStr(s.d.ID), "aid": idStr(a.ID)} + rec := postFormAs(s.h.HandleRegenPassword, globalPrincipal, "/outbound/domains/1/applications/1/password", paths, url.Values{}) + if rec.Code != http.StatusOK || rec.Header().Get("Cache-Control") != "no-store" { + t.Fatalf("new password = %d, Cache-Control %q", rec.Code, rec.Header().Get("Cache-Control")) + } + now := s.sasl["prod-server"] + if now == old || !strings.Contains(rec.Body.String(), now) { + t.Error("the response does not show the new password") + } +} diff --git a/internal/web/handlers/handlers_delegation_test.go b/internal/web/handlers/handlers_delegation_test.go index 15added..38c6d60 100644 --- a/internal/web/handlers/handlers_delegation_test.go +++ b/internal/web/handlers/handlers_delegation_test.go @@ -272,7 +272,8 @@ func TestInboundListWithNothingLeftIs404(t *testing.T) { } // The outbound side of the same rule, route by route: a domain administrator -// asking for another tenant's sending domain or application by id gets 404, +// asking for another tenant's sending domain or application by id gets 404 — +// also when the application is addressed through the caller's own domain — // and so does a user whose reach is inbound only. func TestOutboundRoutesAnswerAnotherTenant404(t *testing.T) { h, domains := serverWithTwoDomains(t) @@ -291,25 +292,35 @@ func TestOutboundRoutesAnswerAnotherTenant404(t *testing.T) { inboundOnly := userWith(t, h.store, "inbound-only", store.RoleDomain, store.Reach{InboundDomainIDs: []int64{in.ID}}) did := map[string]string{"id": idStr(second.ID)} - aid := map[string]string{"aid": idStr(foreignApp.ID)} - routes := []struct { + // Another tenant's application, addressed through its own domain — and + // through the caller's own domain, which it is not in: the URL names a + // domain the caller may open, but the application still is not theirs. + aid := map[string]string{"id": idStr(second.ID), "aid": idStr(foreignApp.ID)} + smuggled := map[string]string{"id": idStr(first.ID), "aid": idStr(foreignApp.ID)} + type outRoute struct { method, target string handler http.HandlerFunc pathValues map[string]string - }{ + } + routes := []outRoute{ {"GET", "/outbound/domains/{id}", h.HandleDomainDetail, did}, + {"GET", "/outbound/domains/{id}/settings", h.HandleDomainDetail, did}, {"POST", "/outbound/domains/{id}/dns-recheck", h.HandleDomainDNSRecheck, did}, - {"POST", "/outbound/domains/{id}/applications", h.HandleAddApplication, did}, {"POST", "/outbound/domains/{id}/settings/ratelimit", h.HandleDomainRateLimit, did}, {"POST", "/outbound/domains/{id}/settings/ratelimit/recalc", h.HandleDomainRateLimitRecalc, did}, {"POST", "/outbound/domains/{id}/settings/reports", h.HandleDomainDMARC, did}, {"POST", "/outbound/domains/{id}/settings/export", h.HandleExportDomain, did}, - {"POST", "/applications/{aid}/mode", h.HandleUpdateAppMode, aid}, - {"POST", "/applications/{aid}/authips", h.HandleAppAuthIPs, aid}, - {"POST", "/applications/{aid}/password", h.HandleRegenPassword, aid}, - {"POST", "/applications/{aid}/ratelimit", h.HandleAppRateLimit, aid}, - {"POST", "/applications/{aid}/ratelimit/recalc", h.HandleAppRateLimitRecalc, aid}, - {"POST", "/applications/{aid}/delete", h.HandleDeleteApplication, aid}, + {"GET", "/outbound/domains/{id}/applications/new", h.HandleApplicationNew, did}, + {"POST", "/outbound/domains/{id}/applications/new", h.HandleApplicationCreate, did}, + } + for _, pv := range []map[string]string{aid, smuggled} { + routes = append(routes, + outRoute{"GET", "/outbound/domains/{id}/applications/{aid}", h.HandleApplicationEdit, pv}, + outRoute{"POST", "/outbound/domains/{id}/applications/{aid}", h.HandleApplicationSave, pv}, + outRoute{"POST", "/outbound/domains/{id}/applications/{aid}/password", h.HandleRegenPassword, pv}, + outRoute{"POST", "/outbound/domains/{id}/applications/{aid}/ratelimit/recalc", h.HandleAppRateLimitRecalc, pv}, + outRoute{"POST", "/outbound/domains/{id}/applications/{aid}/delete", h.HandleDeleteApplication, pv}, + ) } for _, rt := range routes { for who, p := range map[string]*auth.Principal{ diff --git a/internal/web/handlers/handlers_monitor_test.go b/internal/web/handlers/handlers_monitor_test.go index fbe8e7e..a700cc7 100644 --- a/internal/web/handlers/handlers_monitor_test.go +++ b/internal/web/handlers/handlers_monitor_test.go @@ -443,7 +443,7 @@ func getBodyAs(t *testing.T, h http.HandlerFunc, target string, p auth.Principal rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, target, nil) req = auth.RequestWithPrincipal(req, p) - if rest, ok := strings.CutPrefix(req.URL.Path, "/outbound/log/"); ok && rest != "rows" { + if rest, ok := strings.CutPrefix(req.URL.Path, "/outbound/log/"); ok && rest != "fragment" { req.SetPathValue("id", rest) } h(rec, req) diff --git a/internal/web/handlers/handlers_ratelimit.go b/internal/web/handlers/handlers_ratelimit.go index 08c2b70..26de882 100644 --- a/internal/web/handlers/handlers_ratelimit.go +++ b/internal/web/handlers/handlers_ratelimit.go @@ -165,58 +165,6 @@ func (h *Handlers) HandleDomainRateLimit(w http.ResponseWriter, r *http.Request) http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?ratelimit=1", d.ID), http.StatusSeeOther) } -func (h *Handlers) HandleAppRateLimit(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) - if !ok { - return - } - d, err := h.domains.Get(a.DomainID) - if err != nil { - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - in, err := parseRateLimitForm(r, h.l1Messages()) - if err != nil { - h.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{ - FormMode: store.AddressModeWildcard, - RateLimitErr: fmt.Sprintf("%s: %s", a.Login, err.Error()), - }) - return - } - if err := h.applyRateLimit(in, store.RateLimitScopeApp, a.ID, h.apps); err != nil { - logf("panel: application %d: save rate limit: %v", a.ID, err) - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?ratelimit=1", a.DomainID), http.StatusSeeOther) -} - -func (h *Handlers) HandleAppAuthIPs(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) - if !ok { - return - } - d, err := h.domains.Get(a.DomainID) - if err != nil { - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - restrict, ips, err := parseAppAuthIPsForm(r) - if err != nil { - h.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{ - FormMode: store.AddressModeWildcard, - RateLimitErr: fmt.Sprintf("%s: %s", a.Login, err.Error()), - }) - return - } - if err := h.apps.UpdateAuthIPs(a.ID, restrict, ips); err != nil { - logf("panel: application %d: save auth IPs: %v", a.ID, err) - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?authips=1", a.DomainID), http.StatusSeeOther) -} - func (h *Handlers) HandleDomainRateLimitRecalc(w http.ResponseWriter, r *http.Request) { d, ok := h.lookupDomain(w, r) if !ok { @@ -234,12 +182,11 @@ func (h *Handlers) HandleDomainRateLimitRecalc(w http.ResponseWriter, r *http.Re } func (h *Handlers) HandleAppRateLimitRecalc(w http.ResponseWriter, r *http.Request) { - a, ok := h.lookupApplication(w, r) + a, d, ok := h.lookupDomainApplication(w, r) if !ok { return } if err := h.recalcRateLimit(store.RateLimitScopeApp, a.ID); err != nil { - d, _ := h.domains.Get(a.DomainID) h.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{ FormMode: store.AddressModeWildcard, RateLimitErr: fmt.Sprintf("%s: %s", a.Login, err.Error()), diff --git a/internal/web/view/templates/domain_detail.html b/internal/web/view/templates/domain_detail.html index 7d470fc..436285e 100644 --- a/internal/web/view/templates/domain_detail.html +++ b/internal/web/view/templates/domain_detail.html @@ -228,7 +228,7 @@ above the domains list. */}}

Add an application

-
+ -
-
+ {{/* The application form is one POST: sender, client IPs and rate + limit are saved together by the one button under the columns. */}} +

Address mode

Which From addresses this application may use.

-
- -

Client IP allow-list @@ -335,7 +335,6 @@ MAIL FROM. When off, any client IP is allowed. Like the level-2 rate limit this runs in the journal-milter and is fail-open — it is a sending control, not an authentication boundary.

-
- -

Level-2 rate limit @@ -353,11 +350,11 @@ ceiling may be higher or lower than the domain setting (≤ level 1). When unset, the domain limit{{if $.DomainHasRL}} ({{$.DomainRLMaxNum}}){{end}} or level 1 applies.

-
- + + +
@@ -381,24 +378,17 @@ messages are sent.{{if .AutoUpdated}} Last recalculated {{.AutoUpdated}}.{{end}} Peak {{.Stats.PeakPerHour}} msg/h shown for comparison.

- -
- - {{if .IsAuto}} -
- -
- {{end}} - {{if .HasLimit}} -
- - -
- {{end}} -
+ +
+ + {{if .IsAuto}} +
+ +
+ {{end}} +
diff --git a/internal/web/web.go b/internal/web/web.go index fb4881c..7382d63 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -202,13 +202,16 @@ func (s *Server) muxes() (public, authed *http.ServeMux) { authed.HandleFunc("POST /outbound/domains/{id}/settings/ratelimit", h.HandleDomainRateLimit) authed.HandleFunc("POST /outbound/domains/{id}/settings/ratelimit/recalc", h.HandleDomainRateLimitRecalc) authed.HandleFunc("POST /outbound/domains/{id}/settings/export", h.HandleExportDomain) - authed.HandleFunc("POST /outbound/domains/{id}/applications", h.HandleAddApplication) - authed.HandleFunc("POST /applications/{aid}/mode", h.HandleUpdateAppMode) - authed.HandleFunc("POST /applications/{aid}/authips", h.HandleAppAuthIPs) - authed.HandleFunc("POST /applications/{aid}/password", h.HandleRegenPassword) - authed.HandleFunc("POST /applications/{aid}/ratelimit", h.HandleAppRateLimit) - authed.HandleFunc("POST /applications/{aid}/ratelimit/recalc", h.HandleAppRateLimitRecalc) - authed.HandleFunc("POST /applications/{aid}/delete", h.HandleDeleteApplication) + // An application lives under its domain, and its form is one POST: sender, + // client IPs and rate limit are saved together. The page that shows a new + // password is the response to its POST and has no GET path. + authed.HandleFunc("GET /outbound/domains/{id}/applications/new", h.HandleApplicationNew) + authed.HandleFunc("POST /outbound/domains/{id}/applications/new", h.HandleApplicationCreate) + authed.HandleFunc("GET /outbound/domains/{id}/applications/{aid}", h.HandleApplicationEdit) + authed.HandleFunc("POST /outbound/domains/{id}/applications/{aid}", h.HandleApplicationSave) + authed.HandleFunc("POST /outbound/domains/{id}/applications/{aid}/password", h.HandleRegenPassword) + authed.HandleFunc("POST /outbound/domains/{id}/applications/{aid}/ratelimit/recalc", h.HandleAppRateLimitRecalc) + authed.HandleFunc("POST /outbound/domains/{id}/applications/{aid}/delete", h.HandleDeleteApplication) authed.HandleFunc("GET /outbound/log", h.HandleDeliveries) authed.HandleFunc("GET /outbound/log/fragment", h.HandleDeliveriesRows) diff --git a/test/e2e/negative_test.go b/test/e2e/negative_test.go index ea4bc2d..7dd39d8 100644 --- a/test/e2e/negative_test.go +++ b/test/e2e/negative_test.go @@ -26,7 +26,7 @@ func testLevel2RateLimit(t *testing.T, sc *scenario) { if err != nil { t.Fatal(err) } - if err := sc.panel.setRateLimit(fmt.Sprintf("/applications/%s/ratelimit", appID), 1, 3600); err != nil { + if err := sc.panel.setApplicationLimit(sc.domainID, appID, 1, 3600); err != nil { t.Fatalf("save application rate limit: %v", err) } diff --git a/test/e2e/panel_client.go b/test/e2e/panel_client.go index af24655..0f1598e 100644 --- a/test/e2e/panel_client.go +++ b/test/e2e/panel_client.go @@ -172,7 +172,7 @@ func (c *panelClient) dkimRecord(domainID string) (name, value string, err error // login/password the panel renders inline (security.md — never recoverable // later, so this is the only place to read it). func (c *panelClient) addApplication(domainID, login, mode, addresses string) (appLogin, password string, err error) { - resp, body, err := c.postForm("/outbound/domains/"+domainID+"/applications", url.Values{ + resp, body, err := c.postForm("/outbound/domains/"+domainID+"/applications/new", url.Values{ "login": {login}, "mode": {mode}, "addresses": {addresses}, @@ -194,14 +194,17 @@ func (c *panelClient) addApplication(domainID, login, mode, addresses string) (a return appLogin, password, nil } -// setRateLimit saves a level-2 limit (guide § Rate limiting) on an application -// or domain (/applications/{id}/ratelimit or /outbound/domains/{id}/settings/ratelimit). -func (c *panelClient) setRateLimit(path string, maxMessages, windowSeconds int) error { +// setApplicationLimit saves a manual level-2 limit (guide § Rate limiting) on +// an application. The application form is one POST, so the sender rule goes +// with it: the application stays on the domain wildcard it was created with. +func (c *panelClient) setApplicationLimit(domainID, appID string, maxMessages, windowSeconds int) error { vals := url.Values{ + "mode": {"wildcard"}, + "rl_mode": {"manual"}, "max_messages": {fmt.Sprintf("%d", maxMessages)}, "window_seconds": {fmt.Sprintf("%d", windowSeconds)}, } - resp, body, err := c.postForm(path, vals) + resp, body, err := c.postForm("/outbound/domains/"+domainID+"/applications/"+appID, vals) if err != nil { return err }