Skip to content

Commit 66dcd1f

Browse files
committed
Merge branch 'v26-03' into release
2 parents d1bde2f + ef82119 commit 66dcd1f

8 files changed

Lines changed: 333 additions & 191 deletions

File tree

app/Access/Controllers/HandlesPartialLogins.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,10 @@ protected function currentOrLastAttemptedUser(): User
2222

2323
return $user;
2424
}
25+
26+
protected function clearLastAttemptedUser(): void
27+
{
28+
$loginService = app()->make(LoginService::class);
29+
$loginService->clearLastLoginAttempted();
30+
}
2531
}

app/Access/Controllers/MfaBackupCodesController.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use BookStack\Access\Mfa\BackupCodeService;
77
use BookStack\Access\Mfa\MfaSession;
88
use BookStack\Access\Mfa\MfaValue;
9+
use BookStack\Access\Mfa\MfaVerificationLimiter;
910
use BookStack\Activity\ActivityType;
1011
use BookStack\Exceptions\NotFoundException;
1112
use BookStack\Http\Controller;
@@ -19,6 +20,11 @@ class MfaBackupCodesController extends Controller
1920

2021
protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-backup-codes';
2122

23+
public function __construct(
24+
protected MfaVerificationLimiter $limiter,
25+
) {
26+
}
27+
2228
/**
2329
* Show a view that generates and displays backup codes.
2430
*/
@@ -71,6 +77,12 @@ public function confirm()
7177
public function verify(Request $request, BackupCodeService $codeService, MfaSession $mfaSession, LoginService $loginService)
7278
{
7379
$user = $this->currentOrLastAttemptedUser();
80+
$this->limiter->incrementAttempts($user, $request);
81+
if ($this->limiter->hasHitLimit($user, $request)) {
82+
$this->clearLastAttemptedUser();
83+
$this->limiter->throwException();
84+
}
85+
7486
$codes = MfaValue::getValueForUser($user, MfaValue::METHOD_BACKUP_CODES) ?? '[]';
7587

7688
$this->validate($request, [
@@ -89,6 +101,7 @@ function ($attribute, $value, $fail) use ($codeService, $codes) {
89101

90102
$mfaSession->markVerifiedForUser($user);
91103
$loginService->reattemptLoginFor($user);
104+
$this->limiter->decrementAttempts($user, $request);
92105

93106
if ($codeService->countCodesInSet($updatedCodes) < 5) {
94107
$this->showWarningNotification(trans('auth.mfa_backup_codes_usage_limit_warning'));

app/Access/Controllers/MfaTotpController.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use BookStack\Access\LoginService;
66
use BookStack\Access\Mfa\MfaSession;
77
use BookStack\Access\Mfa\MfaValue;
8+
use BookStack\Access\Mfa\MfaVerificationLimiter;
89
use BookStack\Access\Mfa\TotpService;
910
use BookStack\Access\Mfa\TotpValidationRule;
1011
use BookStack\Activity\ActivityType;
@@ -20,7 +21,8 @@ class MfaTotpController extends Controller
2021
protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-totp-secret';
2122

2223
public function __construct(
23-
protected TotpService $totp
24+
protected TotpService $totp,
25+
protected MfaVerificationLimiter $limiter,
2426
) {
2527
}
2628

@@ -86,6 +88,12 @@ public function confirm(Request $request)
8688
public function verify(Request $request, LoginService $loginService, MfaSession $mfaSession)
8789
{
8890
$user = $this->currentOrLastAttemptedUser();
91+
$this->limiter->incrementAttempts($user, $request);
92+
if ($this->limiter->hasHitLimit($user, $request)) {
93+
$this->clearLastAttemptedUser();
94+
$this->limiter->throwException();
95+
}
96+
8997
$totpSecret = MfaValue::getValueForUser($user, MfaValue::METHOD_TOTP);
9098

9199
$this->validate($request, [
@@ -98,6 +106,7 @@ public function verify(Request $request, LoginService $loginService, MfaSession
98106

99107
$mfaSession->markVerifiedForUser($user);
100108
$loginService->reattemptLoginFor($user);
109+
$this->limiter->decrementAttempts($user, $request);
101110

102111
return redirect()->intended();
103112
}

app/Access/LoginService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ protected function setLastLoginAttemptedForUser(User $user, string $method, bool
126126
/**
127127
* Clear the last login attempted session value.
128128
*/
129-
protected function clearLastLoginAttempted(): void
129+
public function clearLastLoginAttempted(): void
130130
{
131131
session()->remove(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY);
132132
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
namespace BookStack\Access\Mfa;
4+
5+
use BookStack\Exceptions\NotifyException;
6+
use BookStack\Users\Models\User;
7+
use Illuminate\Cache\RateLimiter;
8+
use Illuminate\Http\Request;
9+
use Illuminate\Http\Response;
10+
11+
/**
12+
* A rate limit specifically for MFA verification.
13+
* Limits across both the attempted user (on a tight limit) and the
14+
* request IP (on a less strict limit).
15+
*/
16+
class MfaVerificationLimiter
17+
{
18+
protected int $maxUserAttemptsPerMinute = 5;
19+
protected int $maxIpAttemptsPerMinute = 60;
20+
21+
public function __construct(
22+
protected RateLimiter $rateLimiter
23+
) {
24+
}
25+
26+
public function throwException(): never
27+
{
28+
throw new NotifyException(
29+
trans('auth.mfa_throttle', ['seconds' => 60]),
30+
'/login',
31+
Response::HTTP_TOO_MANY_REQUESTS
32+
);
33+
}
34+
35+
public function incrementAttempts(User $user, Request $request): void
36+
{
37+
$this->rateLimiter->hit($this->getUserKey($user));
38+
$this->rateLimiter->hit($this->getRequestKey($request));
39+
}
40+
41+
public function decrementAttempts(User $user, Request $request): void
42+
{
43+
$this->rateLimiter->decrement($this->getUserKey($user));
44+
$this->rateLimiter->decrement($this->getRequestKey($request));
45+
}
46+
47+
public function hasHitLimit(User $user, Request $request): bool
48+
{
49+
return $this->rateLimiter->tooManyAttempts($this->getUserKey($user), $this->maxUserAttemptsPerMinute + 1)
50+
|| $this->rateLimiter->tooManyAttempts($this->getRequestKey($request), $this->maxIpAttemptsPerMinute + 1);
51+
}
52+
53+
protected function getUserKey(User $user): string
54+
{
55+
return "mfa-attempt::user::{$user->id}";
56+
}
57+
58+
protected function getRequestKey(Request $request): string
59+
{
60+
return "mfa-attempt::request::{$request->ip()}";
61+
}
62+
}

0 commit comments

Comments
 (0)