mirror of
https://github.com/BookStackApp/BookStack.git
synced 2024-11-23 19:32:29 +01:00
Added Backup code verification logic
Also added testing to cover as part of this in addition to adding the core backup code handling required. Also added the standardised translations for switching mfa mode and adding testing for this switching.
This commit is contained in:
parent
a3f19ebe96
commit
4597069083
@ -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
|
||||
|
@ -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)));
|
||||
}
|
||||
}
|
@ -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.
|
||||
*/
|
||||
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
@ -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()
|
||||
|
@ -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',
|
||||
];
|
@ -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.',
|
||||
|
@ -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
|
||||
|
||||
|
19
resources/views/mfa/verify/backup_codes.blade.php
Normal file
19
resources/views/mfa/verify/backup_codes.blade.php
Normal file
@ -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>
|
@ -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'))
|
||||
|
@ -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');
|
||||
|
@ -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];
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in New Issue
Block a user