feat(rate-limit): add rate limiting - #2272
Conversation
| * Counts requests by client IP. Requires {@see \Tempest\Http\Ip\TrustedProxiesConfig} | ||
| * for reliable client IPs behind proxies. | ||
| */ | ||
| final readonly class ClientIpKeyResolver implements RateLimitKeyResolver |
There was a problem hiding this comment.
Can you update the acronym casing to conform to our guidelines?
There was a problem hiding this comment.
According to this guideline on acronym casing, was https://github.com/tempestphp/tempest-framework/blob/3.x/packages/support/src/Ip/IpAddress.php from #2244 also meant to be named IPAddress? 👀 I suppose now we need to rename it as well, and put it out as a breaking change with a Rector rule for easy upgrade, or maybe that's for 4.x?
There was a problem hiding this comment.
Eh... yes. 🫣😛
Let me create an issue for it and we'll track to 4.x, I think.
There was a problem hiding this comment.
For the purpose of this PR, let's rename though.
innocenzi
left a comment
There was a problem hiding this comment.
Looks pretty good overall, I like the API. Not a full review, just a few nitpicks.
There was a problem hiding this comment.
Let's name it rate-limit.config.php
There was a problem hiding this comment.
I'm gussing the current rule goes that framework-owned configuration files are camelCase, whereas user-made ones are kebab-case?
- https://github.com/tempestphp/tempest-framework/blob/3.x/packages/command-bus/src/Config/commandBus.config.php
- https://github.com/tempestphp/tempest-framework/blob/3.x/packages/event-bus/src/Config/eventBus.config.php
- https://github.com/tempestphp/tempest-framework/blob/3.x/packages/router/src/Config/staticRoutes.config.php
Or we want to eventually move the framework-owned configs to be kebab-case as well?
There was a problem hiding this comment.
Good catch, we need to actually come to an agreement and document the convention here. Let's forget about that for now, we can always change it later, it's not gonna be a breaking change.
There was a problem hiding this comment.
@osbre you're bringing us to shame with our inconsistencies here. 🙈😅
| * ``` | ||
| */ | ||
| #[Attribute(Attribute::IS_REPEATABLE | Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)] | ||
| final readonly class ThrottleWith implements Throttles |
There was a problem hiding this comment.
Why not just add a profile property to Throttle? Would avoid to remember what both attributes do, and would avoid the awkward trait as well.
There was a problem hiding this comment.
That was my initial idea. I split it because merging makes attempts nullable and needs a runtime check for the invalid combinations (neither set, both set, or profile combined with per/every/bucket) plus an exception type for it - whereas two attributes make those states unrepresentable by splitting responsibilities.
There was a problem hiding this comment.
Makes sense. In my opinion the runtime check is worth the improved DX though. Any opinion here @aidan-casey?
| * | ||
| * @var class-string<RateLimitStorage> | ||
| */ | ||
| public string $storageClass = CacheRateLimitStorage::class, |
There was a problem hiding this comment.
I'd rather have an interface for the config, and add a dedicated CacheRateLimitConfig implementation. We use that pattern in many places.
There was a problem hiding this comment.
Brilliant, thank you! Now we can also move lockTimeoutInSeconds into a Cache-specific config 👍
|
|
||
| public function __invoke(Request $request, HttpMiddlewareCallable $next): Response | ||
| { | ||
| if (! $this->config->enabled) { |
There was a problem hiding this comment.
What's the advantage of having this vs. just not adding the middleware?
There was a problem hiding this comment.
It was there to make preventThrottling() and allowThrottling() work in testing. I just realised we could achieve the same with UnlimitedRateLimiter as an alternative to the config parameter: a09d24c (this PR)
|
Nice work here! Like @innocenzi, I like the API. More of a side note, I am expecting we may have to consider potential edge cases with worker mode. There's a chance it may not be an issue given the storage drivers, but just making a mental note. |
This PR adds a
tempest/rate-limitpackage. It limits how often an action may happen within a window of time, using a fixed window algorithm. The design was initially inspired by Laravel's rate limiter.Counters live in the cache by default, so the package works out of the box with no extra infrastructure. Applications serving concurrent traffic can switch to Redis, which counts atomically.
Usage
#[Throttle]Limits how often a route may be requested. The attribute is repeatable, and is valid on both a controller method and the controller itself.
perandeverywiden it, as inper: Per::MINUTE, every: 5.bucketscopes the limit to the client and nothing else, so several routes naming the same bucket share one allowance.X-RateLimit-Limit,X-RateLimit-RemainingandX-RateLimit-Reset. Rejected ones get a429withRetry-After. Both can be switched off withincludeHeaders: false.#[ThrottleWith]andRateLimitProfileFor limits that depend on the request itself, such as a higher allowance for paying customers and none at all for internal traffic.
resolve(Request): RateLimit[]may return several limits, or an empty array to leave the request unlimited.Limits a profile returns are scoped exactly like the ones
#[Throttle]declares. Unkeyed, they get a counter per route and per client. Keyed withwithKey(), they become a shared bucket.#[Throttle]and#[ThrottleWith]are separate attributes rather than one attribute with mutually exclusive arguments. A constructor cannot be handed a combination the component has to reject at runtime. Both implementThrottles, which is what puts the middleware on the route and what the middleware collects.RateLimitKeyResolverDecides what is being counted. It defaults to
ClientIpKeyResolver. Implement the interface and pointkeyResolverClassat it to count by authenticated user, API key, or anything else. Returningnullputs the request in a shared "unidentified" bucket, so an unidentifiable client never escapes its limit.RateLimiterThe limiter is not tied to HTTP. Inject it and count attempts against any key.
RateLimit::perSecond(),perMinute(),perHour()andperDay(), each taking an optional multiplier.withKey()scopes a limit, andscopedTo()appends to an existing key.attempt()records an attempt, or several withby:, and returns aRateLimitResultexposingallowed,exceeded,limit,hits,remaining,retryAfterandresetsAt.peek()inspects without consuming, andclear()discards recorded attempts.throttle()runs a callback only when the limit allows it, throwingRateLimitWasExceededotherwise.RateLimitWasExceededis the caller's to handle. It is not converted into a429by the framework: the throw is howthrottle()reports a rejection, and code reaching for the limiter by hand has already opted out of#[Throttle]. A global middleware translating it would run on every request in every application to catch an exception most never throw, and would silently answer for callers who never asked it to. Catch it, or useattempt()and branch on the result.RateLimitStorageWhere windows are persisted.
CacheRateLimitStorage(the default) andRedisRateLimitStorageship with the package. PointstorageClassat your own implementation to store windows anywhere else.Configuration
RateLimitConfigships as a discovered config file and holds everything above:lockTimeoutInSecondsonly applies to the cache storage, which is the one that needs a lock.Testing
RateLimitTesteris available onIntegrationTestas$this->rateLimit.fake()swaps in in-memory storage, so tests need neither Redis nor a cache, and counters cannot leak between them.Design decisions
The algorithm
A window opens on the first attempt, and its end is fixed at that moment. Later attempts increment the counter but never push the end out, so hammering a limit cannot extend it. Once the window elapses the counter is gone, through cache expiry or Redis TTL, and the next attempt opens a fresh one.
attempt()records before evaluating, so a rejected attempt can never nudge the window.The known trade-off is the boundary burst. Under
perMinute(60), 60 requests at11:59:59and 60 more at12:00:00are both legal. This is inherent to the fixed window approach.Sliding window and token bucket are out of scope. The storage contract is fixed-window-shaped, so adding them later means changing
RateLimitStorage, not just adding aRateLimiter.Two interfaces
RateLimiteris the API surface, to inject and call, following theGeneric*convention used across the framework.RateLimitStorageis used for persistence, with three implementations and a config knob. Keeping them apart meansfind,incrementandremovenever leak into what users type-hint, and no storage backend is baked into the API.RateLimitStateis what gets persisted:hitsplusresetsAtInSecondsin a single value. A counter never exists without the window it belongs to.Storage
CacheRateLimitStorage(the default) works on any configured cache. Read-modify-write is not atomic there, so each increment takes a lock. It may undercount under simultaneous arrivals, and it is only as durable as the cache itself.RedisRateLimitStoragedoes increment-and-open-window in one Lua script. It counts correctly under concurrency and needs no lock. A window opens only when the TTL is negative, not when hits reset, so a reset counter does not get a second window.Redis scripts are sent with
EVALrather than cached withEVALSHA. They are a couple of hundred bytes, and the clients Tempest supports disagree on how a missing script is signalled.Both implementations derive their storage key through
RateLimitConfig::storageKey(), which applies the configured prefix and hashes withxxh128. A limit scoped to something unbounded cannot produce a key the store rejects.The HTTP path
ThrottlesextendsRouteDecorator, following the existingIdempotentattribute. At discovery it appendsThrottleMiddlewareto the route's middleware. The middleware is#[SkipDiscovery], which means it is never constructed for unthrottled routes, it shows up in the route's own middleware list for debugging, and#[WithoutMiddleware]disables an inherited controller-wide throttle.At request time the middleware:
Throttlesattributes from the handler method and its declaring class, and asks each to resolve the limits it subjects the request to.ThrottleCounterKey, which decides the counter it is spent from. A named bucket is scoped to the client alone. An unnamed one is scoped to the declaring class, the scope, the allowance it describes, and the client, plus the method and URI for route-scope limits. Controller scope omits those two, so every route it covers shares one counter. Identified clients are prefixed, so no resolver can return a value landing in the unidentified bucket. Limits landing in the same counter are collapsed, since one allowance declared twice is one limit.429s. The outcome does not depend on attribute declaration order.HttpRequestFailedwith the status and headers. Error responses are re-rendered from scratch, so the headers ride on the exception rather than being set on a response that would be discarded. This is what feat(router): allow error responses to carry headers #2267 enabled.