diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php
index 9823caafa..cc0cb06f3 100644
--- a/app/Auth/Access/LoginService.php
+++ b/app/Auth/Access/LoginService.php
@@ -37,6 +37,8 @@ class LoginService
throw new StoppedAuthenticationException($user, $this);
// TODO - Does 'remember' still work? Probably not right now.
+ // TODO - Need to clear MFA sessions out upon logout
+
// Old MFA middleware todos:
// TODO - Need to redirect to setup if not configured AND ONLY IF NO OPTIONS CONFIGURED
diff --git a/app/Auth/Access/Mfa/BackupCodeService.php b/app/Auth/Access/Mfa/BackupCodeService.php
index cc533bd31..6fbbf64b5 100644
--- a/app/Auth/Access/Mfa/BackupCodeService.php
+++ b/app/Auth/Access/Mfa/BackupCodeService.php
@@ -12,10 +12,55 @@ class BackupCodeService
public function generateNewSet(): array
{
$codes = [];
- for ($i = 0; $i < 16; $i++) {
+ while (count($codes) < 16) {
$code = Str::random(5) . '-' . Str::random(5);
- $codes[] = strtolower($code);
+ if (!in_array($code, $codes)) {
+ $codes[] = strtolower($code);
+ }
}
+
return $codes;
}
+
+ /**
+ * Check if the given code matches one of the available options.
+ */
+ public function inputCodeExistsInSet(string $code, string $codeSet): bool
+ {
+ $cleanCode = $this->cleanInputCode($code);
+ $codes = json_decode($codeSet);
+ return in_array($cleanCode, $codes);
+ }
+
+ /**
+ * Remove the given input code from the given available options.
+ * Will return null if no codes remain otherwise will be a JSON string to contain
+ * the codes.
+ */
+ public function removeInputCodeFromSet(string $code, string $codeSet): ?string
+ {
+ $cleanCode = $this->cleanInputCode($code);
+ $codes = json_decode($codeSet);
+ $pos = array_search($cleanCode, $codes, true);
+ array_splice($codes, $pos, 1);
+
+ if (count($codes) === 0) {
+ return null;
+ }
+
+ return json_encode($codes);
+ }
+
+ /**
+ * Count the number of codes in the given set.
+ */
+ public function countCodesInSet(string $codeSet): int
+ {
+ return count(json_decode($codeSet));
+ }
+
+ protected function cleanInputCode(string $code): string
+ {
+ return strtolower(str_replace(' ', '-', trim($code)));
+ }
}
\ No newline at end of file
diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php
index ec0d5d563..228a6d43f 100644
--- a/app/Auth/Access/Mfa/MfaValue.php
+++ b/app/Auth/Access/Mfa/MfaValue.php
@@ -58,6 +58,17 @@ class MfaValue extends Model
return $mfaVal ? $mfaVal->getValue() : null;
}
+ /**
+ * Delete any stored MFA values for the given user and method.
+ */
+ public static function deleteValuesForUser(User $user, string $method): void
+ {
+ static::query()
+ ->where('user_id', '=', $user->id)
+ ->where('method', '=', $method)
+ ->delete();
+ }
+
/**
* Decrypt the value attribute upon access.
*/
diff --git a/app/Http/Controllers/Auth/MfaBackupCodesController.php b/app/Http/Controllers/Auth/MfaBackupCodesController.php
index b89050565..1353d4562 100644
--- a/app/Http/Controllers/Auth/MfaBackupCodesController.php
+++ b/app/Http/Controllers/Auth/MfaBackupCodesController.php
@@ -3,10 +3,15 @@
namespace BookStack\Http\Controllers\Auth;
use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\LoginService;
use BookStack\Auth\Access\Mfa\BackupCodeService;
+use BookStack\Auth\Access\Mfa\MfaSession;
use BookStack\Auth\Access\Mfa\MfaValue;
+use BookStack\Exceptions\NotFoundException;
use BookStack\Http\Controllers\Controller;
use Exception;
+use Illuminate\Http\Request;
+use Illuminate\Validation\ValidationException;
class MfaBackupCodesController extends Controller
{
@@ -46,4 +51,39 @@ class MfaBackupCodesController extends Controller
$this->logActivity(ActivityType::MFA_SETUP_METHOD, 'backup-codes');
return redirect('/mfa/setup');
}
+
+ /**
+ * Verify the MFA method submission on check.
+ * @throws NotFoundException
+ * @throws ValidationException
+ */
+ public function verify(Request $request, BackupCodeService $codeService, MfaSession $mfaSession, LoginService $loginService)
+ {
+ $user = $this->currentOrLastAttemptedUser();
+ $codes = MfaValue::getValueForUser($user, MfaValue::METHOD_BACKUP_CODES) ?? '[]';
+
+ $this->validate($request, [
+ 'code' => [
+ 'required',
+ 'max:12', 'min:8',
+ function ($attribute, $value, $fail) use ($codeService, $codes) {
+ if (!$codeService->inputCodeExistsInSet($value, $codes)) {
+ $fail(trans('validation.backup_codes'));
+ }
+ }
+ ]
+ ]);
+
+ $updatedCodes = $codeService->removeInputCodeFromSet($request->get('code'), $codes);
+ MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, $updatedCodes);
+
+ $mfaSession->markVerifiedForUser($user);
+ $loginService->reattemptLoginFor($user, 'mfa-backup_codes');
+
+ if ($codeService->countCodesInSet($updatedCodes) < 5) {
+ $this->showWarningNotification('You have less than 5 backup codes remaining, Please generate and store a new set before you run out of codes to prevent being locked out of your account.');
+ }
+
+ return redirect()->intended();
+ }
}
diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php
index 6d868b6f3..75cd46eef 100644
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@@ -5,7 +5,7 @@ namespace BookStack\Http\Controllers\Auth;
use BookStack\Actions\ActivityType;
use BookStack\Auth\Access\Mfa\MfaValue;
use BookStack\Http\Controllers\Controller;
-use BookStack\Http\Request;
+use Illuminate\Http\Request;
class MfaController extends Controller
{
@@ -47,7 +47,6 @@ class MfaController extends Controller
*/
public function verify(Request $request)
{
- // TODO - Test this
$desiredMethod = $request->get('method');
$userMethods = $this->currentOrLastAttemptedUser()
->mfaValues()
diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php
index d64fce93a..2159cfb3b 100644
--- a/resources/lang/en/auth.php
+++ b/resources/lang/en/auth.php
@@ -73,5 +73,9 @@ return [
'user_invite_page_welcome' => 'Welcome to :appName!',
'user_invite_page_text' => 'To finalise your account and gain access you need to set a password which will be used to log-in to :appName on future visits.',
'user_invite_page_confirm_button' => 'Confirm Password',
- 'user_invite_success' => 'Password set, you now have access to :appName!'
+ 'user_invite_success' => 'Password set, you now have access to :appName!',
+
+ // Multi-factor Authentication
+ 'mfa_use_totp' => 'Verify using a mobile app',
+ 'mfa_use_backup_codes' => 'Verify using a backup code',
];
\ No newline at end of file
diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php
index 5796d04c6..1963b0df2 100644
--- a/resources/lang/en/validation.php
+++ b/resources/lang/en/validation.php
@@ -15,6 +15,7 @@ return [
'alpha_dash' => 'The :attribute may only contain letters, numbers, dashes and underscores.',
'alpha_num' => 'The :attribute may only contain letters and numbers.',
'array' => 'The :attribute must be an array.',
+ 'backup_codes' => 'The provided code is not valid or has already been used.',
'before' => 'The :attribute must be a date before :date.',
'between' => [
'numeric' => 'The :attribute must be between :min and :max.',
diff --git a/resources/views/mfa/verify.blade.php b/resources/views/mfa/verify.blade.php
index e818852c2..7cab2edc4 100644
--- a/resources/views/mfa/verify.blade.php
+++ b/resources/views/mfa/verify.blade.php
@@ -32,7 +32,9 @@
@if(count($otherMethods) > 0)
<hr class="my-l">
@foreach($otherMethods as $otherMethod)
- <a href="{{ url("/mfa/verify?method={$otherMethod}") }}">Use {{$otherMethod}}</a>
+ <div class="text-center">
+ <a href="{{ url("/mfa/verify?method={$otherMethod}") }}">{{ trans('auth.mfa_use_' . $otherMethod) }}</a>
+ </div>
@endforeach
@endif
diff --git a/resources/views/mfa/verify/backup_codes.blade.php b/resources/views/mfa/verify/backup_codes.blade.php
new file mode 100644
index 000000000..abfb8a9d0
--- /dev/null
+++ b/resources/views/mfa/verify/backup_codes.blade.php
@@ -0,0 +1,19 @@
+<div class="setting-list-label">Backup Code</div>
+
+<p class="small mb-m">
+ Enter one of your remaining backup codes below:
+</p>
+
+<form action="{{ url('/mfa/verify/backup_codes') }}" method="post">
+ {{ csrf_field() }}
+ <input type="text"
+ name="code"
+ placeholder="Enter backup code here"
+ class="input-fill-width {{ $errors->has('code') ? 'neg' : '' }}">
+ @if($errors->has('code'))
+ <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div>
+ @endif
+ <div class="mt-s text-right">
+ <button class="button">{{ trans('common.confirm') }}</button>
+ </div>
+</form>
\ No newline at end of file
diff --git a/resources/views/mfa/verify/totp.blade.php b/resources/views/mfa/verify/totp.blade.php
index 7ff691552..55784dc8f 100644
--- a/resources/views/mfa/verify/totp.blade.php
+++ b/resources/views/mfa/verify/totp.blade.php
@@ -8,7 +8,6 @@
{{ csrf_field() }}
<input type="text"
name="code"
- aria-labelledby="totp-verify-input-details"
placeholder="Provide your app generated code here"
class="input-fill-width {{ $errors->has('code') ? 'neg' : '' }}">
@if($errors->has('code'))
diff --git a/routes/web.php b/routes/web.php
index 029649685..eadbca5e8 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -236,6 +236,7 @@ Route::get('/mfa/backup-codes-generate', 'Auth\MfaBackupCodesController@generate
Route::post('/mfa/backup-codes-confirm', 'Auth\MfaBackupCodesController@confirm');
Route::get('/mfa/verify', 'Auth\MfaController@verify');
Route::post('/mfa/verify/totp', 'Auth\MfaTotpController@verify');
+Route::post('/mfa/verify/backup_codes', 'Auth\MfaBackupCodesController@verify');
// Social auth routes
Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login');
diff --git a/tests/Auth/MfaVerificationTest.php b/tests/Auth/MfaVerificationTest.php
index f19700a5a..3f272cffb 100644
--- a/tests/Auth/MfaVerificationTest.php
+++ b/tests/Auth/MfaVerificationTest.php
@@ -54,6 +54,114 @@ class MfaVerificationTest extends TestCase
$this->assertNull(auth()->user());
}
+ public function test_backup_code_verification()
+ {
+ [$user, $codes, $loginResp] = $this->startBackupCodeLogin();
+ $loginResp->assertRedirect('/mfa/verify');
+
+ $resp = $this->get('/mfa/verify');
+ $resp->assertSee('Verify Access');
+ $resp->assertSee('Backup Code');
+ $resp->assertSee('Enter one of your remaining backup codes below:');
+ $resp->assertElementExists('form[action$="/mfa/verify/backup_codes"] input[name="code"]');
+
+ $resp = $this->post('/mfa/verify/backup_codes', [
+ 'code' => $codes[1],
+ ]);
+
+ $resp->assertRedirect('/');
+ $this->assertEquals($user->id, auth()->user()->id);
+ // Ensure code no longer exists in available set
+ $userCodes = MfaValue::getValueForUser($user, MfaValue::METHOD_BACKUP_CODES);
+ $this->assertStringNotContainsString($codes[1], $userCodes);
+ $this->assertStringContainsString($codes[0], $userCodes);
+ }
+
+ public function test_backup_code_verification_fails_on_missing_or_invalid_code()
+ {
+ [$user, $codes, $loginResp] = $this->startBackupCodeLogin();
+
+ $resp = $this->get('/mfa/verify');
+ $resp = $this->post('/mfa/verify/backup_codes', [
+ 'code' => '',
+ ]);
+ $resp->assertRedirect('/mfa/verify');
+
+ $resp = $this->get('/mfa/verify');
+ $resp->assertSeeText('The code field is required.');
+ $this->assertNull(auth()->user());
+
+ $resp = $this->post('/mfa/verify/backup_codes', [
+ 'code' => 'ab123-ab456',
+ ]);
+ $resp->assertRedirect('/mfa/verify');
+
+ $resp = $this->get('/mfa/verify');
+ $resp->assertSeeText('The provided code is not valid or has already been used.');
+ $this->assertNull(auth()->user());
+ }
+
+ public function test_backup_code_verification_fails_on_attempted_code_reuse()
+ {
+ [$user, $codes, $loginResp] = $this->startBackupCodeLogin();
+
+ $this->post('/mfa/verify/backup_codes', [
+ 'code' => $codes[0],
+ ]);
+ $this->assertNotNull(auth()->user());
+ auth()->logout();
+ session()->flush();
+
+ $this->post('/login', ['email' => $user->email, 'password' => 'password']);
+ $this->get('/mfa/verify');
+ $resp = $this->post('/mfa/verify/backup_codes', [
+ 'code' => $codes[0],
+ ]);
+ $resp->assertRedirect('/mfa/verify');
+ $this->assertNull(auth()->user());
+
+ $resp = $this->get('/mfa/verify');
+ $resp->assertSeeText('The provided code is not valid or has already been used.');
+ }
+
+ public function test_backup_code_verification_shows_warning_when_limited_codes_remain()
+ {
+ [$user, $codes, $loginResp] = $this->startBackupCodeLogin(['abc12-def45', 'abc12-def46']);
+
+ $resp = $this->post('/mfa/verify/backup_codes', [
+ 'code' => $codes[0],
+ ]);
+ $resp = $this->followRedirects($resp);
+ $resp->assertSeeText('You have less than 5 backup codes remaining, Please generate and store a new set before you run out of codes to prevent being locked out of your account.');
+ }
+
+ public function test_both_mfa_options_available_if_set_on_profile()
+ {
+ $user = $this->getEditor();
+ $user->password = Hash::make('password');
+ $user->save();
+
+ MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'abc123');
+ MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, '["abc12-def456"]');
+
+ /** @var TestResponse $mfaView */
+ $mfaView = $this->followingRedirects()->post('/login', [
+ 'email' => $user->email,
+ 'password' => 'password',
+ ]);
+
+ // Totp shown by default
+ $mfaView->assertElementExists('form[action$="/mfa/verify/totp"] input[name="code"]');
+ $mfaView->assertElementContains('a[href$="/mfa/verify?method=backup_codes"]', 'Verify using a backup code');
+
+ // Ensure can view backup_codes view
+ $resp = $this->get('/mfa/verify?method=backup_codes');
+ $resp->assertElementExists('form[action$="/mfa/verify/backup_codes"] input[name="code"]');
+ $resp->assertElementContains('a[href$="/mfa/verify?method=totp"]', 'Verify using a mobile app');
+ }
+
+ // TODO !! - Test no-existing MFA
+
/**
* @return Array<User, string, TestResponse>
*/
@@ -72,4 +180,21 @@ class MfaVerificationTest extends TestCase
return [$user, $secret, $loginResp];
}
+ /**
+ * @return Array<User, string, TestResponse>
+ */
+ protected function startBackupCodeLogin($codes = ['kzzu6-1pgll','bzxnf-plygd','bwdsp-ysl51','1vo93-ioy7n','lf7nw-wdyka','xmtrd-oplac']): array
+ {
+ $user = $this->getEditor();
+ $user->password = Hash::make('password');
+ $user->save();
+ MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, json_encode($codes));
+ $loginResp = $this->post('/login', [
+ 'email' => $user->email,
+ 'password' => 'password',
+ ]);
+
+ return [$user, $codes, $loginResp];
+ }
+
}
\ No newline at end of file