Skip to content

Commit be554b9

Browse files
ssddanbrownssddanbrown
committed
Added configurable API throttling, Handled API errors standardly
1 parent 1350136 commit be554b9

9 files changed

Lines changed: 125 additions & 2 deletions

File tree

‎.env.example.complete‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,7 @@ ALLOW_ROBOTS=null
260260

261261
# The default and maximum item-counts for listing API requests.
262262
API_DEFAULT_ITEM_COUNT=100
263-
API_MAX_ITEM_COUNT=500
263+
API_MAX_ITEM_COUNT=500
264+
265+
# The number of API requests that can be made per minute by a single user.
266+
API_REQUESTS_PER_MIN=180

‎app/Config/api.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,7 @@
1717
// The maximum number of items that can be returned in a listing API request.
1818
'max_item_count' => env('API_MAX_ITEM_COUNT', 500),
1919

20+
// The number of API requests that can be made per minute by a single user.
21+
'requests_per_minute' => env('API_REQUESTS_PER_MIN', 180)
22+
2023
];

‎app/Exceptions/Handler.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
use Illuminate\Auth\AuthenticationException;
88
use Illuminate\Database\Eloquent\ModelNotFoundException;
99
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
10+
use Illuminate\Http\JsonResponse;
11+
use Illuminate\Http\Request;
12+
use Illuminate\Http\Response;
1013
use Illuminate\Validation\ValidationException;
1114
use Symfony\Component\HttpKernel\Exception\HttpException;
1215
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -47,6 +50,10 @@ public function report(Exception $e)
4750
*/
4851
public function render($request, Exception $e)
4952
{
53+
if ($this->isApiRequest($request)) {
54+
return $this->renderApiException($e);
55+
}
56+
5057
// Handle notify exceptions which will redirect to the
5158
// specified location then show a notification message.
5259
if ($this->isExceptionType($e, NotifyException::class)) {
@@ -70,6 +77,41 @@ public function render($request, Exception $e)
7077
return parent::render($request, $e);
7178
}
7279

80+
/**
81+
* Check if the given request is an API request.
82+
*/
83+
protected function isApiRequest(Request $request): bool
84+
{
85+
return strpos($request->path(), 'api/') === 0;
86+
}
87+
88+
/**
89+
* Render an exception when the API is in use.
90+
*/
91+
protected function renderApiException(Exception $e): JsonResponse
92+
{
93+
$code = $e->getCode() === 0 ? 500 : $e->getCode();
94+
$headers = [];
95+
if ($e instanceof HttpException) {
96+
$code = $e->getStatusCode();
97+
$headers = $e->getHeaders();
98+
}
99+
100+
$responseData = [
101+
'error' => [
102+
'message' => $e->getMessage(),
103+
]
104+
];
105+
106+
if ($e instanceof ValidationException) {
107+
$responseData['error']['validation'] = $e->errors();
108+
$code = $e->status;
109+
}
110+
111+
$responseData['error']['code'] = $code;
112+
return new JsonResponse($responseData, $code, $headers);
113+
}
114+
73115
/**
74116
* Check the exception chain to compare against the original exception type.
75117
* @param Exception $e

‎app/Http/Kernel.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class Kernel extends HttpKernel
3232
\BookStack\Http\Middleware\GlobalViewData::class,
3333
],
3434
'api' => [
35-
'throttle:60,1',
35+
\BookStack\Http\Middleware\ThrottleApiRequests::class,
3636
\BookStack\Http\Middleware\EncryptCookies::class,
3737
\BookStack\Http\Middleware\StartSessionIfCookieExists::class,
3838
\BookStack\Http\Middleware\ApiAuthenticate::class,

‎app/Http/Middleware/ThrottleApiRequests.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
namespace BookStack\Http\Middleware;
4+
5+
use Illuminate\Routing\Middleware\ThrottleRequests as Middleware;
6+
7+
class ThrottleApiRequests extends Middleware
8+
{
9+
10+
/**
11+
* Resolve the number of attempts if the user is authenticated or not.
12+
*/
13+
protected function resolveMaxAttempts($request, $maxAttempts)
14+
{
15+
return (int) config('api.requests_per_minute');
16+
}
17+
18+
}

‎phpunit.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,6 @@
5050
<server name="APP_URL" value="http://bookstack.dev"/>
5151
<server name="DEBUGBAR_ENABLED" value="false"/>
5252
<server name="SAML2_ENABLED" value="false"/>
53+
<server name="API_REQUESTS_PER_MIN" value="180"/>
5354
</php>
5455
</phpunit>

‎tests/Api/ApiAuthTest.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,29 @@ public function test_email_confirmation_checked_using_api_auth()
120120
$resp->assertJson($this->errorResponse("The email address for the account in use needs to be confirmed", 401));
121121
}
122122

123+
public function test_rate_limit_headers_active_on_requests()
124+
{
125+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
126+
$resp->assertHeader('x-ratelimit-limit', 180);
127+
$resp->assertHeader('x-ratelimit-remaining', 179);
128+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
129+
$resp->assertHeader('x-ratelimit-remaining', 178);
130+
}
131+
132+
public function test_rate_limit_hit_gives_json_error()
133+
{
134+
config()->set(['api.requests_per_minute' => 1]);
135+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
136+
$resp->assertStatus(200);
137+
138+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
139+
$resp->assertStatus(429);
140+
$resp->assertHeader('x-ratelimit-remaining', 0);
141+
$resp->assertHeader('retry-after');
142+
$resp->assertJson([
143+
'error' => [
144+
'code' => 429,
145+
]
146+
]);
147+
}
123148
}

‎tests/Api/ApiConfigTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,15 @@ public function test_max_item_count_limits_listing_requests()
4444
$resp->assertJsonCount(2, 'data');
4545
}
4646

47+
public function test_requests_per_min_alters_rate_limit()
48+
{
49+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
50+
$resp->assertHeader('x-ratelimit-limit', 180);
51+
52+
config()->set(['api.requests_per_minute' => 10]);
53+
54+
$resp = $this->actingAsApiEditor()->get($this->endpoint);
55+
$resp->assertHeader('x-ratelimit-limit', 10);
56+
}
57+
4758
}

‎tests/Api/BooksApiTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,26 @@ public function test_create_endpoint()
3838
$this->assertActivityExists('book_create', $newItem);
3939
}
4040

41+
public function test_book_name_needed_to_create()
42+
{
43+
$this->actingAsApiEditor();
44+
$details = [
45+
'description' => 'A book created via the API',
46+
];
47+
48+
$resp = $this->postJson($this->baseEndpoint, $details);
49+
$resp->assertStatus(422);
50+
$resp->assertJson([
51+
"error" => [
52+
"message" => "The given data was invalid.",
53+
"validation" => [
54+
"name" => ["The name field is required."]
55+
],
56+
"code" => 422,
57+
],
58+
]);
59+
}
60+
4161
public function test_read_endpoint()
4262
{
4363
$this->actingAsApiEditor();

0 commit comments

Comments
 (0)