From 66661d1b1d4a97ac28dc874a821036794da2bef1 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 14:04:52 -0800 Subject: [PATCH 1/3] Add rate limiting to login endpoint to prevent brute force Apply per-IP rate limiting (5 attempts/minute) to POST /login using golang.org/x/time/rate. Returns 429 Too Many Requests when exceeded. Closes #12 --- internal/middleware/middleware.go | 67 ++++++++++++++++ internal/middleware/ratelimit_test.go | 107 ++++++++++++++++++++++++++ internal/server/routes.go | 2 +- 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 internal/middleware/ratelimit_test.go diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 637b51d..6e465e4 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -5,6 +5,7 @@ import ( "log/slog" "net" "net/http" + "sync" "time" "github.com/99designs/basicauth-go" @@ -12,6 +13,7 @@ import ( "github.com/go-chi/cors" "github.com/gorilla/csrf" "go.uber.org/fx" + "golang.org/x/time/rate" "git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/globals" @@ -162,6 +164,71 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler { ) } +// loginRateLimit configures the login rate limiter. +const ( + loginRateLimit = rate.Limit(5.0 / 60.0) // 5 requests per 60 seconds + loginBurst = 5 // allow burst of 5 +) + +// ipLimiter tracks per-IP rate limiters for login attempts. +type ipLimiter struct { + mu sync.Mutex + limiters map[string]*rate.Limiter +} + +func newIPLimiter() *ipLimiter { + return &ipLimiter{ + limiters: make(map[string]*rate.Limiter), + } +} + +func (i *ipLimiter) getLimiter(ip string) *rate.Limiter { + i.mu.Lock() + defer i.mu.Unlock() + + limiter, exists := i.limiters[ip] + if !exists { + limiter = rate.NewLimiter(loginRateLimit, loginBurst) + i.limiters[ip] = limiter + } + + return limiter +} + +// loginLimiter is the singleton IP rate limiter for login attempts. +// +//nolint:gochecknoglobals // intentional singleton for rate limiting state +var loginLimiter = newIPLimiter() + +// LoginRateLimit returns middleware that rate-limits login attempts per IP. +// It allows 5 attempts per minute and returns 429 Too Many Requests when exceeded. +func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func( + writer http.ResponseWriter, + request *http.Request, + ) { + ip := ipFromHostPort(request.RemoteAddr) + limiter := loginLimiter.getLimiter(ip) + + if !limiter.Allow() { + m.log.WarnContext(request.Context(), "login rate limit exceeded", + "remoteIP", ip, + ) + http.Error( + writer, + "Too Many Requests", + http.StatusTooManyRequests, + ) + + return + } + + next.ServeHTTP(writer, request) + }) + } +} + // SetupRequired returns middleware that redirects to setup if no user exists. func (m *Middleware) SetupRequired() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go new file mode 100644 index 0000000..ea3a448 --- /dev/null +++ b/internal/middleware/ratelimit_test.go @@ -0,0 +1,107 @@ +package middleware + +import ( + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "git.eeqj.de/sneak/upaas/internal/config" +) + +func newTestMiddleware(t *testing.T) *Middleware { + t.Helper() + + return &Middleware{ + log: slog.Default(), + params: &Params{ + Config: &config.Config{}, + }, + } +} + +func TestLoginRateLimitAllowsUpToBurst(t *testing.T) { + // Reset the global limiter to get clean state + loginLimiter = newIPLimiter() + + mw := newTestMiddleware(t) + + handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + // First 5 requests should succeed (burst) + for i := range 5 { + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "192.168.1.1:12345" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code, "request %d should succeed", i+1) + } + + // 6th request should be rate limited + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "192.168.1.1:12345" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + assert.Equal(t, http.StatusTooManyRequests, rec.Code, "6th request should be rate limited") +} + +func TestLoginRateLimitIsolatesIPs(t *testing.T) { + loginLimiter = newIPLimiter() + + mw := newTestMiddleware(t) + + handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + // Exhaust IP1's budget + for range 5 { + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "10.0.0.1:1234" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + } + + // IP1 should be blocked + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "10.0.0.1:1234" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + assert.Equal(t, http.StatusTooManyRequests, rec.Code) + + // IP2 should still work + req2 := httptest.NewRequest(http.MethodPost, "/login", nil) + req2.RemoteAddr = "10.0.0.2:1234" + rec2 := httptest.NewRecorder() + handler.ServeHTTP(rec2, req2) + assert.Equal(t, http.StatusOK, rec2.Code, "different IP should not be rate limited") +} + +func TestLoginRateLimitReturns429Body(t *testing.T) { + loginLimiter = newIPLimiter() + + mw := newTestMiddleware(t) + + handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + // Exhaust burst + for range 5 { + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "172.16.0.1:5555" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + } + + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "172.16.0.1:5555" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + assert.Equal(t, http.StatusTooManyRequests, rec.Code) + assert.Contains(t, rec.Body.String(), "Too Many Requests") +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 860d0b9..2fb632f 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -46,7 +46,7 @@ func (s *Server) SetupRoutes() { // Public routes r.Get("/login", s.handlers.HandleLoginGET()) - r.Post("/login", s.handlers.HandleLoginPOST()) + r.With(s.mw.LoginRateLimit()).Post("/login", s.handlers.HandleLoginPOST()) r.Get("/setup", s.handlers.HandleSetupGET()) r.Post("/setup", s.handlers.HandleSetupPOST()) -- 2.45.2 From a1b06219e7e8a8de58b68366c0742ccb80d4161f Mon Sep 17 00:00:00 2001 From: user Date: Sun, 15 Feb 2026 14:18:07 -0800 Subject: [PATCH 2/3] fix: add eviction for stale IP rate limiter entries and Retry-After header - Store lastSeen timestamp per IP limiter entry - Lazy sweep removes entries older than 10 minutes on each request - Add Retry-After header to 429 responses - Add test for stale entry eviction Fixes memory leak under sustained attack from many IPs. --- internal/middleware/middleware.go | 64 ++++++++++++++++++++++----- internal/middleware/ratelimit_test.go | 28 ++++++++++++ 2 files changed, 82 insertions(+), 10 deletions(-) diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 6e465e4..56e8707 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -2,7 +2,9 @@ package middleware import ( + "fmt" "log/slog" + "math" "net" "net/http" "sync" @@ -166,33 +168,64 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler { // loginRateLimit configures the login rate limiter. const ( - loginRateLimit = rate.Limit(5.0 / 60.0) // 5 requests per 60 seconds - loginBurst = 5 // allow burst of 5 + loginRateLimit = rate.Limit(5.0 / 60.0) // 5 requests per 60 seconds + loginBurst = 5 // allow burst of 5 + limiterExpiry = 10 * time.Minute // evict entries not seen in 10 minutes + limiterCleanupEvery = 1 * time.Minute // sweep interval ) -// ipLimiter tracks per-IP rate limiters for login attempts. +// ipLimiterEntry stores a rate limiter with its last-seen timestamp. +type ipLimiterEntry struct { + limiter *rate.Limiter + lastSeen time.Time +} + +// ipLimiter tracks per-IP rate limiters for login attempts with automatic +// eviction of stale entries to prevent unbounded memory growth. type ipLimiter struct { mu sync.Mutex - limiters map[string]*rate.Limiter + limiters map[string]*ipLimiterEntry + lastSweep time.Time } func newIPLimiter() *ipLimiter { return &ipLimiter{ - limiters: make(map[string]*rate.Limiter), + limiters: make(map[string]*ipLimiterEntry), + lastSweep: time.Now(), } } +// sweep removes entries not seen within limiterExpiry. Must be called with mu held. +func (i *ipLimiter) sweep(now time.Time) { + for ip, entry := range i.limiters { + if now.Sub(entry.lastSeen) > limiterExpiry { + delete(i.limiters, ip) + } + } + i.lastSweep = now +} + func (i *ipLimiter) getLimiter(ip string) *rate.Limiter { i.mu.Lock() defer i.mu.Unlock() - limiter, exists := i.limiters[ip] - if !exists { - limiter = rate.NewLimiter(loginRateLimit, loginBurst) - i.limiters[ip] = limiter + now := time.Now() + + // Lazy sweep: clean up stale entries periodically. + if now.Sub(i.lastSweep) >= limiterCleanupEvery { + i.sweep(now) } - return limiter + entry, exists := i.limiters[ip] + if !exists { + entry = &ipLimiterEntry{ + limiter: rate.NewLimiter(loginRateLimit, loginBurst), + } + i.limiters[ip] = entry + } + entry.lastSeen = now + + return entry.limiter } // loginLimiter is the singleton IP rate limiter for login attempts. @@ -215,6 +248,17 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { m.log.WarnContext(request.Context(), "login rate limit exceeded", "remoteIP", ip, ) + + // Compute seconds until the next token is available. + reservation := limiter.Reserve() + delay := reservation.Delay() + reservation.Cancel() + retryAfter := int(math.Ceil(delay.Seconds())) + if retryAfter < 1 { + retryAfter = 1 + } + writer.Header().Set("Retry-After", fmt.Sprintf("%d", retryAfter)) + http.Error( writer, "Too Many Requests", diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go index ea3a448..a932ccf 100644 --- a/internal/middleware/ratelimit_test.go +++ b/internal/middleware/ratelimit_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" @@ -104,4 +105,31 @@ func TestLoginRateLimitReturns429Body(t *testing.T) { handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusTooManyRequests, rec.Code) assert.Contains(t, rec.Body.String(), "Too Many Requests") + assert.NotEmpty(t, rec.Header().Get("Retry-After"), "should include Retry-After header") +} + +func TestIPLimiterEvictsStaleEntries(t *testing.T) { + il := newIPLimiter() + + // Add an entry and backdate its lastSeen + il.mu.Lock() + il.limiters["1.2.3.4"] = &ipLimiterEntry{ + limiter: nil, + lastSeen: time.Now().Add(-15 * time.Minute), + } + il.limiters["5.6.7.8"] = &ipLimiterEntry{ + limiter: nil, + lastSeen: time.Now(), + } + il.mu.Unlock() + + // Trigger sweep + il.mu.Lock() + il.sweep(time.Now()) + il.mu.Unlock() + + il.mu.Lock() + defer il.mu.Unlock() + assert.NotContains(t, il.limiters, "1.2.3.4", "stale entry should be evicted") + assert.Contains(t, il.limiters, "5.6.7.8", "fresh entry should remain") } -- 2.45.2 From ef0786c4b4d118e78c9c9856100884f919f06b00 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 21:14:12 -0800 Subject: [PATCH 3/3] fix: extract real client IP from proxy headers (X-Real-IP / X-Forwarded-For) Behind a reverse proxy like Traefik, RemoteAddr always contains the proxy's IP. Add realIP() helper that checks X-Real-IP first, then the first entry of X-Forwarded-For, falling back to RemoteAddr. Update both LoginRateLimit and Logging middleware to use realIP(). Add comprehensive tests for the new function. Fixes #12 --- internal/middleware/middleware.go | 27 +++++++++- internal/middleware/realip_test.go | 83 ++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 internal/middleware/realip_test.go diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 56e8707..daf15f1 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -7,6 +7,7 @@ import ( "math" "net" "net/http" + "strings" "sync" "time" @@ -90,7 +91,7 @@ func (m *Middleware) Logging() func(http.Handler) http.Handler { "request_id", reqID, "referer", request.Referer(), "proto", request.Proto, - "remoteIP", ipFromHostPort(request.RemoteAddr), + "remoteIP", realIP(request), "status", lrw.statusCode, "latency_ms", latency.Milliseconds(), ) @@ -110,6 +111,28 @@ func ipFromHostPort(hostPort string) string { return host } +// realIP extracts the client's real IP address from the request, +// checking proxy headers first (trusted reverse proxy like Traefik), +// then falling back to RemoteAddr. +func realIP(r *http.Request) string { + // 1. X-Real-IP (set by Traefik/nginx) + if ip := strings.TrimSpace(r.Header.Get("X-Real-IP")); ip != "" { + return ip + } + + // 2. X-Forwarded-For: take the first (leftmost/client) IP + if xff := r.Header.Get("X-Forwarded-For"); xff != "" { + if parts := strings.SplitN(xff, ",", 2); len(parts) > 0 { //nolint:mnd + if ip := strings.TrimSpace(parts[0]); ip != "" { + return ip + } + } + } + + // 3. Fall back to RemoteAddr + return ipFromHostPort(r.RemoteAddr) +} + // CORS returns CORS middleware. func (m *Middleware) CORS() func(http.Handler) http.Handler { return cors.Handler(cors.Options{ @@ -241,7 +264,7 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { writer http.ResponseWriter, request *http.Request, ) { - ip := ipFromHostPort(request.RemoteAddr) + ip := realIP(request) limiter := loginLimiter.getLimiter(ip) if !limiter.Allow() { diff --git a/internal/middleware/realip_test.go b/internal/middleware/realip_test.go new file mode 100644 index 0000000..3271a85 --- /dev/null +++ b/internal/middleware/realip_test.go @@ -0,0 +1,83 @@ +package middleware + +import ( + "net/http" + "testing" +) + +func TestRealIP(t *testing.T) { + tests := []struct { + name string + remoteAddr string + xRealIP string + xff string + want string + }{ + { + name: "X-Real-IP takes priority", + remoteAddr: "10.0.0.1:1234", + xRealIP: "203.0.113.5", + xff: "198.51.100.1, 10.0.0.1", + want: "203.0.113.5", + }, + { + name: "X-Forwarded-For used when no X-Real-IP", + remoteAddr: "10.0.0.1:1234", + xff: "198.51.100.1, 10.0.0.1", + want: "198.51.100.1", + }, + { + name: "X-Forwarded-For single IP", + remoteAddr: "10.0.0.1:1234", + xff: "203.0.113.10", + want: "203.0.113.10", + }, + { + name: "falls back to RemoteAddr", + remoteAddr: "192.168.1.1:5678", + want: "192.168.1.1", + }, + { + name: "RemoteAddr without port", + remoteAddr: "192.168.1.1", + want: "192.168.1.1", + }, + { + name: "X-Real-IP with whitespace", + remoteAddr: "10.0.0.1:1234", + xRealIP: " 203.0.113.5 ", + want: "203.0.113.5", + }, + { + name: "X-Forwarded-For with whitespace", + remoteAddr: "10.0.0.1:1234", + xff: " 198.51.100.1 , 10.0.0.1", + want: "198.51.100.1", + }, + { + name: "empty X-Real-IP falls through to XFF", + remoteAddr: "10.0.0.1:1234", + xRealIP: " ", + xff: "198.51.100.1", + want: "198.51.100.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = tt.remoteAddr + if tt.xRealIP != "" { + req.Header.Set("X-Real-IP", tt.xRealIP) + } + if tt.xff != "" { + req.Header.Set("X-Forwarded-For", tt.xff) + } + + got := realIP(req) + if got != tt.want { + t.Errorf("realIP() = %q, want %q", got, tt.want) + } + }) + } +} -- 2.45.2