Skip to content

Commit 62477b3

Browse files
[11.x] Throw exception if named rate limiter and model property do not exist (#50908)
* Add tests * Fix test * fix test * Account for worst case scenario on tests * naive approach * working impl * add a more specific message if the user is authenticated * Use `hasAttribute` method to deal with accessors * use null-safe operator * trigger ci * formatting * fix tests --------- Co-authored-by: Taylor Otwell <taylor@laravel.com>
1 parent 24f16ba commit 62477b3

File tree

3 files changed

+153
-4
lines changed

3 files changed

+153
-4
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace Illuminate\Routing\Exceptions;
4+
5+
use Exception;
6+
7+
class MissingRateLimiterException extends Exception
8+
{
9+
/**
10+
* Create a new exception for invalid named rate limiter.
11+
*
12+
* @param string $limiter
13+
* @return static
14+
*/
15+
public static function forLimiter(string $limiter)
16+
{
17+
return new static("Rate limiter [{$limiter}] is not defined.");
18+
}
19+
20+
/**
21+
* Create a new exception for an invalid rate limiter based on a model property.
22+
*
23+
* @param string $limiter
24+
* @param class-string $model
25+
* @return static
26+
*/
27+
public static function forLimiterAndUser(string $limiter, string $model)
28+
{
29+
return new static("Rate limiter [{$model}::{$limiter}] is not defined.");
30+
}
31+
}

src/Illuminate/Routing/Middleware/ThrottleRequests.php

+16-4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Cache\RateLimiting\Unlimited;
88
use Illuminate\Http\Exceptions\HttpResponseException;
99
use Illuminate\Http\Exceptions\ThrottleRequestsException;
10+
use Illuminate\Routing\Exceptions\MissingRateLimiterException;
1011
use Illuminate\Support\Arr;
1112
use Illuminate\Support\InteractsWithTime;
1213
use RuntimeException;
@@ -78,6 +79,7 @@ public static function with($maxAttempts = 60, $decayMinutes = 1, $prefix = '')
7879
* @return \Symfony\Component\HttpFoundation\Response
7980
*
8081
* @throws \Illuminate\Http\Exceptions\ThrottleRequestsException
82+
* @throws \Illuminate\Routing\Exceptions\MissingRateLimiterException
8183
*/
8284
public function handle($request, Closure $next, $maxAttempts = 60, $decayMinutes = 1, $prefix = '')
8385
{
@@ -175,17 +177,27 @@ protected function handleRequest($request, Closure $next, array $limits)
175177
* @param \Illuminate\Http\Request $request
176178
* @param int|string $maxAttempts
177179
* @return int
180+
* @throws \Illuminate\Routing\Exceptions\MissingRateLimiterException
178181
*/
179182
protected function resolveMaxAttempts($request, $maxAttempts)
180183
{
181184
if (str_contains($maxAttempts, '|')) {
182185
$maxAttempts = explode('|', $maxAttempts, 2)[$request->user() ? 1 : 0];
183186
}
184187

185-
if (! is_numeric($maxAttempts) && $request->user()) {
188+
if (! is_numeric($maxAttempts) &&
189+
$request->user()?->hasAttribute($maxAttempts)
190+
) {
186191
$maxAttempts = $request->user()->{$maxAttempts};
187192
}
188193

194+
// If we still don't have a numeric value, there was no matching rate limiter...
195+
if (! is_numeric($maxAttempts)) {
196+
is_null($request->user())
197+
? throw MissingRateLimiterException::forLimiter($maxAttempts)
198+
: throw MissingRateLimiterException::forLimiterAndUser($maxAttempts, get_class($request->user()));
199+
}
200+
189201
return (int) $maxAttempts;
190202
}
191203

@@ -271,9 +283,9 @@ protected function addHeaders(Response $response, $maxAttempts, $remainingAttemp
271283
* @return array
272284
*/
273285
protected function getHeaders($maxAttempts,
274-
$remainingAttempts,
275-
$retryAfter = null,
276-
?Response $response = null)
286+
$remainingAttempts,
287+
$retryAfter = null,
288+
?Response $response = null)
277289
{
278290
if ($response &&
279291
! is_null($response->headers->get('X-RateLimit-Remaining')) &&

tests/Integration/Http/ThrottleRequestsTest.php

+106
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,26 @@
66
use Illuminate\Cache\RateLimiting\GlobalLimit;
77
use Illuminate\Cache\RateLimiting\Limit;
88
use Illuminate\Container\Container;
9+
use Illuminate\Database\Eloquent\Model;
10+
use Illuminate\Foundation\Auth\User;
11+
use Illuminate\Foundation\Testing\RefreshDatabase;
912
use Illuminate\Http\Exceptions\ThrottleRequestsException;
13+
use Illuminate\Routing\Exceptions\MissingRateLimiterException;
1014
use Illuminate\Routing\Middleware\ThrottleRequests;
1115
use Illuminate\Support\Carbon;
1216
use Illuminate\Support\Facades\Route;
1317
use Orchestra\Testbench\Attributes\WithConfig;
18+
use Orchestra\Testbench\Attributes\WithMigration;
1419
use Orchestra\Testbench\TestCase;
1520
use PHPUnit\Framework\Attributes\DataProvider;
1621
use Throwable;
1722

1823
#[WithConfig('hashing.driver', 'bcrypt')]
24+
#[WithMigration]
1925
class ThrottleRequestsTest extends TestCase
2026
{
27+
use RefreshDatabase;
28+
2129
public function testLockOpensImmediatelyAfterDecay()
2230
{
2331
Carbon::setTestNow(Carbon::create(2018, 1, 1, 0, 0, 0));
@@ -233,4 +241,102 @@ public function testItCanThrottlePerSecond()
233241
$response = $this->get('/');
234242
$response->assertOk();
235243
}
244+
245+
public function testItFailsIfNamedLimiterDoesNotExist()
246+
{
247+
$this->expectException(MissingRateLimiterException::class);
248+
$this->expectExceptionMessage('Rate limiter [test] is not defined.');
249+
250+
Route::get('/', fn () => 'ok')->middleware(ThrottleRequests::using('test'));
251+
252+
$this->withoutExceptionHandling()->get('/');
253+
}
254+
255+
public function testItFailsIfNamedLimiterDoesNotExistAndAuthenticatedUserDoesNotHaveFallbackProperty()
256+
{
257+
$this->expectException(MissingRateLimiterException::class);
258+
$this->expectExceptionMessage('Rate limiter [' . User::class . '::rateLimiting] is not defined.');
259+
260+
Route::get('/', fn () => 'ok')->middleware(['auth', ThrottleRequests::using('rateLimiting')]);
261+
262+
// The reason we're enabling strict mode and actually creating a user is to ensure we never even try to access
263+
// a property within the user model that does not exist. If an application is in strict mode and there is
264+
// no matching rate limiter, it should throw a rate limiter exception, not a property access exception.
265+
Model::shouldBeStrict();
266+
$user = User::forceCreate([
267+
'name' => 'Mateus',
268+
'email' => 'mateus@example.org',
269+
'password' => 'password',
270+
]);
271+
272+
$this->withoutExceptionHandling()->actingAs($user)->get('/');
273+
}
274+
275+
public function testItFallbacksToUserPropertyWhenThereIsNoNamedLimiterWhenAuthenticated()
276+
{
277+
$user = User::make()->forceFill([
278+
'rateLimiting' => 1,
279+
]);
280+
281+
Carbon::setTestNow(Carbon::create(2018, 1, 1, 0, 0, 0));
282+
283+
// The `rateLimiting` named limiter does not exist, but the `rateLimiting` property on the
284+
// User model does, so it should fallback to that property within the authenticated model.
285+
Route::get('/', fn () => 'yes')->middleware(['auth', ThrottleRequests::using('rateLimiting')]);
286+
287+
$response = $this->withoutExceptionHandling()->actingAs($user)->get('/');
288+
$this->assertSame('yes', $response->getContent());
289+
$this->assertEquals(1, $response->headers->get('X-RateLimit-Limit'));
290+
$this->assertEquals(0, $response->headers->get('X-RateLimit-Remaining'));
291+
292+
Carbon::setTestNow(Carbon::create(2018, 1, 1, 0, 0, 58));
293+
294+
try {
295+
$this->withoutExceptionHandling()->actingAs($user)->get('/');
296+
} catch (Throwable $e) {
297+
$this->assertInstanceOf(ThrottleRequestsException::class, $e);
298+
$this->assertEquals(429, $e->getStatusCode());
299+
$this->assertEquals(1, $e->getHeaders()['X-RateLimit-Limit']);
300+
$this->assertEquals(0, $e->getHeaders()['X-RateLimit-Remaining']);
301+
$this->assertEquals(2, $e->getHeaders()['Retry-After']);
302+
$this->assertEquals(Carbon::now()->addSeconds(2)->getTimestamp(), $e->getHeaders()['X-RateLimit-Reset']);
303+
}
304+
}
305+
306+
public function testItFallbacksToUserAccessorWhenThereIsNoNamedLimiterWhenAuthenticated()
307+
{
308+
$user = UserWithAcessor::make();
309+
310+
Carbon::setTestNow(Carbon::create(2018, 1, 1, 0, 0, 0));
311+
312+
// The `rateLimiting` named limiter does not exist, but the `rateLimiting` accessor (not property!)
313+
// on the User model does, so it should fallback to that accessor within the authenticated model.
314+
Route::get('/', fn () => 'yes')->middleware(['auth', ThrottleRequests::using('rateLimiting')]);
315+
316+
$response = $this->withoutExceptionHandling()->actingAs($user)->get('/');
317+
$this->assertSame('yes', $response->getContent());
318+
$this->assertEquals(1, $response->headers->get('X-RateLimit-Limit'));
319+
$this->assertEquals(0, $response->headers->get('X-RateLimit-Remaining'));
320+
321+
Carbon::setTestNow(Carbon::create(2018, 1, 1, 0, 0, 58));
322+
323+
try {
324+
$this->withoutExceptionHandling()->actingAs($user)->get('/');
325+
} catch (Throwable $e) {
326+
$this->assertInstanceOf(ThrottleRequestsException::class, $e);
327+
$this->assertEquals(429, $e->getStatusCode());
328+
$this->assertEquals(1, $e->getHeaders()['X-RateLimit-Limit']);
329+
$this->assertEquals(0, $e->getHeaders()['X-RateLimit-Remaining']);
330+
$this->assertEquals(2, $e->getHeaders()['Retry-After']);
331+
$this->assertEquals(Carbon::now()->addSeconds(2)->getTimestamp(), $e->getHeaders()['X-RateLimit-Reset']);
332+
}
333+
}
334+
}
335+
336+
class UserWithAcessor extends User
337+
{
338+
public function getRateLimitingAttribute(): int
339+
{
340+
return 1;
341+
}
236342
}

0 commit comments

Comments
 (0)