From 50f55b43087818cec41e110d2f0402ca1bba3b62 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 24 Feb 2022 13:10:10 -0800 Subject: [PATCH 1/3] Fixes broken API throttling Signed-off-by: snipe --- .env.example | 2 +- app/Http/Kernel.php | 1 - config/app.php | 11 +++++++++++ routes/api.php | 2 +- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.env.example b/.env.example index a631d4a1a2..cf2e488ece 100644 --- a/.env.example +++ b/.env.example @@ -153,4 +153,4 @@ IMPORT_TIME_LIMIT=600 IMPORT_MEMORY_LIMIT=500M REPORT_TIME_LIMIT=12000 REQUIRE_SAML=false - +API_THROTTLE_PER_MINUTE=120 \ No newline at end of file diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 69afddc74f..f94d390d76 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -45,7 +45,6 @@ class Kernel extends HttpKernel ], 'api' => [ - 'throttle:120,1', 'auth:api', ], ]; diff --git a/config/app.php b/config/app.php index f08334a6e4..f0e61c548c 100755 --- a/config/app.php +++ b/config/app.php @@ -425,4 +425,15 @@ return [ ], + /* + |-------------------------------------------------------------------------- + | API Throttling + |-------------------------------------------------------------------------- + | + | This value determines the number of API requests permitted per minute + | + */ + + 'api_throttle_per_minute' => env('API_THROTTLE_PER_MINUTE', 120), + ]; diff --git a/routes/api.php b/routes/api.php index 398af78bce..e6f841531b 100644 --- a/routes/api.php +++ b/routes/api.php @@ -14,7 +14,7 @@ use Illuminate\Http\Request; */ -Route::group(['prefix' => 'v1','namespace' => 'Api', 'middleware' => 'auth:api'], function () { +Route::group(['prefix' => 'v1','namespace' => 'Api', 'middleware' => ['api', 'throttle:'.config('app.api_throttle_per_minute').',1']], function () { Route::get('/', function() { From 2906a8944200a9c2228204de4adf9f45b8a9b15d Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 24 Feb 2022 13:10:34 -0800 Subject: [PATCH 2/3] Make the 429 error less stupid Signed-off-by: snipe --- app/Exceptions/Handler.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 3b396ee7c7..ced68c5bfd 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -84,10 +84,12 @@ class Handler extends ExceptionHandler switch ($e->getStatusCode()) { case '404': return response()->json(Helper::formatStandardApiResponse('error', null, $statusCode . ' endpoint not found'), 404); - case '405': + 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: - return response()->json(Helper::formatStandardApiResponse('error', null, $statusCode), 405); + return response()->json(Helper::formatStandardApiResponse('error', null, $statusCode), $statusCode); } } From 83f21d0ddf6809a44901bd37fb33c6f75d25942b Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 24 Feb 2022 13:41:16 -0800 Subject: [PATCH 3/3] Added a comment about why we use the middleware there Signed-off-by: snipe --- routes/api.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/routes/api.php b/routes/api.php index e6f841531b..eec8920d63 100644 --- a/routes/api.php +++ b/routes/api.php @@ -11,6 +11,10 @@ use Illuminate\Http\Request; | routes are loaded by the RouteServiceProvider within a group which | is assigned the "api" middleware group. Enjoy building your API! | +| We *could* put the middleware speficication in the RouteServiceProvider's mapApiRoutes() +| method, but we felt it was clearer to keep it here, since we look at the api routes for more +| often than we look at the RouteServiceProvider. - @snipe +| */