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

diff --git a/app/Access/Controllers/ForgotPasswordController.php b/app/Access/Controllers/ForgotPasswordController.php
index 86fbe8fa36798085cbd492e44870f1b81f017784..36dd977558b7c47fc93f5909ccece40ce8c36176 100644 (file)
--- a/app/Access/Controllers/ForgotPasswordController.php
+++ b/app/Access/Controllers/ForgotPasswordController.php
@@ -6,6 +6,7 @@ use BookStack\Activity\ActivityType;
 use BookStack\Http\Controller;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Password;
+use Illuminate\Support\Sleep;
 
 class ForgotPasswordController extends Controller
 {
@@ -32,6 +33,10 @@ class ForgotPasswordController extends Controller
             '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.

diff --git a/app/Access/Controllers/ResetPasswordController.php b/app/Access/Controllers/ResetPasswordController.php
index a8a45dddf1f87f4ca80fb13951fafdf58c4fceab..3af65d17fb607207180417b64df10bd92cbd52cc 100644 (file)
--- a/app/Access/Controllers/ResetPasswordController.php
+++ b/app/Access/Controllers/ResetPasswordController.php
@@ -15,14 +15,11 @@ use Illuminate\Validation\Rules\Password as PasswordRule;
 
 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;
     }
 
     /**

diff --git a/app/App/Providers/RouteServiceProvider.php b/app/App/Providers/RouteServiceProvider.php
index 3a155920eb4f681ee313a291a8741e43169d7d81..d7c1cb737569618d5e8eb022499ef0e9cb27fe0a 100644 (file)
--- a/app/App/Providers/RouteServiceProvider.php
+++ b/app/App/Providers/RouteServiceProvider.php
@@ -81,5 +81,9 @@ class RouteServiceProvider extends ServiceProvider
         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());
+        });
     }
 }

diff --git a/routes/web.php b/routes/web.php
index 03595288f92952ef873f89b329aca170bc6dd665..58b8f4e543107aec56185e4927460d3ce413442c 100644 (file)
--- a/routes/web.php
+++ b/routes/web.php
@@ -317,8 +317,8 @@ Route::get('/register/confirm', [AccessControllers\ConfirmEmailController::class
 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']);
@@ -338,16 +338,16 @@ Route::get('/oidc/callback', [AccessControllers\OidcController::class, 'callback
 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');

diff --git a/tests/Auth/RegistrationTest.php b/tests/Auth/RegistrationTest.php
index 60ae17573eb1560df60d0f34ea4dfda1c96183f9..42d1120e4a89bd70afda399fe8b538f557aafdc1 100644 (file)
--- a/tests/Auth/RegistrationTest.php
+++ b/tests/Auth/RegistrationTest.php
@@ -203,4 +203,33 @@ class RegistrationTest extends TestCase
         $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);
+    }
 }

diff --git a/tests/Auth/ResetPasswordTest.php b/tests/Auth/ResetPasswordTest.php
index d2af17b9cfb9318237966872f2c93d1f4a465ba9..026f8c5ba3f78a42f623e6fc9d6f1fc6c61a9598 100644 (file)
--- a/tests/Auth/ResetPasswordTest.php
+++ b/tests/Auth/ResetPasswordTest.php
@@ -4,11 +4,19 @@ namespace Tests\Auth;
 
 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();
@@ -75,6 +83,17 @@ class ResetPasswordTest extends TestCase
             ->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' => '[email protected]',
+        ]);
+
+        Sleep::assertSlept(function (CarbonInterval $duration): bool {
+            return $duration->totalMilliseconds > 999;
+        }, 1);
+    }
+
     public function test_reset_page_shows_sign_links()
     {
         $this->setSettings(['registration-enabled' => 'true']);
@@ -98,4 +117,27 @@ class ResetPasswordTest extends TestCase
         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' => '[email protected]',
+            ]);
+        }
+
+        $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);
+    }
 }

diff --git a/tests/Auth/UserInviteTest.php b/tests/Auth/UserInviteTest.php
index a9dee0007f5870e25a6cc65391d3f8eeb02347ac..434de6aa6e9343fc0234674554efcd3a5508b369 100644 (file)
--- a/tests/Auth/UserInviteTest.php
+++ b/tests/Auth/UserInviteTest.php
@@ -137,4 +137,24 @@ class UserInviteTest extends TestCase
         $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);
+    }
 }