security: add strict per-IP rate limiting to auth endpoints

[intel352]

Summary

• Adds dedicated per-IP rate limiters to /api/v1/auth/login (10 req/min) and /api/v1/auth/register (5 req/hour), replacing the shared global 120 req/min limiter that offered minimal protection
• Extends RateLimitMiddleware to support fractional per-minute rates via float64 token buckets, enabling the hourly-rate config needed for registration
• Adds NewRateLimitMiddlewareWithHourlyRate constructor and requestsPerHour config field to the factory/schema so YAML configs can express low-frequency limits naturally

Changes

admin/config.yaml

• Add auth-login-ratelimit: 10 req/min, burst 10 per IP
• Add auth-register-ratelimit: 5 req/hour, burst 5 per IP
• Update login and register routes to use the dedicated limiters (not the shared 120/min one)

module/http_middleware.go

• client.tokens changed from int to float64 for fractional refill support
• RateLimitMiddleware gains ratePerMinute float64 field (used internally)
• New NewRateLimitMiddlewareWithHourlyRate(name, requestsPerHour, burstSize) constructor
• cleanupStaleClients updated to use fractional rate for stale threshold

plugins/http/modules.go

• rateLimitMiddlewareFactory now checks requestsPerHour key first; falls back to requestsPerMinute

plugins/http/schemas.go

• Adds requestsPerHour config field to http.middleware.ratelimit schema

Test plan

• TestNewRateLimitMiddlewareWithHourlyRate_AllowsBurst — burst of 5 then 429
• TestNewRateLimitMiddlewareWithHourlyRate_PerIP — separate buckets per IP
• TestNewRateLimitMiddlewareWithHourlyRate_RatePerMinute — correct fractional rate
• TestRateLimitMiddlewareFactory_RequestsPerHour — factory int and float64 config
• All existing RateLimit* tests pass (no regressions in token bucket or IP-based limiting)
• go build ./module/... ./plugins/... ./api/... — clean build
• golangci-lint run — 0 issues on changed packages
• Pre-push hook passes

Closes #94

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds strict per-IP rate limiting to authentication endpoints (/auth/login and /auth/register) to mitigate brute-force and credential stuffing attacks. It extends the existing RateLimitMiddleware to support fractional per-minute rates via float64 token buckets, enabling low-frequency hourly rate limits needed for registration (5 req/hour). The implementation includes a new constructor for hourly rates, updates to the factory/schema for configuration support, and comprehensive test coverage for the new functionality.

Changes:

• Extended rate limiting middleware to support fractional per-minute rates for hourly limits
• Added dedicated rate limiters to login (10 req/min) and register (5 req/hour) endpoints
• Includes unrelated JWT algorithm confusion tests and extensive documentation updates

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
module/http_middleware.go	Changed token bucket to use float64 for fractional rates; added NewRateLimitMiddlewareWithHourlyRate constructor; updated refill logic and cleanup calculations
module/http_middleware_test.go	Added three tests for hourly rate functionality: burst exhaustion, per-IP isolation, and rate calculation verification
plugins/http/modules.go	Updated factory to check requestsPerHour first (takes precedence over requestsPerMinute) for both int and float64 config values
plugins/http/schemas.go	Added requestsPerHour config field to rate limit schema with precedence documentation
plugins/http/plugin_test.go	Added factory test verifying both int and float64 requestsPerHour configuration handling
admin/config.yaml	Added two dedicated rate limiters (auth-login-ratelimit, auth-register-ratelimit) and applied them to respective auth endpoints
module/jwt_auth_test.go	Added JWT algorithm confusion attack prevention tests (unrelated to rate limiting)
DOCUMENTATION.md	Added extensive documentation for CI/CD steps, AI modules, infrastructure modules (unrelated to rate limiting)

Comments suppressed due to low confidence (1)

admin/config.yaml:163

• The /api/v1/auth/refresh endpoint lacks rate limiting protection. While login and register now have strict per-IP limits, the refresh endpoint only has CORS middleware. An attacker with a stolen or leaked refresh token could abuse this endpoint to generate unlimited access tokens. Consider applying rate limiting to this endpoint as well, though it could be more permissive than login (e.g., 30-60 requests/minute) since legitimate clients may need to refresh tokens frequently during long sessions.

      - method: POST
        path: "/api/v1/auth/refresh"
        handler: admin-auth
        middlewares: [admin-cors]

[Copilot]

The test suite lacks coverage for verifying that token refill works correctly with fractional rates over time. While TestNewRateLimitMiddlewareWithHourlyRate_RatePerMinute verifies the ratePerMinute field is set correctly, there's no test that verifies a client can make another request after waiting long enough for tokens to refill with the fractional rate. Consider adding a test that exhausts the burst, sleeps for a calculated duration, and verifies that exactly the expected number of additional requests are allowed based on the fractional refill rate.

Suggested change

	}
	}
	func TestNewRateLimitMiddlewareWithHourlyRate_FractionalRefill(t *testing.T) {
	// Use an hourly rate that produces a fractional ratePerMinute but short time-per-token.
	// 3599 requests/hour -> ratePerMinute = 3599 / 60 ≈ 59.9833 (fractional),
	// time per token ≈ 3600 / 3599 seconds ≈ 1.00028s.
	m := NewRateLimitMiddlewareWithHourlyRate("rl-hour-fractional", 3599, 1)
	handler := m.Process(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	w.WriteHeader(http.StatusOK)
	}))
	// First request should be allowed (uses burst token).
	req1 := httptest.NewRequest("GET", "/fractional", nil)
	rec1 := httptest.NewRecorder()
	handler.ServeHTTP(rec1, req1)
	if rec1.Code != http.StatusOK {
	t.Fatalf("first request: expected 200, got %d", rec1.Code)
	}
	// Second immediate request should be rate limited (burst exhausted, no time to refill).
	req2 := httptest.NewRequest("GET", "/fractional", nil)
	rec2 := httptest.NewRecorder()
	handler.ServeHTTP(rec2, req2)
	if rec2.Code != http.StatusTooManyRequests {
	t.Fatalf("second request: expected 429, got %d", rec2.Code)
	}
	// Calculate how long it should take to refill a single token based on the fractional rate.
	if m.ratePerMinute <= 0 {
	t.Fatalf("ratePerMinute must be positive, got %f", m.ratePerMinute)
	}
	timePerToken := time.Duration(float64(time.Minute) / m.ratePerMinute)
	// Wait slightly longer than required for one token to refill.
	time.Sleep(timePerToken + 100*time.Millisecond)
	// After waiting, exactly one additional request should be allowed.
	req3 := httptest.NewRequest("GET", "/fractional", nil)
	rec3 := httptest.NewRecorder()
	handler.ServeHTTP(rec3, req3)
	if rec3.Code != http.StatusOK {
	t.Fatalf("third request after refill: expected 200, got %d", rec3.Code)
	}
	// A subsequent immediate request should still be rate limited since only ~1 token refilled.
	req4 := httptest.NewRequest("GET", "/fractional", nil)
	rec4 := httptest.NewRecorder()
	handler.ServeHTTP(rec4, req4)
	if rec4.Code != http.StatusTooManyRequests {
	t.Fatalf("fourth request after refill: expected 429, got %d", rec4.Code)
	}
	}

[intel352]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@intel352 I've opened a new pull request, #107, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

plugins/http/modules.go:185

• rateLimitMiddlewareFactory contains a duplicated/obsolete implementation block after the new parsing logic. This redeclares requestsPerMinute in the same scope (won’t compile) and also unconditionally returns an hourly limiter even when requestsPerHour is 0/negative, which would create a limiter with ratePerMinute=0 (no refill). Remove the old block (lines 170-184) and keep a single code path that (a) only uses requestsPerHour when > 0 and (b) otherwise falls back to validated requestsPerMinute defaults.

	// requestsPerHour takes precedence over requestsPerMinute for low-frequency
	// endpoints (e.g. registration) where fractional per-minute rates are needed.
	if rph, ok := cfg["requestsPerHour"].(int); ok {
		return module.NewRateLimitMiddlewareWithHourlyRate(name, rph, burstSize)
	} else if rph, ok := cfg["requestsPerHour"].(float64); ok {
		return module.NewRateLimitMiddlewareWithHourlyRate(name, int(rph), burstSize)
	}

	requestsPerMinute := 60
	if rpm, ok := cfg["requestsPerMinute"].(int); ok {
		requestsPerMinute = rpm
	} else if rpm, ok := cfg["requestsPerMinute"].(float64); ok {
		requestsPerMinute = int(rpm)
	}
	return module.NewRateLimitMiddleware(name, requestsPerMinute, burstSize)
}

[Copilot]

TestRateLimitMiddlewareFactory_InvalidValues claims to verify fallback/default behavior (negative RPM uses default, zero burst uses default), but it only asserts that the factory returns a non-nil module. This won’t catch regressions. Consider exercising the returned middleware via Process and asserting observable behavior (e.g., burst default allows 10 immediate requests; default RPM affects Retry-After calculation).

[Copilot]

TestNewRateLimitMiddlewareWithHourlyRate_FractionalRefill uses a real time.Sleep (~1s+) to wait for refill, which slows the unit test suite and can be flaky under CI load. Since this test is in the module package, it can avoid sleeping by directly adjusting the relevant client’s lastTimestamp backwards (under lock) to simulate elapsed time, then issuing the next request.

Suggested change

	// Wait slightly longer than the time needed to refill one token.
	if m.ratePerMinute <= 0 {
	t.Fatalf("ratePerMinute must be positive, got %f", m.ratePerMinute)
	}
	timePerToken := time.Duration(float64(time.Minute) / m.ratePerMinute)
	time.Sleep(timePerToken + 100*time.Millisecond)
	// After waiting, exactly one additional request should be allowed.
	// Simulate waiting slightly longer than the time needed to refill one token
	// by adjusting the internal client's lastTimestamp instead of sleeping.
	if m.ratePerMinute <= 0 {
	t.Fatalf("ratePerMinute must be positive, got %f", m.ratePerMinute)
	}
	timePerToken := time.Duration(float64(time.Minute) / m.ratePerMinute)
	adjustment := timePerToken + 100*time.Millisecond
	// Move the client's lastTimestamp backward under lock so that, from the
	// rate limiter's perspective, enough time has passed to refill one token.
	m.mu.Lock()
	for _, client := range m.clients {
	client.mu.Lock()
	client.lastTimestamp = client.lastTimestamp.Add(-adjustment)
	client.mu.Unlock()
	}
	m.mu.Unlock()
	// After simulating the wait, exactly one additional request should be allowed.