Skip to content

Commit 69af9e0

Browse files
committed
Routes: Added throttling to a range of auth-related endpoints
Some already throttled in some means, but this adds a simple ip-based non-request-specific layer to many endpoints. Related to #4993
1 parent 72c5141 commit 69af9e0

File tree

7 files changed

+109
-12
lines changed

7 files changed

+109
-12
lines changed

app/Access/Controllers/ForgotPasswordController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use BookStack\Http\Controller;
77
use Illuminate\Http\Request;
88
use Illuminate\Support\Facades\Password;
9+
use Illuminate\Support\Sleep;
910

1011
class ForgotPasswordController extends Controller
1112
{
@@ -32,6 +33,10 @@ public function sendResetLinkEmail(Request $request)
3233
'email' => ['required', 'email'],
3334
]);
3435

36+
// Add random pause to the response to help avoid time-base sniffing
37+
// of valid resets via slower email send handling.
38+
Sleep::for(random_int(1000, 3000))->milliseconds();
39+
3540
// We will send the password reset link to this user. Once we have attempted
3641
// to send the link, we will examine the response then see the message we
3742
// need to show to the user. Finally, we'll send out a proper response.

app/Access/Controllers/ResetPasswordController.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,11 @@
1515

1616
class ResetPasswordController extends Controller
1717
{
18-
protected LoginService $loginService;
19-
20-
public function __construct(LoginService $loginService)
21-
{
18+
public function __construct(
19+
protected LoginService $loginService
20+
) {
2221
$this->middleware('guest');
2322
$this->middleware('guard:standard');
24-
25-
$this->loginService = $loginService;
2623
}
2724

2825
/**

app/App/Providers/RouteServiceProvider.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,9 @@ protected function configureRateLimiting(): void
8181
RateLimiter::for('api', function (Request $request) {
8282
return Limit::perMinute(60)->by($request->user()?->id ?: $request->ip());
8383
});
84+
85+
RateLimiter::for('public', function (Request $request) {
86+
return Limit::perMinute(10)->by($request->ip());
87+
});
8488
}
8589
}

routes/web.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,8 @@
317317
Route::get('/register/confirm/awaiting', [AccessControllers\ConfirmEmailController::class, 'showAwaiting']);
318318
Route::post('/register/confirm/resend', [AccessControllers\ConfirmEmailController::class, 'resend']);
319319
Route::get('/register/confirm/{token}', [AccessControllers\ConfirmEmailController::class, 'showAcceptForm']);
320-
Route::post('/register/confirm/accept', [AccessControllers\ConfirmEmailController::class, 'confirm']);
321-
Route::post('/register', [AccessControllers\RegisterController::class, 'postRegister']);
320+
Route::post('/register/confirm/accept', [AccessControllers\ConfirmEmailController::class, 'confirm'])->middleware('throttle:public');
321+
Route::post('/register', [AccessControllers\RegisterController::class, 'postRegister'])->middleware('throttle:public');
322322

323323
// SAML routes
324324
Route::post('/saml2/login', [AccessControllers\Saml2Controller::class, 'login']);
@@ -338,16 +338,16 @@
338338
Route::post('/oidc/logout', [AccessControllers\OidcController::class, 'logout']);
339339

340340
// User invitation routes
341-
Route::get('/register/invite/{token}', [AccessControllers\UserInviteController::class, 'showSetPassword']);
342-
Route::post('/register/invite/{token}', [AccessControllers\UserInviteController::class, 'setPassword']);
341+
Route::get('/register/invite/{token}', [AccessControllers\UserInviteController::class, 'showSetPassword'])->middleware('throttle:public');
342+
Route::post('/register/invite/{token}', [AccessControllers\UserInviteController::class, 'setPassword'])->middleware('throttle:public');
343343

344344
// Password reset link request routes
345345
Route::get('/password/email', [AccessControllers\ForgotPasswordController::class, 'showLinkRequestForm']);
346-
Route::post('/password/email', [AccessControllers\ForgotPasswordController::class, 'sendResetLinkEmail']);
346+
Route::post('/password/email', [AccessControllers\ForgotPasswordController::class, 'sendResetLinkEmail'])->middleware('throttle:public');
347347

348348
// Password reset routes
349349
Route::get('/password/reset/{token}', [AccessControllers\ResetPasswordController::class, 'showResetForm']);
350-
Route::post('/password/reset', [AccessControllers\ResetPasswordController::class, 'reset']);
350+
Route::post('/password/reset', [AccessControllers\ResetPasswordController::class, 'reset'])->middleware('throttle:public');
351351

352352
// Metadata routes
353353
Route::view('/help/wysiwyg', 'help.wysiwyg');

tests/Auth/RegistrationTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,4 +203,33 @@ public function test_registration_simple_honeypot_active()
203203
$resp = $this->followRedirects($resp);
204204
$this->withHtml($resp)->assertElementExists('form input[name="username"].text-neg');
205205
}
206+
207+
public function test_registration_endpoint_throttled()
208+
{
209+
$this->setSettings(['registration-enabled' => 'true']);
210+
211+
for ($i = 0; $i < 11; $i++) {
212+
$response = $this->post('/register/', [
213+
'name' => "Barry{$i}",
214+
'email' => "barry{$i}@example.com",
215+
'password' => "barryIsTheBest{$i}",
216+
]);
217+
auth()->logout();
218+
}
219+
220+
$response->assertStatus(429);
221+
}
222+
223+
public function test_registration_confirmation_throttled()
224+
{
225+
$this->setSettings(['registration-enabled' => 'true']);
226+
227+
for ($i = 0; $i < 11; $i++) {
228+
$response = $this->post('/register/confirm/accept', [
229+
'token' => "token{$i}",
230+
]);
231+
}
232+
233+
$response->assertStatus(429);
234+
}
206235
}

tests/Auth/ResetPasswordTest.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,19 @@
44

55
use BookStack\Access\Notifications\ResetPasswordNotification;
66
use BookStack\Users\Models\User;
7+
use Carbon\CarbonInterval;
78
use Illuminate\Support\Facades\Notification;
9+
use Illuminate\Support\Sleep;
810
use Tests\TestCase;
911

1012
class ResetPasswordTest extends TestCase
1113
{
14+
protected function setUp(): void
15+
{
16+
parent::setUp();
17+
Sleep::fake();
18+
}
19+
1220
public function test_reset_flow()
1321
{
1422
Notification::fake();
@@ -75,6 +83,17 @@ public function test_reset_flow_shows_success_message_even_if_wrong_password_to_
7583
->assertSee('The password reset token is invalid for this email address.');
7684
}
7785

86+
public function test_reset_request_with_not_found_user_still_has_delay()
87+
{
88+
$this->followingRedirects()->post('/password/email', [
89+
'email' => 'barrynotfoundrandomuser@example.com',
90+
]);
91+
92+
Sleep::assertSlept(function (CarbonInterval $duration): bool {
93+
return $duration->totalMilliseconds > 999;
94+
}, 1);
95+
}
96+
7897
public function test_reset_page_shows_sign_links()
7998
{
8099
$this->setSettings(['registration-enabled' => 'true']);
@@ -98,4 +117,27 @@ public function test_reset_request_is_throttled()
98117
Notification::assertSentTimes(ResetPasswordNotification::class, 1);
99118
$resp->assertSee('A password reset link will be sent to ' . $editor->email . ' if that email address is found in the system.');
100119
}
120+
121+
public function test_reset_request_with_not_found_user_is_throttled()
122+
{
123+
for ($i = 0; $i < 11; $i++) {
124+
$response = $this->post('/password/email', [
125+
'email' => 'barrynotfoundrandomuser@example.com',
126+
]);
127+
}
128+
129+
$response->assertStatus(429);
130+
}
131+
132+
public function test_reset_call_is_throttled()
133+
{
134+
for ($i = 0; $i < 11; $i++) {
135+
$response = $this->post('/password/reset', [
136+
'email' => "arandomuser{$i}@example.com",
137+
'token' => "randomtoken{$i}",
138+
]);
139+
}
140+
141+
$response->assertStatus(429);
142+
}
101143
}

tests/Auth/UserInviteTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,24 @@ public function test_token_expires_after_two_weeks()
137137
$setPasswordPageResp->assertRedirect('/password/email');
138138
$setPasswordPageResp->assertSessionHas('error', 'This invitation link has expired. You can instead try to reset your account password.');
139139
}
140+
141+
public function test_set_password_view_is_throttled()
142+
{
143+
for ($i = 0; $i < 11; $i++) {
144+
$response = $this->get("/register/invite/tokenhere{$i}");
145+
}
146+
147+
$response->assertStatus(429);
148+
}
149+
150+
public function test_set_password_post_is_throttled()
151+
{
152+
for ($i = 0; $i < 11; $i++) {
153+
$response = $this->post("/register/invite/tokenhere{$i}", [
154+
'password' => 'my test password',
155+
]);
156+
}
157+
158+
$response->assertStatus(429);
159+
}
140160
}

0 commit comments

Comments
 (0)