From 1a50dd20d9159a78b1959eba04387940a3b48366 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 21:11:42 +0000 Subject: [PATCH] Test that login attempts are rate limited per client (closes #66) The tests build the server's real routes and log in as a browser does. The attempt after LoginAttemptsPerMinute failed logins from one client must get 429 with Retry-After; another client must still get the login form and log in with the signing key; two clients behind a trusted proxy must be counted separately; X-Forwarded-For from an untrusted peer must not get around the limit; an IPv6 client must be counted by its /64. They do not compile yet: LoginAttemptsPerMinute comes with the change. Model: opus-5-5 --- .../server/login_rate_limit_internal_test.go | 262 ++++++++++++++++++ 1 file changed, 262 insertions(+) create mode 100644 internal/server/login_rate_limit_internal_test.go diff --git a/internal/server/login_rate_limit_internal_test.go b/internal/server/login_rate_limit_internal_test.go new file mode 100644 index 0000000..721f148 --- /dev/null +++ b/internal/server/login_rate_limit_internal_test.go @@ -0,0 +1,262 @@ +package server + +import ( + "io" + "net/http" + "net/http/httptest" + "net/netip" + "net/url" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" + + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/handlers" + "sneak.berlin/go/pixa/internal/logger" + "sneak.berlin/go/pixa/internal/middleware" +) + +// testSigningKey is a throwaway signing key; submitting it logs in. +const testSigningKey = "test-signing-key-0123456789abcdef" + +// wrongKey is submitted for a failed login. +const wrongKey = "not-the-signing-key" + +// Addresses for the login rate limit tests. The test server trusts +// 10.0.0.0/8 as its proxies, so the X-Forwarded-For sent by proxyPeer is +// believed and the one sent by firstClient or secondClient is ignored. +const ( + firstClient = "198.51.100.1:40000" + secondClient = "198.51.100.2:40000" + proxyPeer = "10.0.0.1:40000" + firstForwarded = "203.0.113.1" + secondForwarded = "203.0.113.2" +) + +// csrfFieldPattern extracts the CSRF token rendered into the login form. +var csrfFieldPattern = regexp.MustCompile( + `name="gorilla\.csrf\.Token" value="([^"]+)"`) + +// newTestServer builds the server's real routes from the constructors +// cmd/pixad uses, with a throwaway state directory. Debug marks requests +// as plain HTTP, so the CSRF check runs without an https Referer. +func newTestServer(t *testing.T) *Server { + t.Helper() + + stateDir := t.TempDir() + cfg := &config.Config{ + Debug: true, + SigningKey: testSigningKey, + StateDir: stateDir, + DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"), + TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, + } + + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}}) + if err != nil { + t.Fatalf("logger.New() error = %v", err) + } + + db, err := database.New(lc, database.Params{Logger: log, Config: cfg}) + if err != nil { + t.Fatalf("database.New() error = %v", err) + } + + h, err := handlers.New(lc, handlers.Params{ + Logger: log, Database: db, Config: cfg, + }) + if err != nil { + t.Fatalf("handlers.New() error = %v", err) + } + + mw, err := middleware.New(lc, middleware.Params{Logger: log, Config: cfg}) + if err != nil { + t.Fatalf("middleware.New() error = %v", err) + } + + lc.RequireStart() + t.Cleanup(lc.RequireStop) + + s := &Server{config: cfg, mw: mw, h: h} + s.SetupRoutes() + + return s +} + +// clientRequest builds a request for / arriving from remoteAddr, carrying +// forwardedFor as its X-Forwarded-For header when that is not empty. +func clientRequest( + t *testing.T, method string, body io.Reader, remoteAddr, forwardedFor string, +) *http.Request { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), method, "/", body) + req.RemoteAddr = remoteAddr + + if forwardedFor != "" { + req.Header.Set("X-Forwarded-For", forwardedFor) + } + + return req +} + +// postLogin loads the login form with GET / and submits key in it with +// POST /, as a browser does, both from the same client. GET / is not rate +// limited, so the form must load even for a client over the limit. +func postLogin( + t *testing.T, s *Server, remoteAddr, forwardedFor, key string, +) *httptest.ResponseRecorder { + t.Helper() + + page := httptest.NewRecorder() + s.ServeHTTP(page, + clientRequest(t, http.MethodGet, nil, remoteAddr, forwardedFor)) + + if page.Code != http.StatusOK { + t.Fatalf("GET / status = %d, want %d", page.Code, http.StatusOK) + } + + match := csrfFieldPattern.FindStringSubmatch(page.Body.String()) + if match == nil { + t.Fatalf("no CSRF token field found in the login form") + } + + form := url.Values{"key": {key}, "gorilla.csrf.Token": {match[1]}} + req := clientRequest(t, http.MethodPost, + strings.NewReader(form.Encode()), remoteAddr, forwardedFor) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + for _, c := range page.Result().Cookies() { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, req) + + return rec +} + +// tripLoginRateLimit makes LoginAttemptsPerMinute failed logins from one +// client, each answered with the login form again, then one more, which +// must be refused with 429. It returns the response to that last attempt. +func tripLoginRateLimit( + t *testing.T, s *Server, remoteAddr, forwardedFor string, +) *httptest.ResponseRecorder { + t.Helper() + + for attempt := range LoginAttemptsPerMinute { + rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey) + if rec.Code != http.StatusOK { + t.Fatalf("failed login %d status = %d, want %d", + attempt+1, rec.Code, http.StatusOK) + } + } + + rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey) + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("login over the limit status = %d, want %d", + rec.Code, http.StatusTooManyRequests) + } + + return rec +} + +// TestLoginRateLimitRefusesAttemptOverLimit verifies the login attempt +// after LoginAttemptsPerMinute failed ones from one client is refused with +// 429 and a Retry-After header, and that the client cannot get around the +// limit by sending X-Forwarded-For: from a peer that is not a trusted +// proxy, the header is ignored. +func TestLoginRateLimitRefusesAttemptOverLimit(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + + rec := tripLoginRateLimit(t, s, firstClient, "") + + retryAfter := rec.Header().Get("Retry-After") + + seconds, err := strconv.Atoi(retryAfter) + if err != nil || seconds <= 0 { + t.Errorf("Retry-After = %q, want a positive number of seconds", + retryAfter) + } + + rec = postLogin(t, s, firstClient, secondForwarded, wrongKey) + if rec.Code != http.StatusTooManyRequests { + t.Errorf("login with X-Forwarded-For from an untrusted peer "+ + "status = %d, want %d", rec.Code, http.StatusTooManyRequests) + } +} + +// TestLoginRateLimitLeavesOtherClientsAlone verifies one client going over +// the limit does not limit another: a failed login from a different +// address is answered with the login form, and the signing key still logs +// it in. +func TestLoginRateLimitLeavesOtherClientsAlone(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + + tripLoginRateLimit(t, s, firstClient, "") + + rec := postLogin(t, s, secondClient, "", wrongKey) + if rec.Code != http.StatusOK { + t.Errorf("failed login from another client status = %d, want %d", + rec.Code, http.StatusOK) + } + + rec = postLogin(t, s, secondClient, "", testSigningKey) + if rec.Code != http.StatusSeeOther { + t.Errorf("login with the signing key from another client "+ + "status = %d, want %d", rec.Code, http.StatusSeeOther) + } +} + +// TestLoginRateLimitCountsClientsBehindProxySeparately verifies the limit +// counts the client address resolved from X-Forwarded-For, not the address +// of the trusted proxy the requests arrive from, so two clients behind the +// same proxy are counted separately. +func TestLoginRateLimitCountsClientsBehindProxySeparately(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + + tripLoginRateLimit(t, s, proxyPeer, firstForwarded) + + rec := postLogin(t, s, proxyPeer, secondForwarded, wrongKey) + if rec.Code != http.StatusOK { + t.Errorf("failed login from a second client behind the proxy "+ + "status = %d, want %d", rec.Code, http.StatusOK) + } +} + +// TestLoginRateLimitCountsIPv6ClientsByPrefix verifies an IPv6 client is +// counted by its /64: another address in the same /64 is refused too, +// while an address in a different /64 is not. +func TestLoginRateLimitCountsIPv6ClientsByPrefix(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + + tripLoginRateLimit(t, s, proxyPeer, "2001:db8::1") + + rec := postLogin(t, s, proxyPeer, "2001:db8::2", wrongKey) + if rec.Code != http.StatusTooManyRequests { + t.Errorf("login from the same /64 status = %d, want %d", + rec.Code, http.StatusTooManyRequests) + } + + rec = postLogin(t, s, proxyPeer, "2001:db8:0:1::1", wrongKey) + if rec.Code != http.StatusOK { + t.Errorf("login from another /64 status = %d, want %d", + rec.Code, http.StatusOK) + } +}