fix(throttle): key the API rate limiter on the consumer, not the proxy IP - #280
Merged
Merged
Conversation
…y IP ThrottleRequests runs before API authentication, so Laravel's default signature always fell back to route domain + client IP. Behind a load balancer that IP is the balancer's, and no route has a domain, so every tenant, API key and console visitor shared one bucket: one busy integration returned 429 to the whole platform. Key the limiter on the hashed credential (API key, Sanctum token, basic auth), falling back to the user and then the IP only when none is sent, and scope it by first path segment so /v1 and /int stay separate. Also keep Retry-After and X-RateLimit-* on the 429 response so throttled clients know when to retry.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 7676 7679 +3
===========================================
Files 433 433
Lines 25009 25016 +7
===========================================
+ Hits 25009 25016 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Sep 29, 2026
Merged
roncodes
force-pushed
the
release/v1.6.66
branch
from
September 29, 2026 08:28
f4fc81a to
2f008a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A single tenant sending order create/update requests through the public API put the whole platform into
429 Too many requests: other tenants' integrations, the driver and storefront apps, and console login.Fleetbase\Http\Middleware\ThrottleRequestsis the first middleware infleetbase.api, ahead ofAuthenticateOnceWithBasicAuth. When it runs,$request->user()is always null, so Laravel'sresolveRequestSignature()falls back toroute domain + $request->ip(). No route has a domain, and behind a load balancer the IP is the balancer's. Every caller therefore shared one 120/min bucket. The console's public routes (int/v1/auth/login,two-fa/check,settings/branding,lookup/*) produced the same cache key.Reproduced locally through the full kernel: after 5 requests from tenant A (limit 5), both tenant B (different key, different client IP) and an anonymous console lookup got 429.
Fix
resolveRequestSignature()keys on the SHA-1 of the presented credential (Bearer API key, Sanctum token or basic auth). It needs no database lookup and doesn't depend on proxy configuration. It falls back to the authenticated user, then the IP, only when no credential is sent./v1and/intnever share a bucket.Handler: the 429 response now keepsRetry-AfterandX-RateLimit-*. Previously they were dropped, so clients couldn't back off correctly.Related PRs
Tests
/intstays separate from/v1; the user/IP fallback ordering; the 429 keeps its headers.MiddlewareContractsTestandExceptionHandlerTest: 59 passed.composer test:lintis clean.