fix(api): count /verify's per-IP limit per API key

README.md

@@ -447,7 +447,7 @@ The compose file pulls `latest-mysql`. Set `GATECHA_IMAGE` to run another image,
 | `GATECHA_MAX_BODY_BYTES` | `1048576` | Maximum accepted request body size, in bytes |
 | `GATECHA_RATE_LIMIT_ENABLED` | `true` | Enable per-IP rate limiting |
 | `GATECHA_RATE_LIMIT_LOGIN` | `5` | Admin login requests per minute, per IP |
-| `GATECHA_RATE_LIMIT_API` | `60` | Public API (`/api/v1/*`) requests per minute, per IP |
+| `GATECHA_RATE_LIMIT_API` | `60` | Public API requests per minute: per IP on `/api/v1/challenge` and `/api/v1/stats`, per IP **and API key** on `/api/v1/verify`, so sites sharing a hosting provider's outbound IP each get the full budget |
 
 > ⚠️ **Behind a reverse proxy, set `GATECHA_TRUST_PROXY=true`.**
 > Per-IP rate limiting keys off the connecting IP. With `GATECHA_TRUST_PROXY=false`

internal/api/ratelimit.go

@@ -111,6 +111,76 @@ func RateLimitMiddleware(requestsPerMinute int) func(http.Handler) http.Handler
 	}
 }
 
+// exhausted reports whether ip has no token left, refilling but not consuming.
+func (l *ipRateLimiter) exhausted(ip string) bool {
+	now := l.now()
+
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	v, ok := l.visitors[ip]
+	if !ok {
+		return false
+	}
+	return min(l.burst, v.tokens+now.Sub(v.lastSeen).Seconds()*l.ratePerSec) < 1
+}
+
+// statusRecorder remembers the status code a handler wrote.
+type statusRecorder struct {
+	http.ResponseWriter
+	status int
+}
+
+func (s *statusRecorder) WriteHeader(code int) {
+	s.status = code
+	s.ResponseWriter.WriteHeader(code)
+}
+
+// FailedAuthRateLimitMiddleware caps, per IP, the requests that fail API key
+// authentication (401). It runs before APIKeyMiddleware, so an IP cycling
+// through made-up keys is stopped before each one costs a database lookup,
+// while requests carrying a valid key never draw on this budget.
+func FailedAuthRateLimitMiddleware(requestsPerMinute int) func(http.Handler) http.Handler {
+	limiter := newIPRateLimiter(requestsPerMinute)
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			ip := clientIP(r)
+			if limiter.exhausted(ip) {
+				writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "rate limit exceeded"})
+				return
+			}
+			rec := &statusRecorder{ResponseWriter: w, status: http.StatusOK}
+			next.ServeHTTP(rec, r)
+			if rec.status == http.StatusUnauthorized {
+				limiter.allow(ip)
+			}
+		})
+	}
+}
+
+// SourceAndKeyRateLimitMiddleware applies the per-IP budget to each (client IP,
+// API key) pair rather than to the IP alone. It guards /verify, which is called
+// by site backends, not browsers: on shared hosting many sites post from one
+// outbound IP, and a plain per-IP bucket made them throttle each other. Each
+// site now gets the full budget from that IP, and one site still cannot exceed
+// it. It must run after APIKeyMiddleware, so only real keys open a bucket.
+func SourceAndKeyRateLimitMiddleware(requestsPerMinute int) func(http.Handler) http.Handler {
+	limiter := newIPRateLimiter(requestsPerMinute)
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			bucket := clientIP(r)
+			if key := GetAPIKeyFromContext(r); key != nil {
+				bucket += "|" + key.KeyID
+			}
+			if !limiter.allow(bucket) {
+				writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "rate limit exceeded"})
+				return
+			}
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
 // keyRateLimiter is a token-bucket limiter keyed by an arbitrary string (here an
 // API key ID). Unlike ipRateLimiter the per-minute budget is supplied per call,
 // so each key enforces its own configured limit and a runtime change to that

internal/api/ratelimit_test.go

@@ -3,8 +3,12 @@ package api
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
+
+	"github.com/Upellift99/GateCHA/internal/models"
+	"github.com/Upellift99/GateCHA/internal/testutil"
 )
 
 // fakeClock returns a controllable time source for deterministic limiter tests.
@@ -180,3 +184,88 @@ func TestClientIP(t *testing.T) {
 		}
 	}
 }
+
+// limitedRouter is the full router with the per-IP limit on and set to n.
+func limitedRouter(t *testing.T, n int) (http.Handler, string, string) {
+	t.Helper()
+	db := testutil.SetupTestDB(t)
+	a, _ := models.CreateAPIKey(db, "site A", "", 0, 0, "")
+	b, _ := models.CreateAPIKey(db, "site B", "", 0, 0, "")
+	router := NewRouter(db, testSecretKey, RouterConfig{RateLimitEnabled: true, RateLimitAPI: n, RateLimitLogin: 5})
+	return router, a.KeyID, b.KeyID
+}
+
+func hit(router http.Handler, method, path, ip string) int {
+	req := httptest.NewRequest(method, path, strings.NewReader(`{"payload":"x"}`))
+	req.RemoteAddr = ip + ":1234"
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+	return w.Code
+}
+
+// Sites on one shared-hosting IP must not throttle each other on /verify, yet
+// each still gets no more than the per-IP budget from that IP.
+func TestVerifyRateLimit_PerSourceAndKey(t *testing.T) {
+	router, keyA, keyB := limitedRouter(t, 3)
+	const host = "203.0.113.7"
+
+	for i := range 3 {
+		if code := hit(router, "POST", "/api/v1/verify?apiKey="+keyA, host); code == http.StatusTooManyRequests {
+			t.Fatalf("site A request %d throttled early", i+1)
+		}
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey="+keyA, host); code != http.StatusTooManyRequests {
+		t.Errorf("site A over budget: expected 429, got %d", code)
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey="+keyB, host); code == http.StatusTooManyRequests {
+		t.Error("site B on the same IP was throttled by site A's traffic")
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey="+keyA, "198.51.100.9"); code == http.StatusTooManyRequests {
+		t.Error("site A from another IP was throttled")
+	}
+}
+
+// Challenges come from browsers: the budget stays per IP, whatever the key.
+func TestChallengeRateLimit_StaysPerIP(t *testing.T) {
+	router, keyA, keyB := limitedRouter(t, 3)
+	const visitor = "192.0.2.44"
+
+	for range 3 {
+		hit(router, "GET", "/api/v1/challenge?apiKey="+keyA, visitor)
+	}
+	if code := hit(router, "GET", "/api/v1/challenge?apiKey="+keyB, visitor); code != http.StatusTooManyRequests {
+		t.Errorf("challenge over the per-IP budget with another key: expected 429, got %d", code)
+	}
+	// Verifications do not draw on the challenge budget.
+	if code := hit(router, "POST", "/api/v1/verify?apiKey="+keyA, visitor); code == http.StatusTooManyRequests {
+		t.Error("verify throttled by the challenge budget")
+	}
+}
+
+// Made-up keys are refused (401) and, past the per-IP budget, stopped (429)
+// before any key lookup; a valid key from the same IP is not affected.
+func TestVerifyRateLimit_FailedAuthCappedPerIP(t *testing.T) {
+	router, keyA, _ := limitedRouter(t, 3)
+	const attacker = "203.0.113.8"
+
+	for i := range 3 {
+		if code := hit(router, "POST", "/api/v1/verify?apiKey=gk_000000000000000000000000", attacker); code != http.StatusUnauthorized {
+			t.Fatalf("request %d: expected 401, got %d", i+1, code)
+		}
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey=gk_111111111111111111111111", attacker); code != http.StatusTooManyRequests {
+		t.Errorf("failed-auth budget spent: expected 429, got %d", code)
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey=gk_000000000000000000000000", "198.51.100.20"); code != http.StatusUnauthorized {
+		t.Errorf("another IP: expected 401, got %d", code)
+	}
+
+	// Valid keys never draw on the failed-auth budget.
+	router, keyA, _ = limitedRouter(t, 3)
+	for range 3 {
+		hit(router, "POST", "/api/v1/verify?apiKey="+keyA, attacker)
+	}
+	if code := hit(router, "POST", "/api/v1/verify?apiKey=gk_000000000000000000000000", attacker); code != http.StatusUnauthorized {
+		t.Errorf("valid traffic consumed the failed-auth budget: expected 401, got %d", code)
+	}
+}

internal/api/router.go

@@ -56,20 +56,42 @@ func NewRouter(db *gorm.DB, secretKey string, cfg RouterConfig) http.Handler {
 
 	// Public API (API key auth)
 	r.Route("/api/v1", func(r chi.Router) {
-		if cfg.RateLimitEnabled {
-			r.Use(RateLimitMiddleware(cfg.RateLimitAPI))
-		}
+		// Per-key throttle runs after auth so the key (and its limit) is known.
+		// No-op for keys whose RateLimitPerMin is 0. One instance, so challenges
+		// and verifications draw on the same per-key budget.
+		perKey := PerKeyRateLimitMiddleware()
+
+		// Browsers fetch challenges: the per-IP budget applies before auth.
 		r.Group(func(r chi.Router) {
+			if cfg.RateLimitEnabled {
+				r.Use(RateLimitMiddleware(cfg.RateLimitAPI))
+			}
 			r.Use(APIKeyMiddleware(db))
-			// Per-key throttle runs after auth so the key (and its limit) is known.
-			// No-op for keys whose RateLimitPerMin is 0.
-			r.Use(PerKeyRateLimitMiddleware())
+			r.Use(perKey)
 			r.Get("/challenge", challengeHandler.ServeHTTP)
+		})
+		// Site backends verify: the per-IP budget is counted per key, so sites
+		// sharing a hosting provider's outbound IP do not throttle each other.
+		// Failed authentications still count per IP before the key lookup.
+		r.Group(func(r chi.Router) {
+			if cfg.RateLimitEnabled {
+				r.Use(FailedAuthRateLimitMiddleware(cfg.RateLimitAPI))
+			}
+			r.Use(APIKeyMiddleware(db))
+			if cfg.RateLimitEnabled {
+				r.Use(SourceAndKeyRateLimitMiddleware(cfg.RateLimitAPI))
+			}
+			r.Use(perKey)
 			r.Post("/verify", verifyHandler.ServeHTTP)
 		})
 		// Authenticated by the key's stats token, not its public key ID, so it
 		// sits outside the API key middleware.
-		r.Get("/stats", keyStatsHandler.ServeHTTP)
+		r.Group(func(r chi.Router) {
+			if cfg.RateLimitEnabled {
+				r.Use(RateLimitMiddleware(cfg.RateLimitAPI))
+			}
+			r.Get("/stats", keyStatsHandler.ServeHTTP)
+		})
 	})
 
 	// Admin API