Added configurable API throttling, Handled API errors standardly

This commit is contained in:
Dan Brown 2020-01-18 15:03:28 +00:00
parent 1350136ca3
commit be554b9c79
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
9 changed files with 125 additions and 2 deletions

View File

@ -260,4 +260,7 @@ ALLOW_ROBOTS=null
# The default and maximum item-counts for listing API requests.
API_DEFAULT_ITEM_COUNT=100
API_MAX_ITEM_COUNT=500
API_MAX_ITEM_COUNT=500
# The number of API requests that can be made per minute by a single user.
API_REQUESTS_PER_MIN=180

View File

@ -17,4 +17,7 @@ return [
// The maximum number of items that can be returned in a listing API request.
'max_item_count' => env('API_MAX_ITEM_COUNT', 500),
// The number of API requests that can be made per minute by a single user.
'requests_per_minute' => env('API_REQUESTS_PER_MIN', 180)
];

View File

@ -7,6 +7,9 @@ use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Auth\AuthenticationException;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Validation\ValidationException;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@ -47,6 +50,10 @@ class Handler extends ExceptionHandler
*/
public function render($request, Exception $e)
{
if ($this->isApiRequest($request)) {
return $this->renderApiException($e);
}
// Handle notify exceptions which will redirect to the
// specified location then show a notification message.
if ($this->isExceptionType($e, NotifyException::class)) {
@ -70,6 +77,41 @@ class Handler extends ExceptionHandler
return parent::render($request, $e);
}
/**
* Check if the given request is an API request.
*/
protected function isApiRequest(Request $request): bool
{
return strpos($request->path(), 'api/') === 0;
}
/**
* Render an exception when the API is in use.
*/
protected function renderApiException(Exception $e): JsonResponse
{
$code = $e->getCode() === 0 ? 500 : $e->getCode();
$headers = [];
if ($e instanceof HttpException) {
$code = $e->getStatusCode();
$headers = $e->getHeaders();
}
$responseData = [
'error' => [
'message' => $e->getMessage(),
]
];
if ($e instanceof ValidationException) {
$responseData['error']['validation'] = $e->errors();
$code = $e->status;
}
$responseData['error']['code'] = $code;
return new JsonResponse($responseData, $code, $headers);
}
/**
* Check the exception chain to compare against the original exception type.
* @param Exception $e

View File

@ -32,7 +32,7 @@ class Kernel extends HttpKernel
\BookStack\Http\Middleware\GlobalViewData::class,
],
'api' => [
'throttle:60,1',
\BookStack\Http\Middleware\ThrottleApiRequests::class,
\BookStack\Http\Middleware\EncryptCookies::class,
\BookStack\Http\Middleware\StartSessionIfCookieExists::class,
\BookStack\Http\Middleware\ApiAuthenticate::class,

View File

@ -0,0 +1,18 @@
<?php
namespace BookStack\Http\Middleware;
use Illuminate\Routing\Middleware\ThrottleRequests as Middleware;
class ThrottleApiRequests extends Middleware
{
/**
* Resolve the number of attempts if the user is authenticated or not.
*/
protected function resolveMaxAttempts($request, $maxAttempts)
{
return (int) config('api.requests_per_minute');
}
}

View File

@ -50,5 +50,6 @@
<server name="APP_URL" value="http://bookstack.dev"/>
<server name="DEBUGBAR_ENABLED" value="false"/>
<server name="SAML2_ENABLED" value="false"/>
<server name="API_REQUESTS_PER_MIN" value="180"/>
</php>
</phpunit>

View File

@ -120,4 +120,29 @@ class ApiAuthTest extends TestCase
$resp->assertJson($this->errorResponse("The email address for the account in use needs to be confirmed", 401));
}
public function test_rate_limit_headers_active_on_requests()
{
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertHeader('x-ratelimit-limit', 180);
$resp->assertHeader('x-ratelimit-remaining', 179);
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertHeader('x-ratelimit-remaining', 178);
}
public function test_rate_limit_hit_gives_json_error()
{
config()->set(['api.requests_per_minute' => 1]);
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertStatus(200);
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertStatus(429);
$resp->assertHeader('x-ratelimit-remaining', 0);
$resp->assertHeader('retry-after');
$resp->assertJson([
'error' => [
'code' => 429,
]
]);
}
}

View File

@ -44,4 +44,15 @@ class ApiConfigTest extends TestCase
$resp->assertJsonCount(2, 'data');
}
public function test_requests_per_min_alters_rate_limit()
{
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertHeader('x-ratelimit-limit', 180);
config()->set(['api.requests_per_minute' => 10]);
$resp = $this->actingAsApiEditor()->get($this->endpoint);
$resp->assertHeader('x-ratelimit-limit', 10);
}
}

View File

@ -38,6 +38,26 @@ class BooksApiTest extends TestCase
$this->assertActivityExists('book_create', $newItem);
}
public function test_book_name_needed_to_create()
{
$this->actingAsApiEditor();
$details = [
'description' => 'A book created via the API',
];
$resp = $this->postJson($this->baseEndpoint, $details);
$resp->assertStatus(422);
$resp->assertJson([
"error" => [
"message" => "The given data was invalid.",
"validation" => [
"name" => ["The name field is required."]
],
"code" => 422,
],
]);
}
public function test_read_endpoint()
{
$this->actingAsApiEditor();