diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index e340d70b09..173890b6c3 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -11,6 +11,7 @@ use Illuminate\Support\Facades\Log; use Throwable; use JsonException; use Carbon\Exceptions\InvalidFormatException; +use Illuminate\Http\Exceptions\ThrottleRequestsException; class Handler extends ExceptionHandler { @@ -107,11 +108,10 @@ class Handler extends ExceptionHandler $statusCode = $e->getStatusCode(); + // API throttle requests are handled in the RouteServiceProvider configureRateLimiting() method, so we don't need to handle them here switch ($e->getStatusCode()) { case '404': return response()->json(Helper::formatStandardApiResponse('error', null, $statusCode . ' endpoint not found'), 404); - case '429': - return response()->json(Helper::formatStandardApiResponse('error', null, 'Too many requests'), 429); case '405': return response()->json(Helper::formatStandardApiResponse('error', null, 'Method not allowed'), 405); default: @@ -201,6 +201,7 @@ class Handler extends ExceptionHandler */ public function register() { + $this->reportable(function (Throwable $e) { // }); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index b69e22e4f9..051f53fc02 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -52,6 +52,7 @@ class Kernel extends HttpKernel 'auth:api', \App\Http\Middleware\CheckLocale::class, \Illuminate\Routing\Middleware\SubstituteBindings::class, + \App\Http\Middleware\SetAPIResponseHeaders::class, ], 'health' => [ diff --git a/app/Http/Middleware/SetAPIResponseHeaders.php b/app/Http/Middleware/SetAPIResponseHeaders.php new file mode 100644 index 0000000000..ac277e785c --- /dev/null +++ b/app/Http/Middleware/SetAPIResponseHeaders.php @@ -0,0 +1,82 @@ +headers->get('X-RateLimit-Remaining')) && + (int) $response->headers->get('X-RateLimit-Remaining') <= (int) $remainingAttempts) { + $headers = []; + $headers['Retry-After'] = $retryAfter; // this is the only line we changed + $headers['X-RateLimit-Reset'] = $retryAfter; // this is the only line we changed + $headers['X-RateLimit-Reset-Timestamp'] = $this->availableAt($retryAfter); // this is the only line we changed + return $headers; + } + + $headers = [ + 'X-RateLimit-Limit' => $maxAttempts, + 'X-RateLimit-Remaining' => $remainingAttempts, + ]; + + if (! is_null($retryAfter)) { + $headers['Retry-After'] = $retryAfter; + $headers['X-RateLimit-Reset'] = $retryAfter; // this is the only line we changed + $headers['X-RateLimit-Reset-Timestamp'] = $this->availableAt($retryAfter); // this is the only line we changed + } + + return $headers; + } + + + + /** + * Handle an incoming request. + * + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @return mixed + */ + protected function handleRequest($request, Closure $next, array $limits) + { + foreach ($limits as $limit) { + if ($this->limiter->tooManyAttempts($limit->key, $limit->maxAttempts)) { + throw $this->buildException($request, $limit->key, $limit->maxAttempts, $limit->responseCallback); + } + + $this->limiter->hit($limit->key, $limit->decaySeconds); + } + + $response = $next($request); + + foreach ($limits as $limit) { + $response = $this->addHeaders( + $response, + $limit->maxAttempts, + $this->calculateRemainingAttempts($limit->key, $limit->maxAttempts), + $this->getTimeUntilNextRetry($limit->key) // this is the only line we changed + ); + } + + return $response; + } + +} \ No newline at end of file diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 276aaf1e4f..054ffabe3e 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -76,6 +76,8 @@ class RouteServiceProvider extends ServiceProvider /** * Configure the rate limiters for the application. * + * This ONLY fires on 429 responses. + * * https://laravel.com/docs/8.x/routing#rate-limiting * * @return void @@ -83,9 +85,17 @@ class RouteServiceProvider extends ServiceProvider protected function configureRateLimiting() { - // Rate limiter for API calls + // Rate limiter for API calls - this sends the correct API headers to show the user the remaining time they have to wait and gives them the 429 status code if they are throttled RateLimiter::for('api', function (Request $request) { - return Limit::perMinute(config('app.api_throttle_per_minute'))->by(optional($request->user())->id ?: $request->ip()); + return Limit::perMinute(config('app.api_throttle_per_minute'))->by(optional($request->user())->id ?: $request->ip()) + ->response(function ($request, $headers) { + return response()->json([ + 'status' => 'error', + 'messages' => 'Too many requests. Try again in '.$headers['Retry-After'].' seconds.', + 'status_code' => 429, + 'retryAfter' => $headers['Retry-After'] ?? 60, + ], 429, $headers); + }); }); // Rate limiter for forgotten password requests diff --git a/tests/Feature/ApiRateLimitTest.php b/tests/Feature/ApiRateLimitTest.php new file mode 100644 index 0000000000..1239c00727 --- /dev/null +++ b/tests/Feature/ApiRateLimitTest.php @@ -0,0 +1,42 @@ + 10]); + $this->actingAsForApi(User::factory()->create()) + ->getJson(route('api.users.me')) + ->assertOk() + ->assertHeader('X-Ratelimit-Limit', config('app.api_throttle_per_minute')) + ->assertHeader('X-Ratelimit-Remaining', 9); + } + + public function testRateLimitDecreasesRemaining() + { + config(['app.api_throttle_per_minute' => 5]); + $expected_remaining = (config('app.api_throttle_per_minute') - 1); + $admin = User::factory()->create(); + + for ($x = 0; $x < 5; $x++) { + + $this->actingAsForApi($admin) + ->getJson(route('api.users.me')) + ->assertOk() + ->assertHeader('X-Ratelimit-Remaining', $expected_remaining--); + + } + + $this->actingAsForApi($admin) + ->getJson(route('api.users.me')) + ->assertStatus(429) + ->assertHeader('Retry-After', 60); + } + +}