Skip to content

Commit

Permalink
Routes: Added throttling to a range of auth-related endpoints
Browse files Browse the repository at this point in the history
Some already throttled in some means, but this adds a simple ip-based
non-request-specific layer to many endpoints.
Related to #4993
  • Loading branch information
ssddanbrown committed May 20, 2024
1 parent 72c5141 commit 69af9e0
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 12 deletions.
5 changes: 5 additions & 0 deletions app/Access/Controllers/ForgotPasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use BookStack\Http\Controller;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Password;
use Illuminate\Support\Sleep;

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

// Add random pause to the response to help avoid time-base sniffing
// of valid resets via slower email send handling.
Sleep::for(random_int(1000, 3000))->milliseconds();

// We will send the password reset link to this user. Once we have attempted
// to send the link, we will examine the response then see the message we
// need to show to the user. Finally, we'll send out a proper response.
Expand Down
9 changes: 3 additions & 6 deletions app/Access/Controllers/ResetPasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@

class ResetPasswordController extends Controller
{
protected LoginService $loginService;

public function __construct(LoginService $loginService)
{
public function __construct(
protected LoginService $loginService
) {
$this->middleware('guest');
$this->middleware('guard:standard');

$this->loginService = $loginService;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions app/App/Providers/RouteServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,9 @@ protected function configureRateLimiting(): void
RateLimiter::for('api', function (Request $request) {
return Limit::perMinute(60)->by($request->user()?->id ?: $request->ip());
});

RateLimiter::for('public', function (Request $request) {
return Limit::perMinute(10)->by($request->ip());
});
}
}
12 changes: 6 additions & 6 deletions routes/web.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@
Route::get('/register/confirm/awaiting', [AccessControllers\ConfirmEmailController::class, 'showAwaiting']);
Route::post('/register/confirm/resend', [AccessControllers\ConfirmEmailController::class, 'resend']);
Route::get('/register/confirm/{token}', [AccessControllers\ConfirmEmailController::class, 'showAcceptForm']);
Route::post('/register/confirm/accept', [AccessControllers\ConfirmEmailController::class, 'confirm']);
Route::post('/register', [AccessControllers\RegisterController::class, 'postRegister']);
Route::post('/register/confirm/accept', [AccessControllers\ConfirmEmailController::class, 'confirm'])->middleware('throttle:public');
Route::post('/register', [AccessControllers\RegisterController::class, 'postRegister'])->middleware('throttle:public');

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

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

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

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

// Metadata routes
Route::view('/help/wysiwyg', 'help.wysiwyg');
Expand Down
29 changes: 29 additions & 0 deletions tests/Auth/RegistrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,33 @@ public function test_registration_simple_honeypot_active()
$resp = $this->followRedirects($resp);
$this->withHtml($resp)->assertElementExists('form input[name="username"].text-neg');
}

public function test_registration_endpoint_throttled()
{
$this->setSettings(['registration-enabled' => 'true']);

for ($i = 0; $i < 11; $i++) {
$response = $this->post('/register/', [
'name' => "Barry{$i}",
'email' => "barry{$i}@example.com",
'password' => "barryIsTheBest{$i}",
]);
auth()->logout();
}

$response->assertStatus(429);
}

public function test_registration_confirmation_throttled()
{
$this->setSettings(['registration-enabled' => 'true']);

for ($i = 0; $i < 11; $i++) {
$response = $this->post('/register/confirm/accept', [
'token' => "token{$i}",
]);
}

$response->assertStatus(429);
}
}
42 changes: 42 additions & 0 deletions tests/Auth/ResetPasswordTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@

use BookStack\Access\Notifications\ResetPasswordNotification;
use BookStack\Users\Models\User;
use Carbon\CarbonInterval;
use Illuminate\Support\Facades\Notification;
use Illuminate\Support\Sleep;
use Tests\TestCase;

class ResetPasswordTest extends TestCase
{
protected function setUp(): void
{
parent::setUp();
Sleep::fake();
}

public function test_reset_flow()
{
Notification::fake();
Expand Down Expand Up @@ -75,6 +83,17 @@ public function test_reset_flow_shows_success_message_even_if_wrong_password_to_
->assertSee('The password reset token is invalid for this email address.');
}

public function test_reset_request_with_not_found_user_still_has_delay()
{
$this->followingRedirects()->post('/password/email', [
'email' => 'barrynotfoundrandomuser@example.com',
]);

Sleep::assertSlept(function (CarbonInterval $duration): bool {
return $duration->totalMilliseconds > 999;
}, 1);
}

public function test_reset_page_shows_sign_links()
{
$this->setSettings(['registration-enabled' => 'true']);
Expand All @@ -98,4 +117,27 @@ public function test_reset_request_is_throttled()
Notification::assertSentTimes(ResetPasswordNotification::class, 1);
$resp->assertSee('A password reset link will be sent to ' . $editor->email . ' if that email address is found in the system.');
}

public function test_reset_request_with_not_found_user_is_throttled()
{
for ($i = 0; $i < 11; $i++) {
$response = $this->post('/password/email', [
'email' => 'barrynotfoundrandomuser@example.com',
]);
}

$response->assertStatus(429);
}

public function test_reset_call_is_throttled()
{
for ($i = 0; $i < 11; $i++) {
$response = $this->post('/password/reset', [
'email' => "arandomuser{$i}@example.com",
'token' => "randomtoken{$i}",
]);
}

$response->assertStatus(429);
}
}
20 changes: 20 additions & 0 deletions tests/Auth/UserInviteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,24 @@ public function test_token_expires_after_two_weeks()
$setPasswordPageResp->assertRedirect('/password/email');
$setPasswordPageResp->assertSessionHas('error', 'This invitation link has expired. You can instead try to reset your account password.');
}

public function test_set_password_view_is_throttled()
{
for ($i = 0; $i < 11; $i++) {
$response = $this->get("/register/invite/tokenhere{$i}");
}

$response->assertStatus(429);
}

public function test_set_password_post_is_throttled()
{
for ($i = 0; $i < 11; $i++) {
$response = $this->post("/register/invite/tokenhere{$i}", [
'password' => 'my test password',
]);
}

$response->assertStatus(429);
}
}

0 comments on commit 69af9e0

Please sign in to comment.