From e6af1a7866b0094be38c888cb2a12a91288d0dae Mon Sep 17 00:00:00 2001 From: Mikhail Yenuchenko Date: Thu, 13 Aug 2026 23:35:46 +0300 Subject: [PATCH] test: cover the auth, sign-in and RBAC surfaces P0 shipped through (P4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - internal/web/auth/ratelimit_test.go: the login/setup limiter's ceiling, per-address scope, window reset and the sweep that keeps finished buckets out of memory. - internal/web/auth/handlers_test.go: sign-in (session issued, refusals that do not reveal which usernames exist, a lockout a correct password cannot bypass, the setup hint before the first administrator) and the one-time setup link (creates the first global administrator and then closes, rejects a wrong or expired token, refuses credentials the panel would not accept later, rate-limited). - internal/web/handlers/authz_test.go: every global-only route answers a domain administrator — and a request with no principal — with 404, with a positive control so the table cannot pass on a handler that always 404s. This is the check that would have caught the send-log leak. - test/e2e: CoreDNS pinned to 1.14.6 instead of latest; the level-1 failure message quoted RATE_LIMIT_MESSAGES_PER_IP=5 while the stand sets 50. docs/plans/code-review.md P4 checked off except the optional backup-boot test; CHANGELOG updated. No production code changed. Co-Authored-By: Claude Opus 5 Co-authored-by: Cursor --- CHANGELOG.md | 17 ++ docs/plans/code-review.md | 19 +- internal/web/auth/handlers_test.go | 284 ++++++++++++++++++++++++++++ internal/web/auth/ratelimit_test.go | 100 ++++++++++ internal/web/handlers/authz_test.go | 125 ++++++++++++ test/e2e/compose.override.yml | 7 +- test/e2e/negative_test.go | 2 +- 7 files changed, 546 insertions(+), 8 deletions(-) create mode 100644 internal/web/auth/handlers_test.go create mode 100644 internal/web/auth/ratelimit_test.go create mode 100644 internal/web/handlers/authz_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 56662b2..ef820a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,23 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Changed +- test: the authorization and sign-in surfaces that had no tests now have them. + The login limiter is covered for its ceiling, its per-address scope, the reset + at the end of a window and the sweep that keeps finished buckets out of + memory; sign-in for a successful session, for refusals that do not reveal + which usernames exist, and for a lockout that a correct password cannot + bypass; the one-time setup link for creating the first administrator, closing + afterwards, rejecting a wrong or expired token, and refusing credentials the + panel would not accept later. Every global-only route (`/users`, `/backup`, + domain import, `/status`, `/mail-queue`, `/system-log`, domain add and delete, + reload) is checked to answer a domain administrator — and a request with no + principal — with 404, the check that would have caught the send-log leak. + +- test (e2e): the CoreDNS image is pinned to `1.14.6` instead of `latest`, so + the release gate cannot change under a commit between two runs. The level-1 + rate-limit failure message quoted `RATE_LIMIT_MESSAGES_PER_IP=5` while the + stand sets `50`. + - ci: gofmt on eight files that failed the formatting workflow check (panel config, DNS check, domain transfer export, rate-limit tests, auth principal, domain and delivery handlers, web package doc comment). diff --git a/docs/plans/code-review.md b/docs/plans/code-review.md index 04d3f9c..63c54c3 100644 --- a/docs/plans/code-review.md +++ b/docs/plans/code-review.md @@ -562,12 +562,19 @@ cannot be a single unmarked click. **Model: Opus** for auth/RBAC/limiter tests; **Haiku** for the L1 fatal-string typo; **Sonnet** if e2e docs need a paragraph. -- [ ] `auth/ratelimit.go` unit tests (window, lockout, sweep). -- [ ] Login/setup handler tests (happy path + lockout + setup expiry). -- [ ] Domain-admin 404 on `/users`, `/backup`, `/mail-queue`, `/system-log`, - `/status` (if not already implied by P0 tests). -- [ ] Fix e2e L1 fatal string (`50`, not `5`). -- [ ] Pin `coredns` image digest or a major version. +- [x] `auth/ratelimit.go` unit tests (window, lockout, sweep). Also the + per-key scope: one locked-out address must not lock out the others. +- [x] Login/setup handler tests (happy path + lockout + setup expiry). The + lockout test also states that a correct password does not bypass it, and + that the two refusals are byte-identical (no username enumeration). +- [x] Domain-admin 404 on `/users`, `/backup`, `/mail-queue`, `/system-log`, + `/status` — as a table of every global-only route (`internal/web/handlers/authz_test.go`), + including the write routes, plus the same 404 for a request with no + principal and a positive control so the table cannot pass on a handler + that always 404s. +- [x] Fix e2e L1 fatal string (`50`, not `5`). +- [x] Pin `coredns` image: tag `1.14.6`, not a digest — the tag is a multi-arch + manifest and the stand has to come up on arm64 developer machines. - [ ] Optional later: backup extract + `CheckRestore` + panel boot (heavy; e2e or integration). Not a P4 blocker. diff --git a/internal/web/auth/handlers_test.go b/internal/web/auth/handlers_test.go new file mode 100644 index 0000000..7bfa250 --- /dev/null +++ b/internal/web/auth/handlers_test.go @@ -0,0 +1,284 @@ +package auth + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "golang.org/x/crypto/bcrypt" +) + +const testPassword = "correct-horse-battery" + +// moduleWithAdmin returns a panel that has already been through setup, with one +// global administrator whose password is testPassword. +func moduleWithAdmin(t *testing.T) *Module { + t.Helper() + m := testModule(t, false) + hash, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.MinCost) + if err != nil { + t.Fatalf("hash password: %v", err) + } + if err := m.store.CreateGlobalUser("admin", string(hash)); err != nil { + t.Fatalf("create user: %v", err) + } + return m +} + +// postLogin submits the sign-in form from remoteAddr (the limiter's key) and +// returns what the handler wrote. +func postLogin(m *Module, remoteAddr, username, password string) *httptest.ResponseRecorder { + form := url.Values{"username": {username}, "password": {password}} + 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.RemoteAddr = remoteAddr + rec := httptest.NewRecorder() + m.HandleLogin(rec, r) + return rec +} + +// sessionCookieValue returns the session token the response issued, or "" if it +// issued none. +func sessionCookieValue(t *testing.T, m *Module, rec *httptest.ResponseRecorder) string { + t.Helper() + for _, c := range rec.Result().Cookies() { + if c.Name == m.sessionCookie() { + return c.Value + } + } + return "" +} + +func TestLoginSignsInWithTheRightPassword(t *testing.T) { + m := moduleWithAdmin(t) + + rec := postLogin(m, "203.0.113.7:5000", "admin", testPassword) + + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" { + t.Fatalf("status = %d, Location = %q; want a redirect to /", rec.Code, rec.Header().Get("Location")) + } + token := sessionCookieValue(t, m, rec) + if token == "" { + t.Fatal("no session cookie was issued") + } + name, ok := m.sessions.Lookup(token) + if !ok || name != "admin" { + t.Fatalf("the cookie's session resolves to %q, %t; want admin", name, ok) + } +} + +// A refused sign-in must not say which half was wrong: the panel is public, and +// distinguishable answers would turn the form into a list of usernames. +func TestLoginRefusesBadCredentialsWithoutSayingWhy(t *testing.T) { + m := moduleWithAdmin(t) + + bodies := make(map[string]string, 2) + for name, creds := range map[string][2]string{ + "wrong password": {"admin", "not-the-password"}, + "unknown user": {"nobody", testPassword}, + } { + rec := postLogin(m, "203.0.113.7:5000", creds[0], creds[1]) + if rec.Code != http.StatusUnauthorized { + t.Errorf("%s: status = %d, want 401", name, rec.Code) + } + if got := sessionCookieValue(t, m, rec); got != "" { + t.Errorf("%s: a session cookie was issued: %q", name, got) + } + bodies[name] = rec.Body.String() + } + if bodies["wrong password"] != bodies["unknown user"] { + t.Error("the two refusals differ, so the form tells an attacker which usernames exist") + } +} + +// The lockout is what makes online guessing pointless, so it has to hold even +// for the request that finally carries the right password — and it has to be +// scoped to the address that spent the attempts. +func TestLoginLocksOutAfterTooManyAttempts(t *testing.T) { + m := moduleWithAdmin(t) + const attacker = "203.0.113.7:5000" + + for i := 0; i < 10; i++ { + if rec := postLogin(m, attacker, "admin", "guess"); rec.Code != http.StatusUnauthorized { + t.Fatalf("attempt %d: status = %d, want 401 (still under the limit)", i+1, rec.Code) + } + } + + rec := postLogin(m, attacker, "admin", testPassword) + if rec.Code != http.StatusTooManyRequests { + t.Errorf("status = %d, want 429; the lockout was bypassed by guessing right", rec.Code) + } + if got := sessionCookieValue(t, m, rec); got != "" { + t.Errorf("a locked-out request was signed in: %q", got) + } + + if rec := postLogin(m, "198.51.100.9:5000", "admin", testPassword); rec.Code != http.StatusSeeOther { + t.Errorf("another address got %d; one guesser locked out the whole internet", rec.Code) + } +} + +// Before the first administrator exists there is nothing to sign in as, so the +// form is replaced by a pointer to the setup link rather than a password box +// that can never succeed. +func TestLoginPointsAtSetupBeforeTheFirstAdministrator(t *testing.T) { + m := testModule(t, false) + + rec := httptest.NewRecorder() + m.HandleLogin(rec, httptest.NewRequest(http.MethodGet, "http://panel.example.com/login", nil)) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + body := rec.Body.String() + if !strings.Contains(body, "No administrator has been created yet") { + t.Errorf("the login page does not point at the setup link:\n%s", body) + } + if strings.Contains(body, `name="password"`) { + t.Errorf("the login page offers a password field with no account to use it:\n%s", body) + } +} + +// getSetup performs the GET the operator's browser makes when it follows the +// one-time link. +func getSetup(m *Module, token string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + m.HandleSetup(rec, httptest.NewRequest(http.MethodGet, "http://panel.example.com/setup/"+token, nil)) + return rec +} + +func postSetup(m *Module, token string, form url.Values) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodPost, "http://panel.example.com/setup/"+token, + strings.NewReader(form.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + m.HandleSetup(rec, r) + return rec +} + +func setupForm(username, password, confirm string) url.Values { + return url.Values{ + "username": {username}, + "password": {password}, + "password_confirm": {confirm}, + } +} + +// The setup link creates the first global administrator and then stops +// existing: the persistent fact is the user row, so the link is dead after a +// restart too, not only for the process that served it. +func TestSetupCreatesTheFirstAdministratorAndThenCloses(t *testing.T) { + m := testModule(t, false) + token, ok := m.setup.activeToken() + if !ok { + t.Fatal("no setup token on a panel with no users") + } + + if rec := getSetup(m, token); rec.Code != http.StatusOK { + t.Fatalf("GET the setup link = %d, want the form", rec.Code) + } + + rec := postSetup(m, token, setupForm("operator", "a-long-enough-password", "a-long-enough-password")) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/login" { + t.Fatalf("status = %d, Location = %q; want a redirect to /login", rec.Code, rec.Header().Get("Location")) + } + + u, err := m.store.GetUserByUsername("operator") + if err != nil { + t.Fatalf("the administrator was not created: %v", err) + } + if u.Role != RoleGlobal { + t.Errorf("the first administrator has role %q, want global", u.Role) + } + if err := bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte("a-long-enough-password")); err != nil { + t.Errorf("the stored hash does not match the password that was set: %v", err) + } + + if rec := getSetup(m, token); rec.Code != http.StatusNotFound { + t.Errorf("the setup link still answers %d after setup completed, want 404", rec.Code) + } + if rec := postSetup(m, token, setupForm("second", "a-long-enough-password", "a-long-enough-password")); rec.Code != http.StatusNotFound { + t.Errorf("a second administrator could be created through the setup link (%d)", rec.Code) + } +} + +// A token that is wrong, or one that has aged out and been replaced, is not a +// hint that setup exists: both answer 404, the same as any unknown path. +func TestSetupRejectsAWrongOrExpiredToken(t *testing.T) { + m := testModule(t, false) + token, ok := m.setup.activeToken() + if !ok { + t.Fatal("no setup token on a panel with no users") + } + + if rec := getSetup(m, token+"x"); rec.Code != http.StatusNotFound { + t.Errorf("a wrong token answered %d, want 404", rec.Code) + } + + expireSetupToken(m) + + if rec := getSetup(m, token); rec.Code != http.StatusNotFound { + t.Errorf("the expired token still opens setup (%d)", rec.Code) + } + fresh, _ := m.setup.activeToken() + if fresh == token { + t.Fatal("the expired token was not replaced") + } + if rec := getSetup(m, fresh); rec.Code != http.StatusOK { + t.Errorf("the reissued token does not open setup (%d)", rec.Code) + } +} + +// The first account is the one that can never be locked out of the panel from +// outside, so the rules that apply to every other user apply here too — before +// anything is written. +func TestSetupRejectsCredentialsItWouldNotAcceptLater(t *testing.T) { + for name, form := range map[string]url.Values{ + "username too short": setupForm("op", "a-long-enough-password", "a-long-enough-password"), + "username not ASCII": setupForm("оператор", "a-long-enough-password", "a-long-enough-password"), + "passwords differ": setupForm("operator", "a-long-enough-password", "a-long-enough-passwerd"), + "password too short": setupForm("operator", "short", "short"), + "no password at all": setupForm("operator", "", ""), + "no username at all": setupForm("", "a-long-enough-password", "a-long-enough-password"), + } { + m := testModule(t, false) + token, _ := m.setup.activeToken() + + rec := postSetup(m, token, form) + + if rec.Code != http.StatusBadRequest { + t.Errorf("%s: status = %d, want 400", name, rec.Code) + } + if exists, err := m.store.UserExists(); err != nil || exists { + t.Errorf("%s: an administrator was created anyway (err=%v)", name, err) + } + if rec := getSetup(m, token); rec.Code != http.StatusOK { + t.Errorf("%s: the setup link was burned by a rejected form (%d)", name, rec.Code) + } + } +} + +// Setup is unauthenticated by definition, so the only thing between the token +// and an offline guesser is the limiter in front of it. +func TestSetupIsRateLimited(t *testing.T) { + m := testModule(t, false) + + for i := 0; i < 10; i++ { + if rec := getSetup(m, "wrong-token"); rec.Code != http.StatusNotFound { + t.Fatalf("attempt %d: status = %d, want 404 (still under the limit)", i+1, rec.Code) + } + } + if rec := getSetup(m, "wrong-token"); rec.Code != http.StatusTooManyRequests { + t.Errorf("status = %d, want 429 after the eleventh attempt", rec.Code) + } +} + +// expireSetupToken ages the current token out, the state the panel reaches when +// nobody follows the link within setupTokenTTL. +func expireSetupToken(m *Module) { + m.setup.mu.Lock() + defer m.setup.mu.Unlock() + m.setup.expiresAt = m.setup.expiresAt.Add(-2 * setupTokenTTL) +} diff --git a/internal/web/auth/ratelimit_test.go b/internal/web/auth/ratelimit_test.go new file mode 100644 index 0000000..86a7f32 --- /dev/null +++ b/internal/web/auth/ratelimit_test.go @@ -0,0 +1,100 @@ +package auth + +import ( + "testing" + "time" +) + +// The limiter is what stands between the public login form and an unlimited +// guessing rate (security.md), so the ceiling has to be exact: the configured +// number of attempts go through and the next one does not, however often it is +// repeated. +func TestRateLimiterStopsAtTheCeiling(t *testing.T) { + r := newRateLimiter(3, time.Minute) + + for i := 1; i <= 3; i++ { + if !r.Allow("203.0.113.7") { + t.Fatalf("attempt %d of 3 was refused before the ceiling", i) + } + } + for i := 4; i <= 6; i++ { + if r.Allow("203.0.113.7") { + t.Fatalf("attempt %d passed after the ceiling of 3", i) + } + } +} + +// Buckets are per key, so one locked-out address must not lock out the rest of +// the internet — a shared counter would turn a single guesser into a denial of +// service against every operator. +func TestRateLimiterKeepsKeysApart(t *testing.T) { + r := newRateLimiter(1, time.Minute) + + if !r.Allow("203.0.113.7") || r.Allow("203.0.113.7") { + t.Fatal("the first key did not use up its single attempt") + } + if !r.Allow("198.51.100.9") { + t.Fatal("a second address was refused because another one was locked out") + } +} + +// The window is fixed, not sliding: once it has elapsed the count starts again +// from zero rather than being carried over. Time is moved by ageing the bucket +// instead of sleeping, so the test states the boundary rather than approaching +// it. +func TestRateLimiterReopensAfterTheWindow(t *testing.T) { + r := newRateLimiter(2, time.Minute) + r.Allow("203.0.113.7") + r.Allow("203.0.113.7") + if r.Allow("203.0.113.7") { + t.Fatal("the ceiling was not reached") + } + + expire(r, "203.0.113.7") + + if !r.Allow("203.0.113.7") { + t.Fatal("the key is still locked out after its window ended") + } + if !r.Allow("203.0.113.7") { + t.Fatal("the new window did not start from an empty count") + } + if r.Allow("203.0.113.7") { + t.Fatal("the new window allowed more than the ceiling") + } +} + +// Every address that ever tried to sign in gets a bucket, and the only thing +// that removes the finished ones is the sweep on a new window. It runs on the +// key that triggered it as well as on the others, so a long-running panel does +// not accumulate a bucket per source address for ever. +func TestRateLimiterSweepsFinishedBuckets(t *testing.T) { + r := newRateLimiter(2, time.Minute) + for _, key := range []string{"203.0.113.7", "198.51.100.9"} { + r.Allow(key) + expire(r, key) + } + r.Allow("192.0.2.5") // still inside its window + + r.Allow("203.0.113.7") // new window for this key: sweeps the rest + + r.mu.Lock() + defer r.mu.Unlock() + if _, ok := r.buckets["198.51.100.9"]; ok { + t.Error("a bucket whose window ended is still held") + } + for _, key := range []string{"203.0.113.7", "192.0.2.5"} { + if _, ok := r.buckets[key]; !ok { + t.Errorf("the sweep dropped %s, whose window is still open", key) + } + } +} + +// expire moves a key's window into the past, the same state it would reach by +// waiting for the window to elapse. +func expire(r *rateLimiter, key string) { + r.mu.Lock() + defer r.mu.Unlock() + if b := r.buckets[key]; b != nil { + b.windowEnds = time.Now().Add(-time.Second) + } +} diff --git a/internal/web/handlers/authz_test.go b/internal/web/handlers/authz_test.go new file mode 100644 index 0000000..d1404f2 --- /dev/null +++ b/internal/web/handlers/authz_test.go @@ -0,0 +1,125 @@ +package handlers + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/mixeme/selfpost/internal/web/auth" +) + +// route is one entry of the authenticated mux, named the way web.go registers +// it so a route added there without a guard is visible as a missing case here. +type route struct { + method string + target string + handler func(*Handlers) http.HandlerFunc + // pathValues are the {id}-style segments the router would have bound. + pathValues map[string]string +} + +// globalOnlyRoutes is every page and action that only a global administrator +// may reach: the panel's users, the whole-server backup and domain import, the +// machine-wide status and log views, and the domain lifecycle. A domain +// administrator is answered 404 rather than 403 so the panel does not confirm +// that the page exists (security.md). +var globalOnlyRoutes = []route{ + {"GET", "/users", func(h *Handlers) http.HandlerFunc { return h.HandleUsers }, nil}, + {"GET", "/users/new", func(h *Handlers) http.HandlerFunc { return h.HandleUserNew }, nil}, + {"POST", "/users/new", func(h *Handlers) http.HandlerFunc { return h.HandleUserNew }, nil}, + {"GET", "/users/1", func(h *Handlers) http.HandlerFunc { return h.HandleUserEdit }, map[string]string{"uid": "1"}}, + {"POST", "/users/1", func(h *Handlers) http.HandlerFunc { return h.HandleUserEdit }, map[string]string{"uid": "1"}}, + {"GET", "/users/1/delete", func(h *Handlers) http.HandlerFunc { return h.HandleUserDeleteConfirm }, map[string]string{"uid": "1"}}, + {"POST", "/users/1/delete", func(h *Handlers) http.HandlerFunc { return h.HandleUserDelete }, map[string]string{"uid": "1"}}, + + {"GET", "/backup", func(h *Handlers) http.HandlerFunc { return h.HandleBackupPage }, nil}, + {"POST", "/backup", func(h *Handlers) http.HandlerFunc { return h.HandleBackup }, nil}, + {"POST", "/domains/import", func(h *Handlers) http.HandlerFunc { return h.HandleImportDomain }, nil}, + + {"GET", "/status", func(h *Handlers) http.HandlerFunc { return h.HandleStatus }, nil}, + {"GET", "/status/fragment", func(h *Handlers) http.HandlerFunc { return h.HandleStatusFragment }, nil}, + {"POST", "/status/recheck", func(h *Handlers) http.HandlerFunc { return h.HandleStatusRecheck }, nil}, + + {"GET", "/mail-queue", func(h *Handlers) http.HandlerFunc { return h.HandleMailQueue }, nil}, + {"GET", "/mail-queue/body", func(h *Handlers) http.HandlerFunc { return h.HandleMailQueueBody }, nil}, + {"GET", "/system-log", func(h *Handlers) http.HandlerFunc { return h.HandleSystemLog }, nil}, + {"GET", "/system-log/body", func(h *Handlers) http.HandlerFunc { return h.HandleSystemLogBody }, nil}, + + {"POST", "/domains", func(h *Handlers) http.HandlerFunc { return h.HandleAddDomain }, nil}, + {"GET", "/domains/1/delete", func(h *Handlers) http.HandlerFunc { return h.HandleDeleteConfirm }, map[string]string{"id": "1"}}, + {"POST", "/domains/1/delete", func(h *Handlers) http.HandlerFunc { return h.HandleDeleteDomain }, map[string]string{"id": "1"}}, + {"POST", "/reload", func(h *Handlers) http.HandlerFunc { return h.HandleReload }, nil}, +} + +// A domain administrator has an account on the panel, so authentication is not +// what keeps them off these pages — the per-handler role check is. Each of them +// is reached here with a valid session for a principal that owns a domain, the +// case the send-log leak (P0, code-review.md) showed is easy to get wrong. +func TestGlobalOnlyRoutesAnswerADomainAdmin404(t *testing.T) { + h, domains := serverWithTwoDomains(t) + p := domainAdmin(domains["first.example.ru"].ID) + + for _, rt := range globalOnlyRoutes { + rec := call(h, rt, p) + if rec.Code != http.StatusNotFound { + t.Errorf("%s %s as a domain administrator = %d, want 404:\n%s", + rt.method, rt.target, rec.Code, rec.Body.String()) + } + } +} + +// The same 404 covers a request that carries no principal at all: the auth +// middleware normally redirects those, but a handler must not depend on +// middleware it cannot see for the role it enforces itself. +func TestGlobalOnlyRoutesAnswerAnUnknownPrincipal404(t *testing.T) { + h, _ := serverWithTwoDomains(t) + + for _, rt := range globalOnlyRoutes { + rec := httptest.NewRecorder() + req := httptest.NewRequest(rt.method, rt.target, nil) + for k, v := range rt.pathValues { + req.SetPathValue(k, v) + } + rt.handler(h)(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("%s %s with no principal = %d, want 404", rt.method, rt.target, rec.Code) + } + } +} + +// The 404s above would also pass if a handler were broken into always returning +// one, so at least the two pages that need nothing but the store and the view +// have to be shown opening for a global administrator. +func TestGlobalOnlyRoutesOpenForAGlobalAdministrator(t *testing.T) { + h, _ := serverWithTwoDomains(t) + + for _, target := range []string{"/users", "/backup"} { + rt := getRoute(t, target) + if rec := call(h, rt, globalPrincipal); rec.Code != http.StatusOK { + t.Errorf("GET %s as a global administrator = %d, want 200:\n%s", + target, rec.Code, rec.Body.String()) + } + } +} + +func getRoute(t *testing.T, target string) route { + t.Helper() + for _, rt := range globalOnlyRoutes { + if rt.method == http.MethodGet && rt.target == target { + return rt + } + } + t.Fatalf("no GET %s among the global-only routes", target) + return route{} +} + +func call(h *Handlers, rt route, p auth.Principal) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + req := httptest.NewRequest(rt.method, rt.target, nil) + req = auth.RequestWithPrincipal(req, p) + for k, v := range rt.pathValues { + req.SetPathValue(k, v) + } + rt.handler(h)(rec, req) + return rec +} diff --git a/test/e2e/compose.override.yml b/test/e2e/compose.override.yml index 6b5c38c..79a38fc 100644 --- a/test/e2e/compose.override.yml +++ b/test/e2e/compose.override.yml @@ -60,8 +60,13 @@ services: # records the test harness publishes, so Postfix's outbound MX lookup for # the sink and the panel's own dnscheck resolve inside the isolated network # instead of hitting the real internet. + # + # Pinned to a release tag rather than `latest`, so a CoreDNS upload cannot + # change what the release gate runs against between two runs of the same + # commit. Not a digest: the tag is a multi-arch manifest and the stand has to + # come up on arm64 developer machines as well as amd64 CI. coredns: - image: coredns/coredns:latest + image: coredns/coredns:1.14.6 command: ["-conf", "/dns/Corefile"] volumes: - ./dns-stage:/dns diff --git a/test/e2e/negative_test.go b/test/e2e/negative_test.go index 4f9be0a..eb5ed6e 100644 --- a/test/e2e/negative_test.go +++ b/test/e2e/negative_test.go @@ -219,5 +219,5 @@ func testLevel1RateLimit(t *testing.T, sc *scenario) { return } } - t.Fatalf("level-1 rate limit (RATE_LIMIT_MESSAGES_PER_IP=5) never tripped after %d sends", maxAttempts) + t.Fatalf("level-1 rate limit (RATE_LIMIT_MESSAGES_PER_IP=50) never tripped after %d sends", maxAttempts) }