-
Notifications
You must be signed in to change notification settings - Fork 0
security: add strict per-IP rate limiting to auth endpoints #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0d9ee9c
2cd1e01
3289baf
b3fedf3
8696196
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -460,8 +460,8 @@ func TestRateLimitMiddleware_RetryAfterHeader(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if rec2.Code != http.StatusTooManyRequests { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Errorf("expected 429, got %d", rec2.Code) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if rec2.Header().Get("Retry-After") != "60" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Errorf("expected Retry-After header '60', got %q", rec2.Header().Get("Retry-After")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if rec2.Header().Get("Retry-After") != "1" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Errorf("expected Retry-After header '1', got %q", rec2.Header().Get("Retry-After")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -640,3 +640,53 @@ func TestNewRateLimitMiddlewareWithHourlyRate_RatePerMinute(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Errorf("expected ratePerMinute=1.0, got %f", m.ratePerMinute) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestNewRateLimitMiddlewareWithHourlyRate_FractionalRefill(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 3600 requests/hour -> ratePerMinute = 60.0, timePerToken = 1 second. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Using a high hourly rate keeps the sleep short while still exercising | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the fractional refill path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m := NewRateLimitMiddlewareWithHourlyRate("rl-hour-fractional", 3600, 1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| handler := m.Process(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusOK) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // First request should be allowed (uses the single 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 must be rate-limited (burst exhausted, no refill yet). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+670
to
+677
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -356,6 +356,41 @@ func TestRateLimitMiddlewareFactory_RequestsPerHour(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func TestRateLimitMiddlewareFactory_InvalidValues(t *testing.T) { | ||
| factories := moduleFactories() | ||
| factory, ok := factories["http.middleware.ratelimit"] | ||
| if !ok { | ||
| t.Fatal("no factory for http.middleware.ratelimit") | ||
| } | ||
|
|
||
| // Zero requestsPerHour must fall through to requestsPerMinute path (not crash). | ||
| modZeroRPH := factory("rl-zero-rph", map[string]any{ | ||
| "requestsPerHour": 0, | ||
| "requestsPerMinute": 30, | ||
| "burstSize": 5, | ||
| }) | ||
| if modZeroRPH == nil { | ||
| t.Fatal("factory returned nil for zero requestsPerHour config") | ||
| } | ||
|
|
||
| // Negative requestsPerMinute must use default (60). | ||
| modNegRPM := factory("rl-neg-rpm", map[string]any{ | ||
| "requestsPerMinute": -10, | ||
| }) | ||
| if modNegRPM == nil { | ||
| t.Fatal("factory returned nil for negative requestsPerMinute config") | ||
| } | ||
|
|
||
| // Zero burstSize must keep default (10). | ||
| modZeroBurst := factory("rl-zero-burst", map[string]any{ | ||
| "requestsPerMinute": 60, | ||
| "burstSize": 0, | ||
| }) | ||
| if modZeroBurst == nil { | ||
| t.Fatal("factory returned nil for zero burstSize config") | ||
| } | ||
| } | ||
|
Comment on lines
+359
to
+392
|
||
|
|
||
| func TestPluginLoaderIntegration(t *testing.T) { | ||
| p := New() | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.