-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[12.x] Introduce "after" rate limiting #57125
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
Conversation
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
I dig this... trying to decide how I feel about RateLimiter::for('resource-not-found', function (Request $request) {
return Limit::perMinute(10)
->by("user:{$request->user()->id}")
->whenResponse(fn (Response $response) => $response->getStatusCode() === 404)
}); But even I think I'm fine with punting on your caveat for now and wait to see community demand, but if it's pretty easy to add your suggest lock / wait stuff that's fine too! |
Are we going to have helper methods for success/404? |
Maybe |
I've realised that the standard Regarding naming, I think I still lean towards |
Thanks! |
Seeems like this comment was lost in the shuffle:
|
This PR continues the amazing work, by @JosephSilber, in #56933 by extending it to allow for a generic callback hook.
Currently, it is only possible to rate limit based on the request. Often, it can be useful to rate limit based on the response.
From the original PR: For example, imagine you have a sign up endpoint that could return validation errors. If you want to throttle to a single sign up per day for an IP address, any validation response would trigger the rate limiter making it impossible to sign up again for the day.
Another example is rate limiting 404 responses to mitigate enumeration attacks on resource identifiers. If a single user hits a certain level of 404 responses in a short time frame, they could be attempting enumeration attacks.
Below is an example of an after limiter that would restrict user's from hitting too many 404 responses in a single minute:
Yet to be solved caveat
Due to the nature of after limiters, it is possible that more than 10 requests could come through per minute. Given the above limiter, if there were already 9 requests and then two requests came in at the same time, a race condition occurs where two requests could make it through. This would effectively mean that 11 requests were made within the minute.
I think we could solve this but I've left it for now to open discussion.
Initial solution idea: I was thinking we could offer the ability to use a cache lock to pause the second incoming request until the first has finished. Replicating what the
Route::block
method achieves:This would use the limit's
key
to determine when to block.