diff --git a/backend/mw/ratelimit_test.go b/backend/mw/ratelimit_test.go index cccd592..223947d 100644 --- a/backend/mw/ratelimit_test.go +++ b/backend/mw/ratelimit_test.go @@ -652,7 +652,180 @@ func TestProgressiveRateLimiter_DelayEscalatesWithSustainedRate(t *testing.T) { } } -// TestProgressiveRateLimit_RejectsOnlyTopTier pins finding 4a + Round 2 Loop B +// ============================================================ +// TRUST_PROXY_HEADERS mode switching on the per-IP and per-user limiters (M24-prep) +// ============================================================ + +// serveRateLimitRequestWithCF serves a request with an explicit RemoteAddr and +// a client-supplied CF-Connecting-IP header, returning the recorder. +func serveRateLimitRequestWithCF(t *testing.T, h http.Handler, remoteAddr, cfHeader string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = remoteAddr + req.Header.Set("CF-Connecting-IP", cfHeader) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + return w +} + +// serveRateLimitUserRequestWithCF serves a RateLimitByUserAndIP/BYUser-wrapped +// request with an optional authenticated userID, an explicit RemoteAddr, and a +// client-supplied CF-Connecting-IP header. +func serveRateLimitUserRequestWithCF(t *testing.T, h http.Handler, userID, remoteAddr, cfHeader string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = remoteAddr + req.Header.Set("CF-Connecting-IP", cfHeader) + if userID != "" { + req = req.WithContext(context.WithValue(req.Context(), UserIDKey, userID)) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + return w +} + +// TestRateLimitMiddleware_TrustedHeaderBecomesKey verifies TRUST_PROXY_HEADERS=true +// on the per-IP limiter: the rate-limit key is the trusted CF-Connecting-IP +// header (the proxy overwrote it with the real client IP), so two clients behind +// the SAME proxy address get independent buckets — without this, every client +// collapses onto the proxy's RemoteAddr and one client could exhaust the shared +// per-IP budget for everyone. The key is the header, not the proxy address: the +// same client stays exhausted across proxy addresses. +func TestRateLimitMiddleware_TrustedHeaderBecomesKey(t *testing.T) { + setTrustProxyHeaders(t, true) + handler, calls := newRateLimitTestHandler(1, time.Minute) + + if w := serveRateLimitRequestWithCF(t, handler, "10.0.0.1:1234", "198.51.100.10"); w.Code != http.StatusOK { + t.Fatalf("client A first request: expected 200, got %d", w.Code) + } + if w := serveRateLimitRequestWithCF(t, handler, "10.0.0.1:1234", "198.51.100.10"); w.Code != http.StatusTooManyRequests { + t.Errorf("client A second request: expected 429 (own bucket exhausted), got %d", w.Code) + } + if w := serveRateLimitRequestWithCF(t, handler, "10.0.0.1:1234", "198.51.100.11"); w.Code != http.StatusOK { + t.Errorf("client B behind the same proxy must keep an independent bucket, got %d", w.Code) + } + // The same client from a different proxy address shares ONE bucket — the + // trusted header, not the proxy address, is the key. + if w := serveRateLimitRequestWithCF(t, handler, "10.0.0.2:1234", "198.51.100.10"); w.Code != http.StatusTooManyRequests { + t.Errorf("client A must stay exhausted across proxy addresses, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls (A and B first hits), got %d", *calls) + } +} + +// TestRateLimitMiddleware_UntrustedHeaderIgnored verifies the origin-exposed +// default (TRUST_PROXY_HEADERS=false) on the per-IP limiter: a client-supplied +// CF-Connecting-IP must NOT become the rate-limit key, or an origin-exposed +// client could rotate the header to mint a fresh bucket per request and bypass +// per-IP rate limiting. The key is the real TCP peer (RemoteAddr). +func TestRateLimitMiddleware_UntrustedHeaderIgnored(t *testing.T) { + setTrustProxyHeaders(t, false) + handler, calls := newRateLimitTestHandler(1, time.Minute) + + if w := serveRateLimitRequestWithCF(t, handler, "192.0.2.50:1234", "203.0.113.99"); w.Code != http.StatusOK { + t.Fatalf("first request: expected 200, got %d", w.Code) + } + if w := serveRateLimitRequestWithCF(t, handler, "192.0.2.50:1234", "203.0.113.100"); w.Code != http.StatusTooManyRequests { + t.Errorf("rotating the spoofed header must NOT mint a fresh bucket, got %d", w.Code) + } + if w := serveRateLimitRequestWithCF(t, handler, "192.0.2.51:1234", "203.0.113.99"); w.Code != http.StatusOK { + t.Errorf("a genuinely different peer must keep an independent bucket, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls, got %d", *calls) + } +} + +// TestRateLimitByUserAndIP_TrustedHeaderKeysIPComponent verifies the per-user +// limiter resolves the IP component through the SAME gated source as the +// per-IP limiter when TRUST_PROXY_HEADERS=true: the key is (user, trusted +// CF-Connecting-IP), so one user behind the shared proxy keeps one budget +// across proxy addresses and a different user gets an independent one. +func TestRateLimitByUserAndIP_TrustedHeaderKeysIPComponent(t *testing.T) { + setTrustProxyHeaders(t, true) + handler, calls := newRateLimitByUserAndIPTestHandler(1, time.Minute) + + if w := serveRateLimitUserRequestWithCF(t, handler, "user-a", "10.0.0.9:1234", "198.51.100.20"); w.Code != http.StatusOK { + t.Fatalf("user A first request: expected 200, got %d", w.Code) + } + // user-a again from a DIFFERENT proxy address but the SAME trusted header: + // the (user, header) key keeps the budget exhausted. + if w := serveRateLimitUserRequestWithCF(t, handler, "user-a", "10.0.0.10:1234", "198.51.100.20"); w.Code != http.StatusTooManyRequests { + t.Errorf("user A must stay exhausted across proxy addresses (header is the IP key), got %d", w.Code) + } + // user-b behind the same proxy address with its own header: independent. + if w := serveRateLimitUserRequestWithCF(t, handler, "user-b", "10.0.0.9:1234", "198.51.100.21"); w.Code != http.StatusOK { + t.Errorf("user B must keep an independent budget, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls (A and B first hits), got %d", *calls) + } +} + +// TestRateLimitByUserAndIP_UntrustedHeaderIgnored verifies the per-user limiter +// ignores a spoofed CF-Connecting-IP when TRUST_PROXY_HEADERS=false: the IP +// component is the real TCP peer, so the same user rotating the spoofed header +// cannot mint a fresh (user, IP) budget per request. +func TestRateLimitByUserAndIP_UntrustedHeaderIgnored(t *testing.T) { + setTrustProxyHeaders(t, false) + handler, calls := newRateLimitByUserAndIPTestHandler(1, time.Minute) + + if w := serveRateLimitUserRequestWithCF(t, handler, "user-a", "192.0.2.60:1234", "203.0.113.201"); w.Code != http.StatusOK { + t.Fatalf("user A first request: expected 200, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "user-a", "192.0.2.60:1234", "203.0.113.202"); w.Code != http.StatusTooManyRequests { + t.Errorf("rotating the spoofed header must NOT mint a fresh (user,IP) bucket, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "user-b", "192.0.2.60:1234", "203.0.113.201"); w.Code != http.StatusOK { + t.Errorf("user B from the same peer must keep an independent budget, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls, got %d", *calls) + } +} + +// TestRateLimitByUser_UnauthenticatedFallback_HonorsHeaderMode verifies the +// B8 user-keyed limiter's UNAUTHENTICATED fallback (key = ClientIP) honors the +// same TRUST_PROXY_HEADERS switching as the per-IP limiter: trusted mode keys +// the fallback on the header, untrusted mode on the RemoteAddr — an +// origin-exposed client cannot rotate the spoofed header to bypass the +// unauthenticated budget. +func TestRateLimitByUser_UnauthenticatedFallback_HonorsHeaderMode(t *testing.T) { + t.Run("trusted_headers_key_on_cf_header", func(t *testing.T) { + setTrustProxyHeaders(t, true) + handler, calls := newRateLimitByUserTestHandler(1, time.Minute) + if w := serveRateLimitUserRequestWithCF(t, handler, "", "10.0.0.1:1234", "198.51.100.30"); w.Code != http.StatusOK { + t.Fatalf("first request: expected 200, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "", "10.0.0.1:1234", "198.51.100.30"); w.Code != http.StatusTooManyRequests { + t.Errorf("same trusted header must stay exhausted, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "", "10.0.0.1:1234", "198.51.100.31"); w.Code != http.StatusOK { + t.Errorf("a distinct trusted header must keep an independent bucket, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls, got %d", *calls) + } + }) + + t.Run("untrusted_headers_key_on_remoteaddr", func(t *testing.T) { + setTrustProxyHeaders(t, false) + handler, calls := newRateLimitByUserTestHandler(1, time.Minute) + if w := serveRateLimitUserRequestWithCF(t, handler, "", "192.0.2.70:1234", "203.0.113.210"); w.Code != http.StatusOK { + t.Fatalf("first request: expected 200, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "", "192.0.2.70:1234", "203.0.113.211"); w.Code != http.StatusTooManyRequests { + t.Errorf("rotating the spoofed header must NOT mint a fresh bucket, got %d", w.Code) + } + if w := serveRateLimitUserRequestWithCF(t, handler, "", "192.0.2.71:1234", "203.0.113.210"); w.Code != http.StatusOK { + t.Errorf("a genuinely different peer must keep an independent bucket, got %d", w.Code) + } + if *calls != 2 { + t.Errorf("expected exactly 2 handler calls, got %d", *calls) + } + }) +} // finding 5: ONLY the top 10s abuse tier rejects the request 429 immediately // instead of sleeping a goroutine (a per-client goroutine-parking amplifier in // front of bcrypt); the 500ms / 2s / 5s tiers keep sleeping (backoff). Before