From 07be0cfb583cc3ece24443856444a448beefa4e7 Mon Sep 17 00:00:00 2001 From: Mix Date: Thu, 15 Oct 2026 11:07:02 +0300 Subject: [PATCH] audit 4: sign-in timing and sessions, server timeouts, polling that recovers Steps 1-3 of stage 4, tested together (their docs share files). Step 1 (M8, L15, L16): an unknown name pays a bcrypt comparison at the panel's work factor too (noSuchUserHash, made once) - TestLoginTakesAsLongForAnUnknownName holds the medians within a factor of two; the Account form's current-password check has its own limiter, per user (AllowPasswordChange); a sign-in destroys every session the request presents before it issues its own. Step 2 (L14): newHTTPServer - ReadTimeout 30 s, WriteTimeout 60 s, IdleTimeout 2 min beside ReadHeaderTimeout; the full backup moves its write deadline to two hours through http.ResponseController (e2e: the log shows no failure to move it). Step 3 (M11), read in htmx 2.0.4's source (dev/upstream/htmx-2.0.4, the npm build byte-equal to the shipped htmx.min.js): the poll passes source: box, and the next refresh is scheduled from htmx:afterRequest, which htmx fires on load, error, abort and timeout, and after a swap on the nearest parent left (the box found by detail.target's id); failures in a row double the wait to 60 s. RequireAuth answers htmx with 401 + HX-Redirect: /login, which htmx follows before it looks at the status. testdata/poll_test.js runs panel.js in node against a stand-in DOM and clock (TestPanelJS; skipped without node, which the test server lacks - run here, and it fails on the old panel.js). Also measured: the stage-3 e2e regression for H1 fails as it should on HEAD with the old entrypoint ("check data/postfix/e2e_extra_map: permission denied", the panel restarting). Tested on the test server: go vet + go test of 25 packages, make e2e (all pass, 639 s) on this tree. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 27 ++++ cmd/panel/httpserver.go | 26 ++- cmd/panel/httpserver_test.go | 25 +++ docs/architecture.md | 12 +- docs/audit-2026-10-12.md | 31 +++- docs/roadmap.md | 2 +- docs/security.md | 15 +- internal/web/auth/auth.go | 34 ++-- internal/web/auth/handlers.go | 24 +++ internal/web/auth/handlers_test.go | 94 +++++++++++ internal/web/auth/middleware.go | 21 ++- internal/web/handlers/handlers_account.go | 2 +- .../web/handlers/handlers_account_test.go | 27 ++++ internal/web/handlers/handlers_backup.go | 9 ++ internal/web/view/paneljs_test.go | 22 +++ internal/web/view/static/panel.js | 54 +++++-- internal/web/view/testdata/poll_test.js | 148 ++++++++++++++++++ test/e2e/restore_test.go | 5 + 18 files changed, 535 insertions(+), 43 deletions(-) create mode 100644 cmd/panel/httpserver_test.go create mode 100644 internal/web/view/paneljs_test.go create mode 100644 internal/web/view/testdata/poll_test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index f86063a..53e5fd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,33 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Security + +- **The sign-in told which usernames exist by how long it took.** A wrong + password cost a bcrypt comparison, an unknown name none. Both now cost one + (audit 2026-10-12, M8). +- **Signing in kept the session the browser already presented.** A sign-in + now ends it before it issues its own, so a session planted before the + sign-in is worth nothing after (audit 2026-10-12, L16). + +### Fixed + +- **Ten wrong current passwords on Account locked the address out of + signing in for 15 minutes.** The check has its own limit now, ten in 15 + minutes per user (audit 2026-10-12, L15). +- **A client could hold the panel's server as long as it liked** once its + request header was read. The body now has 30 seconds, the answer 60, an + idle kept-alive connection 2 minutes; a full backup download moves its own + limit to two hours (audit 2026-10-12, L14). +- **One failed refresh stopped a monitoring page for good.** Overview, + Health and the logs refresh their boxes on a timer; a refresh that failed + - the panel restarting, the network gone - scheduled no next one, and the + page stood still until the tab was hidden and shown again. Each refresh + now schedules the next however it ended, waiting twice as long after each + failure in a row, up to a minute. A box whose session had expired was + filled with the sign-in page; the whole page goes to sign-in now (audit + 2026-10-12, M11). + ## [2.5.3] - 2026-10-15 Stage 3 of the 2026-10-12 audit: persistence and restore. A restore is diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index b51bafc..b72291f 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -134,6 +134,26 @@ func newPanel(cfg config, st *store.Store) (*web.Server, error) { }, cfg.setupTokenPath) } +// newHTTPServer bounds what a client may hold the panel's server for (audit +// 2026-10-12, L14): the request with its body - a form, or a domain export +// of 1 MiB at most - in 30 s; the answer in 60 s, the slowest page being one +// that asks DNS; an idle kept-alive connection 2 min. The full backup moves +// its own write deadline to two hours: it streams for as long as the archive +// takes (handlers.HandleBackup). net/http, Go 1.26: ReadTimeout covers the +// header and the body, WriteTimeout runs from the end of the header's read +// to the end of the response's write, IdleTimeout is the wait for the next +// request on a kept-alive connection. +func newHTTPServer(addr string, h http.Handler) *http.Server { + return &http.Server{ + Addr: addr, + Handler: h, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 60 * time.Second, + IdleTimeout: 2 * time.Minute, + } +} + // serveHTTP runs the control-panel HTTP server until ctx is cancelled, using // the database handle shared by all roles: setup, login and the authenticated // panel surface (security.md). @@ -146,11 +166,7 @@ func serveHTTP(ctx context.Context, cfg config, st *store.Store) error { return err } - srv := &http.Server{ - Addr: cfg.httpAddr, - Handler: srvApp.Handler(), - ReadHeaderTimeout: 10 * time.Second, - } + srv := newHTTPServer(cfg.httpAddr, srvApp.Handler()) // Shut the server down cleanly when the process is asked to stop. go func() { diff --git a/cmd/panel/httpserver_test.go b/cmd/panel/httpserver_test.go new file mode 100644 index 0000000..232cac9 --- /dev/null +++ b/cmd/panel/httpserver_test.go @@ -0,0 +1,25 @@ +package main + +import ( + "testing" + "time" +) + +// The panel's server bounds every phase a client could hold it in (audit +// 2026-10-12, L14); before, only the header's read was. +func TestHTTPServerHasItsTimeouts(t *testing.T) { + s := newHTTPServer(":0", nil) + for name, d := range map[string]time.Duration{ + "ReadHeaderTimeout": s.ReadHeaderTimeout, + "ReadTimeout": s.ReadTimeout, + "WriteTimeout": s.WriteTimeout, + "IdleTimeout": s.IdleTimeout, + } { + if d <= 0 { + t.Errorf("%s is %v", name, d) + } + } + if s.ReadTimeout < s.ReadHeaderTimeout || s.WriteTimeout < 30*time.Second { + t.Errorf("read %v (header %v), write %v: too short for a form or a DNS page", s.ReadTimeout, s.ReadHeaderTimeout, s.WriteTimeout) + } +} diff --git a/docs/architecture.md b/docs/architecture.md index 3735568..cc9c7b2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -724,9 +724,15 @@ a refresh cannot differ. HTMX refreshes them every 5 s while the operator is active on the page, every 30 s when the tab is visible but idle, and not at all when it is hidden — scheduled in `panel.js` via `data-poll`, not `hx-trigger="every …"`, because a trigger filter is evaluated with `new -Function`, which the CSP forbids. Polling does not extend the session idle -timeout (only non-`HX-Request` `GET`s and mutating requests count as -activity). The Outbound queue's retry-policy box is outside the polled region: +Function`, which the CSP forbids. The next refresh is scheduled when a +request of the box ends, however it ended: a refresh that failed - an error +answer, the panel restarting, the network gone - is tried again after twice +the wait, doubling with each failure in a row up to a minute, and the wait +is back to normal after one that works. A request without a session answers +htmx with `HX-Redirect: /login` (status 401), and htmx loads the sign-in +page in place of the whole page; a browser gets the 303 as before. Polling +does not extend the session idle timeout (only non-`HX-Request` `GET`s and +mutating requests count as activity). The Outbound queue's retry-policy box is outside the polled region: it is the start-up `postconf -h` snapshot (see [Panel binary](#panel-binary-cmdpanel)), not a live re-read. diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index f1afcc4..e0bc59c 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -1185,15 +1185,40 @@ command whose leftover is the owner's alone. ### Stage 4 — panel behaviour (M8, M11, L10–L19) → `2.5.4` -- [ ] Login: dummy bcrypt on unknown user; a test that measures both paths within +- [x] Login: dummy bcrypt on unknown user; a test that measures both paths within a tolerance. Account password change gets its own limiter bucket. Login destroys the presented session. — **Opus** -- [ ] `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus** -- [ ] **Polling recovery**: schedule from `htmx:afterRequest` on the box (or pass + *Done 2026-10-15:* `noSuchUserHash` (BcryptCost, made once) compared + against for an unknown name or a failed lookup; + `TestLoginTakesAsLongForAnUnknownName` holds the medians of five tries + each within a factor of two at the real work factor. The bucket is per + user (`AllowPasswordChange(userID)`, 10 in 15 min): the guesser is + whoever holds that user's session. Sign-in destroys every session cookie + the request carries before it creates its own. +- [x] `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus** + *Done 2026-10-15:* 30 s, 60 s, 2 min (`newHTTPServer`); the full backup + would have been cut at 60 s, so `HandleBackup` moves its write deadline + to two hours through `http.ResponseController` (no wrapper of the + ResponseWriter in the chain; e2e checks the log has no failure to move + it). A test pins the four values. +- [x] **Polling recovery**: schedule from `htmx:afterRequest` on the box (or pass `source`), handle `sendError`/`timeout` with back-off; `RequireAuth` answers an `HX-Request` with `HX-Redirect: /login` instead of a 303. A minimal JS test (node + jsdom) or at least an e2e that restarts the panel with Overview open. — **Sonnet** + *Done 2026-10-15,* read in htmx 2.0.4's source (the npm build, byte-equal + to the shipped `htmx.min.js`, in `dev/upstream/htmx-2.0.4`): both - + `source: el`, and the schedule from `htmx:afterRequest`, which + `issueAjaxRequest()` fires on load, error, abort and timeout, and after a + swap that removed the box fires again on the nearest parent still in the + page (so the box is found by `detail.target`'s id); + `detail.successful` tells failure. Back-off doubles per failure in a row + to 60 s. `toLogin`: 401 + `HX-Redirect` for htmx, which + `handleAjaxResponse()` follows before it looks at the status. No jsdom + (no node tooling in the repo): `testdata/poll_test.js` runs panel.js in + node against a stand-in for the DOM and htmx with a moved clock, from + `TestPanelJSPolling` (skipped where node is absent - the test server's + Go has none; it fails on the old panel.js with ten findings). - [ ] `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to `components.go`, flip the test); the recalc button posts only what it needs or warns; `HandleReload` either covers DMARC maps and filter lists or diff --git a/docs/roadmap.md b/docs/roadmap.md index 93a57e7..19986be 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`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`, 3 `2.5.3`; next stage 4 → `2.5.4`) | 55/81 | [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`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`, 3 `2.5.3`; next stage 4 → `2.5.4`) | 58/81 | [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/docs/security.md b/docs/security.md index b289830..32361c3 100644 --- a/docs/security.md +++ b/docs/security.md @@ -104,10 +104,19 @@ The panel is exposed to the internet — the items below are **not optional**. - `postfix reload` and any `exec` run **without** shell interpolation of user input; arguments are passed as separate elements. - Writes to config files are escaped (no injection of Postfix directives). +- The panel's HTTP server bounds every phase a client could hold it in: the + header 10 s, the request 30 s, the answer 60 s, an idle kept-alive + connection 2 min; a full backup download moves its own write limit to + two hours (`2.5.4`). ### Authentication and sessions -- Rate limiting on login (per IP, with lockout or delay). +- Rate limiting on login (per IP, with lockout or delay). The Account form's + current-password check has a limit of its own, ten in 15 minutes per user, + so guesses there do not lock the address out of signing in (`2.5.4`). +- An unknown username is refused in the time a wrong password is: the sign-in + compares a bcrypt hash of the panel's work factor either way, so the delay + does not tell which names exist (`2.5.4`). - Sessions: cryptographically random token; cookie `HttpOnly`, `Secure`, `SameSite`. - Sessions live in SQLite (SHA-256 of the token, not the token itself); sliding @@ -120,7 +129,9 @@ The panel is exposed to the internet — the items below are **not optional**. - **What ends a user's sessions:** deleting the user (all); a password change on Account (that user's *other* sessions — never another user's); a password set for them by a global user on the user form (all of theirs, except the current - one when they set their own); signing out (the current one only). There is no + one when they set their own); signing out (the current one only); signing + in (the session the browser presented, if any - a token planted before the + sign-in is worth nothing after it, `2.5.4`). There is no "sign out everywhere", and a restored backup brings back the session rows it held. - Polling by the monitoring screens (`HX-Request` `GET`s) does not renew a diff --git a/internal/web/auth/auth.go b/internal/web/auth/auth.go index 7ff51ba..f7329a0 100644 --- a/internal/web/auth/auth.go +++ b/internal/web/auth/auth.go @@ -6,6 +6,7 @@ import ( "log" "net" "net/http" + "strconv" "strings" "time" @@ -29,8 +30,9 @@ type Module struct { sessions *sessionStore setup *setupManager - loginLimiter *rateLimiter - setupLimiter *rateLimiter + loginLimiter *rateLimiter + setupLimiter *rateLimiter + passwordLimiter *rateLimiter trustedProxies []*net.IPNet } @@ -43,16 +45,22 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo idleDays = 7 } m := &Module{ - store: st, - cfg: cfg, - view: v, - sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour), - setupLimiter: newRateLimiter(10, time.Minute), - loginLimiter: newRateLimiter(10, 15*time.Minute), - trustedProxies: cfg.TrustedProxyCIDRs, + store: st, + cfg: cfg, + view: v, + sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour), + setupLimiter: newRateLimiter(10, time.Minute), + loginLimiter: newRateLimiter(10, 15*time.Minute), + // The Account form's current-password check has a bucket of its + // own, per user: ten wrong guesses there used to lock the address + // out of signing in (audit 2026-10-12, L15), and the one guessing + // is whoever holds that user's session, wherever from. + passwordLimiter: newRateLimiter(10, 15*time.Minute), + trustedProxies: cfg.TrustedProxyCIDRs, } m.setupLimiter.startSweeper() m.loginLimiter.startSweeper() + m.passwordLimiter.startSweeper() m.setup = newSetupManager(st, cfg.Hostname, setupTokenPath) return m } @@ -63,10 +71,10 @@ func (m *Module) Bootstrap() error { return m.setup.bootstrap() } -// AllowLoginAttempt reports whether a login or account-password change attempt -// from r is within the rate limit (security.md). -func (m *Module) AllowLoginAttempt(r *http.Request) bool { - return m.loginLimiter.Allow(clientIP(r, m.trustedProxies)) +// AllowPasswordChange reports whether another current-password check on the +// Account form is within the user's rate limit (security.md). +func (m *Module) AllowPasswordChange(userID int64) bool { + return m.passwordLimiter.Allow("user:" + strconv.FormatInt(userID, 10)) } // SessionToken returns the session token the request carries, if exactly one diff --git a/internal/web/auth/handlers.go b/internal/web/auth/handlers.go index c25eb74..3e6fe52 100644 --- a/internal/web/auth/handlers.go +++ b/internal/web/auth/handlers.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" "strings" + "sync" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" @@ -117,6 +118,10 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { if !errors.Is(err, store.ErrUserNotFound) { logf("panel: login: get user failed: %v", err) } + // A comparison anyway, so an unknown name is refused in the time + // a wrong password is: the answer's delay told which names exist + // (audit 2026-10-12, M8). + _ = bcrypt.CompareHashAndPassword(noSuchUserHash(), []byte(password)) m.renderLogin(w, http.StatusUnauthorized, "Invalid username or password.") return } @@ -126,6 +131,15 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { return } + // The session the browser presents, if any, ends here: a sign-in issues + // a new token and leaves no other behind it - one planted before the + // sign-in is worth nothing after (audit 2026-10-12, L16). + name := m.sessionCookie() + for _, c := range r.Cookies() { + if c.Name == name { + m.sessions.Destroy(c.Value) + } + } token, err := m.sessions.Create(user.ID) if err != nil { logf("panel: login: create session failed: %v", err) @@ -136,6 +150,16 @@ func (m *Module) submitLogin(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/", http.StatusSeeOther) } +// noSuchUserHash is a hash of the panel's work factor, compared against when +// a sign-in names no user. +var noSuchUserHash = sync.OnceValue(func() []byte { + h, err := bcrypt.GenerateFromPassword([]byte("selfpost: no such user"), BcryptCost) + if err != nil { + panic(err) + } + return h +}) + // setSessionCookie sets the session cookie. SameSite=Lax withholds it from a // cross-site request unless that is a top-level navigation with a safe method // (rfc6265bis § 5.8.3, § 5.6.7.1), so a cross-site POST arrives without it. diff --git a/internal/web/auth/handlers_test.go b/internal/web/auth/handlers_test.go index 232aed1..100b13f 100644 --- a/internal/web/auth/handlers_test.go +++ b/internal/web/auth/handlers_test.go @@ -1,11 +1,14 @@ package auth import ( + "fmt" "net/http" "net/http/httptest" "net/url" + "slices" "strings" "testing" + "time" "golang.org/x/crypto/bcrypt" ) @@ -282,3 +285,94 @@ func expireSetupToken(m *Module) { defer m.setup.mu.Unlock() m.setup.expiresAt = m.setup.expiresAt.Add(-2 * setupTokenTTL) } + +// An unknown name is refused in the time a wrong password is: both pay one +// bcrypt comparison at the panel's work factor (audit 2026-10-12, M8). The +// medians of a few attempts each must be within a factor of two; before the +// fix the unknown name answered some hundred times faster. +func TestLoginTakesAsLongForAnUnknownName(t *testing.T) { + m := testModule(t, false) + hash, err := bcrypt.GenerateFromPassword([]byte(testPassword), BcryptCost) + if err != nil { + t.Fatal(err) + } + if err := m.store.CreateGlobalUser("admin", string(hash)); err != nil { + t.Fatal(err) + } + noSuchUserHash() // made once, not inside a measurement + + median := func(username string) time.Duration { + var d []time.Duration + for i := 0; i < 5; i++ { + start := time.Now() + // A fresh address each time: the limiter is not what is timed. + rec := postLogin(m, fmt.Sprintf("203.0.113.%d:5000", len(d)+10*len(username)), username, "not-the-password") + d = append(d, time.Since(start)) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("%s: status %d, want 401", username, rec.Code) + } + } + slices.Sort(d) + return d[len(d)/2] + } + known, unknown := median("admin"), median("nobody") + if unknown*2 < known || known*2 < unknown { + t.Errorf("a wrong password takes %v, an unknown name %v: the time tells which names exist", known, unknown) + } +} + +// A sign-in ends the session the browser presented: a token planted before +// it is worth nothing after (audit 2026-10-12, L16). +func TestLoginEndsThePresentedSession(t *testing.T) { + m := moduleWithAdmin(t) + u, err := m.store.GetUserByUsername("admin") + if err != nil { + t.Fatal(err) + } + planted, err := m.sessions.Create(u.ID) + if err != nil { + t.Fatal(err) + } + form := url.Values{"username": {"admin"}, "password": {testPassword}} + r := httptest.NewRequest(http.MethodPost, "http://panel.example.com/login", strings.NewReader(form.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + r.AddCookie(&http.Cookie{Name: m.sessionCookie(), Value: planted}) + rec := httptest.NewRecorder() + m.HandleLogin(rec, r) + if rec.Code != http.StatusSeeOther { + t.Fatalf("sign-in = %d", rec.Code) + } + if _, ok := m.sessions.Lookup(planted); ok { + t.Error("the session presented at sign-in still works after it") + } + if fresh := sessionCookieValue(t, m, rec); fresh == "" || fresh == planted { + t.Errorf("the sign-in issued %q", fresh) + } else if _, ok := m.sessions.Lookup(fresh); !ok { + t.Error("the new session does not work") + } +} + +// htmx asking without a session is told to load the sign-in page, not given +// it to swap into the box that asked (audit 2026-10-12, M11); a browser is +// redirected as before. +func TestRequireAuthTellsHtmxToSignIn(t *testing.T) { + m := moduleWithAdmin(t) + h := m.RequireAuth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("a request without a session reached the page") + })) + + r := httptest.NewRequest(http.MethodGet, "http://panel.example.com/overview/stats", nil) + r.Header.Set("HX-Request", "true") + rec := httptest.NewRecorder() + h.ServeHTTP(rec, r) + if rec.Code != http.StatusUnauthorized || rec.Header().Get("HX-Redirect") != "/login" || rec.Header().Get("Location") != "" { + t.Errorf("htmx without a session: %d, HX-Redirect %q, Location %q; want 401 with HX-Redirect /login", + rec.Code, rec.Header().Get("HX-Redirect"), rec.Header().Get("Location")) + } + + rec = httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "http://panel.example.com/overview", nil)) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/login" { + t.Errorf("a browser without a session: %d, Location %q", rec.Code, rec.Header().Get("Location")) + } +} diff --git a/internal/web/auth/middleware.go b/internal/web/auth/middleware.go index b3a53af..80f7877 100644 --- a/internal/web/auth/middleware.go +++ b/internal/web/auth/middleware.go @@ -11,17 +11,17 @@ func (m *Module) RequireAuth(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { token, ok := m.sessionToken(r) if !ok { - http.Redirect(w, r, "/login", http.StatusSeeOther) + toLogin(w, r) return } userID, ok := m.sessions.Lookup(token) if !ok { - http.Redirect(w, r, "/login", http.StatusSeeOther) + toLogin(w, r) return } u, err := m.store.GetUser(userID) if err != nil { - http.Redirect(w, r, "/login", http.StatusSeeOther) + toLogin(w, r) return } // Only a session whose user exists is kept alive by being used. @@ -34,6 +34,21 @@ func (m *Module) RequireAuth(next http.Handler) http.Handler { }) } +// toLogin sends a request without a session to the sign-in page. A browser +// gets a redirect; htmx gets HX-Redirect, which it follows with the whole +// page, before it looks at the status (htmx 2.0.4, src/htmx.js +// handleAjaxResponse(): location.href = the header's value). A 303 to htmx +// was followed inside the request, and the sign-in page swapped into the box +// that asked (audit 2026-10-12, M11). +func toLogin(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("HX-Request") != "" { + w.Header().Set("HX-Redirect", "/login") + w.WriteHeader(http.StatusUnauthorized) + return + } + http.Redirect(w, r, "/login", http.StatusSeeOther) +} + // isSessionActivity: a GET from htmx is a background poll - every panel hx-get // is a load or a poll - and does not renew the session. htmx 2.0.4, src/htmx.js // getHeaders(): every request it issues carries HX-Request: true (and diff --git a/internal/web/handlers/handlers_account.go b/internal/web/handlers/handlers_account.go index e6d698d..736a23b 100644 --- a/internal/web/handlers/handlers_account.go +++ b/internal/web/handlers/handlers_account.go @@ -141,7 +141,7 @@ func (h *Handlers) HandleAccountPassword(w http.ResponseWriter, r *http.Request) f.Err = msg h.renderAccount(w, r, status, u, f) } - if !h.auth.AllowLoginAttempt(r) { + if !h.auth.AllowPasswordChange(u.ID) { fail(http.StatusTooManyRequests, "Too many attempts. Please wait and try again.") return } diff --git a/internal/web/handlers/handlers_account_test.go b/internal/web/handlers/handlers_account_test.go index a285305..98bc80b 100644 --- a/internal/web/handlers/handlers_account_test.go +++ b/internal/web/handlers/handlers_account_test.go @@ -2,6 +2,7 @@ package handlers import ( "net/http" + "net/http/httptest" "net/url" "strings" "testing" @@ -197,3 +198,29 @@ func TestAccountPageForADomainAdministrator(t *testing.T) { } } } + +// The Account form's current-password check has its own bucket, per user: +// it stops a guesser at ten, and does not lock the address out of signing in +// (audit 2026-10-12, L15). +func TestAccountPasswordGuessesHaveTheirOwnLimit(t *testing.T) { + h, _ := settingsServer(t) + wrong := url.Values{"current_password": {"nope"}, "new_password": {"another-long-password-2"}, "new_password_confirm": {"another-long-password-2"}} + for i := 0; i < 10; i++ { + if rec := postFormAs(h.HandleAccountPassword, globalPrincipal, "/account/password", nil, wrong); rec.Code != http.StatusUnauthorized { + t.Fatalf("guess %d = %d, want 401", i+1, rec.Code) + } + } + if rec := postFormAs(h.HandleAccountPassword, globalPrincipal, "/account/password", nil, wrong); rec.Code != http.StatusTooManyRequests { + t.Fatalf("the eleventh guess = %d, want 429", rec.Code) + } + + // The same address may still sign in: postFormAs and this request come + // from httptest's one RemoteAddr. + req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(url.Values{"username": {"x"}, "password": {"y"}}.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + h.auth.HandleLogin(rec, req) + if rec.Code == http.StatusTooManyRequests { + t.Error("the password guesses locked the address out of signing in") + } +} diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index 7aa9554..a3f2179 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -107,6 +107,15 @@ func (h *Handlers) HandleBackup(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", filename)) w.Header().Set("Cache-Control", "no-store") + // The server's write deadline is for pages (cmd/panel newHTTPServer); an + // archive streams for as long as it takes. Two hours carry a gigabyte at + // a little over 1 Mbit/s, and still end a client that keeps the + // connection and stops reading, which TCP alone would not (net/http, Go + // 1.26: ResponseController.SetWriteDeadline). + if err := http.NewResponseController(w).SetWriteDeadline(time.Now().Add(2 * time.Hour)); err != nil { + logf("panel: full backup: lift the write deadline: %v", err) + } + // Everything below streams: past this point headers (and possibly some // bytes) are already on the wire, so a failure cannot switch to a clean // error page. Log it and let the truncated download fail loudly on the diff --git a/internal/web/view/paneljs_test.go b/internal/web/view/paneljs_test.go new file mode 100644 index 0000000..58eccb7 --- /dev/null +++ b/internal/web/view/paneljs_test.go @@ -0,0 +1,22 @@ +package view + +import ( + "os/exec" + "path/filepath" + "testing" +) + +// The refreshes of the monitoring boxes survive a failure and back off, +// checked by running panel.js in node against a stand-in for the DOM and +// htmx (testdata/poll_test.js; audit 2026-10-12, M11). Skipped where node is +// not installed - the test server's Go has none; the workstation runs it. +func TestPanelJSPolling(t *testing.T) { + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is not installed") + } + out, err := exec.Command(node, filepath.Join("testdata", "poll_test.js"), filepath.Join("static", "panel.js")).CombinedOutput() + if err != nil { + t.Fatalf("panel.js polling: %v\n%s", err, out) + } +} diff --git a/internal/web/view/static/panel.js b/internal/web/view/static/panel.js index adb616f..689ea55 100644 --- a/internal/web/view/static/panel.js +++ b/internal/web/view/static/panel.js @@ -221,6 +221,12 @@ // trigger is kept without its filter, so it would fire on every tick. var pollActiveMs = 5000; var pollIdleMs = 30000; + // A refresh that failed - the panel restarting, the network gone, an + // error answer - is tried again, later each time: the wait doubles with + // each failure in a row, up to a minute, and is back to normal after the + // first refresh that works (audit 2026-10-12, M11). + var pollMaxMs = 60000; + var pollFailures = Object.create(null); // No pointer/keyboard/scroll input for this long → treat the tab as idle. var userIdleMs = 30000; var lastActivity = Date.now(); @@ -232,12 +238,19 @@ }, { passive: true }); }); - function pollDelayMs() { - return Date.now() - lastActivity < userIdleMs ? pollActiveMs : pollIdleMs; + function pollDelayMs(id) { + var base = Date.now() - lastActivity < userIdleMs ? pollActiveMs : pollIdleMs; + var n = pollFailures[id] || 0; + return n ? Math.min(base * Math.pow(2, n), pollMaxMs) : base; } + // The box is the request's source, so htmx fires the request's events on + // it and not on the body (htmx 2.0.4, src/htmx.js ajaxHelper(): + // issueAjaxRequest(verb, path, resolveTarget(context.source), ...), and + // with no source issueAjaxRequest() falls back to the document's body). function triggerPoll(el) { htmx.ajax("GET", el.getAttribute("hx-get"), { + source: el, target: "#" + el.id, swap: el.getAttribute("hx-swap") || "outerHTML" }); @@ -266,11 +279,12 @@ return; } triggerPoll(current); - }, pollDelayMs()); + }, pollDelayMs(id)); } - function onPollElementReady(el) { - if (!el || !el.hasAttribute("data-poll")) { + document.body.addEventListener("htmx:afterSwap", function (ev) { + var el = ev.detail.elt; + if (!el || !el.hasAttribute || !el.hasAttribute("data-poll")) { return; } // Swapped-in markup still carries hx-trigger="load"; strip it so htmx does @@ -279,15 +293,31 @@ // elements before doSettle() runs the settle tasks, and only those // (makeAjaxLoadTask -> processNode) act on hx-trigger. el.removeAttribute("hx-trigger"); - schedulePoll(el); - } - - document.body.addEventListener("htmx:afterSwap", function (ev) { - onPollElementReady(ev.detail.elt); }); - document.body.addEventListener("htmx:responseError", function (ev) { - onPollElementReady(ev.detail.elt); + // The next refresh is scheduled when a request of the box ends, however + // it ended. htmx 2.0.4, src/htmx.js issueAjaxRequest(): htmx:afterRequest + // is fired on load, error, abort and timeout alike, and after a swap that + // took the box out of the page it is fired again on the nearest parent + // still in it - the event that reaches the body names the box by its + // target. A refresh that ended in an error answer is not successful + // (handleAjaxResponse(): responseInfo.successful = !isError); one cut off + // never got that far. + document.body.addEventListener("htmx:afterRequest", function (ev) { + var target = ev.detail.target; + if (!target || !target.id) { + return; + } + var el = document.getElementById(target.id); + if (!el || !el.hasAttribute("data-poll")) { + return; + } + if (ev.detail.successful) { + delete pollFailures[el.id]; + } else { + pollFailures[el.id] = (pollFailures[el.id] || 0) + 1; + } + schedulePoll(el); }); document.body.addEventListener("htmx:beforeRequest", function (ev) { diff --git a/internal/web/view/testdata/poll_test.js b/internal/web/view/testdata/poll_test.js new file mode 100644 index 0000000..4213215 --- /dev/null +++ b/internal/web/view/testdata/poll_test.js @@ -0,0 +1,148 @@ +// The polling of panel.js, run in node against a stand-in for the few parts +// of the DOM and htmx it uses, with a clock the test moves. Run by +// TestPanelJSPolling (paneljs_test.go): node poll_test.js . +// Exits non-zero with what went wrong. +"use strict"; +const fs = require("fs"); +const vm = require("vm"); + +const failures = []; +function check(cond, what) { + if (!cond) failures.push(what); +} + +// --- the clock ------------------------------------------------------------ +let now = 1000000; +let timers = []; +let nextTimer = 1; +function setTimeout(fn, ms) { + const id = nextTimer++; + timers.push({ id, at: now + ms, ms, fn }); + return id; +} +function clearTimeout(id) { + timers = timers.filter((t) => t.id !== id); +} +function pendingDelays() { + return timers.map((t) => t.ms); +} +function advance(ms) { + now += ms; + const due = timers.filter((t) => t.at <= now); + timers = timers.filter((t) => t.at > now); + due.forEach((t) => t.fn()); +} +const FakeDate = { now: () => now }; + +// --- the page --------------------------------------------------------------- +function element(id, attrs) { + return { + id, + attrs: Object.assign({}, attrs), + hasAttribute(n) { return n in this.attrs; }, + getAttribute(n) { return n in this.attrs ? this.attrs[n] : null; }, + removeAttribute(n) { delete this.attrs[n]; }, + }; +} +const listeners = { document: {}, body: {}, window: {} }; +function on(where) { + return (type, fn) => { + (listeners[where][type] = listeners[where][type] || []).push(fn); + }; +} +let inPage = {}; +const document = { + hidden: false, + addEventListener: on("document"), + body: { addEventListener: on("body") }, + getElementById: (id) => inPage[id] || null, + querySelectorAll: (sel) => (sel === "[data-poll]" ? Object.values(inPage).filter((e) => e.hasAttribute("data-poll")) : []), +}; +const window = { addEventListener: on("window"), confirm: () => true }; +const requests = []; +const htmx = { + ajax(verb, path, ctx) { + requests.push({ verb, path, ctx }); + }, +}; +// The operator moved the mouse: the page counts as in use for 30 s. +function activity() { + (listeners.document.mousemove || []).forEach((fn) => fn({})); +} +function fireBody(type, detail) { + (listeners.body[type] || []).forEach((fn) => fn({ type, detail, target: detail.elt })); +} + +vm.runInNewContext(fs.readFileSync(process.argv[2], "utf8"), { + document, window, htmx, setTimeout, clearTimeout, Date: FakeDate, + Object, Math, console, +}); + +// --- the cases -------------------------------------------------------------- +const box = element("overview-stats", { "data-poll": "", "hx-get": "/overview/stats", "hx-trigger": "load" }); +inPage = { [box.id]: box }; + +// The first load ended well: the next refresh in 5 s, as the box's source. +fireBody("htmx:afterRequest", { target: box, elt: box, successful: true }); +check(pendingDelays().join() === "5000", "after a refresh that worked: waits " + pendingDelays() + ", want 5000"); +advance(5000); +check(requests.length === 1, "no refresh was asked for after 5 s"); +check(requests[0] && requests[0].ctx.source === box, "the refresh does not name the box as its source"); +check(requests[0] && requests[0].ctx.target === "#overview-stats", "the refresh targets " + (requests[0] && requests[0].ctx.target)); + +// Failures in a row - an error answer, a dropped connection, a timeout - +// each double the wait, up to a minute; a refresh is always scheduled. +const waits = []; +for (let i = 0; i < 5; i++) { + activity(); + fireBody("htmx:afterRequest", { target: box, elt: box, successful: i === 0 ? false : undefined }); + waits.push(pendingDelays().join()); + advance(Number(pendingDelays()[0] || 0)); +} +check(waits.join(" ") === "10000 20000 40000 60000 60000", "the waits after failures in a row: " + waits.join(" ") + ", want 10000 20000 40000 60000 60000"); +check(requests.length === 6, "refreshes asked for: " + requests.length + ", want one per wait"); + +// One that works puts the wait back. +activity(); +fireBody("htmx:afterRequest", { target: box, elt: box, successful: true }); +check(pendingDelays().join() === "5000", "after failures and a success: waits " + pendingDelays() + ", want 5000"); + +// A swap replaced the box: the event that reaches the body comes from a +// parent and names the old box as its target; the new one is scheduled. +timers = []; +const fresh = element("overview-stats", { "data-poll": "", "hx-get": "/overview/stats", "hx-trigger": "load" }); +inPage = { [fresh.id]: fresh }; +fireBody("htmx:afterSwap", { elt: fresh }); +check(!fresh.hasAttribute("hx-trigger"), "the swapped-in box keeps hx-trigger"); +activity(); +fireBody("htmx:afterRequest", { target: box, elt: { id: "main" }, successful: true }); +check(pendingDelays().join() === "5000", "after a swap: waits " + pendingDelays() + ", want 5000"); +advance(5000); +check(requests[requests.length - 1].ctx.source === fresh, "the refresh after a swap is not the new box's"); + +// Idle for longer than 30 s: 30 s between refreshes, doubled from there. +timers = []; +now += 31000; +fireBody("htmx:afterRequest", { target: fresh, elt: fresh, successful: true }); +check(pendingDelays().join() === "30000", "idle: waits " + pendingDelays() + ", want 30000"); +fireBody("htmx:afterRequest", { target: fresh, elt: fresh, successful: false }); +check(pendingDelays().join() === "60000", "idle, one failure: waits " + pendingDelays() + ", want 60000"); + +// A hidden tab schedules nothing. +timers = []; +document.hidden = true; +fireBody("htmx:afterRequest", { target: fresh, elt: fresh, successful: true }); +check(timers.length === 0, "a hidden tab scheduled a refresh"); +document.hidden = false; + +// A request of something that is not a polled box is not its business. +timers = []; +inPage.other = element("other", {}); +fireBody("htmx:afterRequest", { target: inPage.other, elt: inPage.other, successful: true }); +check(timers.length === 0, "a request of a box that does not poll scheduled a refresh"); + +if (failures.length) { + console.log(failures.join("\n")); + process.exit(1); +} +console.log("ok"); diff --git a/test/e2e/restore_test.go b/test/e2e/restore_test.go index 49e2280..f86d56e 100644 --- a/test/e2e/restore_test.go +++ b/test/e2e/restore_test.go @@ -58,6 +58,11 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { if ct := resp.Header.Get("Content-Type"); resp.StatusCode != 200 || ct != "application/gzip" { t.Fatalf("full backup: status %d, Content-Type %q, want an application/gzip download:\n%.600s", resp.StatusCode, ct, archive) } + // The download moves the server's write deadline (handlers.HandleBackup); + // through the real server, unlike a test recorder, that is supported. + if l := logTail(h.logs("selfpost"), "write deadline"); l != "" { + t.Errorf("the backup could not move its write deadline:\n%s", l) + } // A download that broke off half way is still a 200: the panel streams the // archive and can only log what went wrong. Read it through once while the