diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php index 86126c3b6..9823caafa 100644 --- a/app/Auth/Access/LoginService.php +++ b/app/Auth/Access/LoginService.php @@ -70,7 +70,7 @@ class LoginService */ public function reattemptLoginFor(User $user, string $method) { - if ($user->id !== $this->getLastLoginAttemptUser()) { + if ($user->id !== ($this->getLastLoginAttemptUser()->id ?? null)) { throw new Exception('Login reattempt user does align with current session state'); } diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php index 9f9ab29a5..ec0d5d563 100644 --- a/app/Auth/Access/Mfa/MfaValue.php +++ b/app/Auth/Access/Mfa/MfaValue.php @@ -44,10 +44,24 @@ class MfaValue extends Model $mfaVal->save(); } + /** + * Easily get the decrypted MFA value for the given user and method. + */ + public static function getValueForUser(User $user, string $method): ?string + { + /** @var MfaValue $mfaVal */ + $mfaVal = static::query() + ->where('user_id', '=', $user->id) + ->where('method', '=', $method) + ->first(); + + return $mfaVal ? $mfaVal->getValue() : null; + } + /** * Decrypt the value attribute upon access. */ - public function getValue(): string + protected function getValue(): string { return decrypt($this->value); } @@ -55,7 +69,7 @@ class MfaValue extends Model /** * Encrypt the value attribute upon access. */ - public function setValue($value): void + protected function setValue($value): void { $this->value = encrypt($value); } diff --git a/app/Http/Controllers/Auth/HandlesPartialLogins.php b/app/Http/Controllers/Auth/HandlesPartialLogins.php index f9bacb95d..4ce67d236 100644 --- a/app/Http/Controllers/Auth/HandlesPartialLogins.php +++ b/app/Http/Controllers/Auth/HandlesPartialLogins.php @@ -8,6 +8,9 @@ use BookStack\Exceptions\NotFoundException; trait HandlesPartialLogins { + /** + * @throws NotFoundException + */ protected function currentOrLastAttemptedUser(): User { $loginService = app()->make(LoginService::class); diff --git a/app/Http/Controllers/Auth/MfaTotpController.php b/app/Http/Controllers/Auth/MfaTotpController.php index 828af6b03..dd1651970 100644 --- a/app/Http/Controllers/Auth/MfaTotpController.php +++ b/app/Http/Controllers/Auth/MfaTotpController.php @@ -3,6 +3,8 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\Access\Mfa\MfaSession; use BookStack\Auth\Access\Mfa\MfaValue; use BookStack\Auth\Access\Mfa\TotpService; use BookStack\Auth\Access\Mfa\TotpValidationRule; @@ -61,4 +63,27 @@ class MfaTotpController extends Controller return redirect('/mfa/setup'); } + + /** + * Verify the MFA method submission on check. + * @throws NotFoundException + */ + public function verify(Request $request, LoginService $loginService, MfaSession $mfaSession) + { + $user = $this->currentOrLastAttemptedUser(); + $totpSecret = MfaValue::getValueForUser($user, MfaValue::METHOD_TOTP); + + $this->validate($request, [ + 'code' => [ + 'required', + 'max:12', 'min:4', + new TotpValidationRule($totpSecret), + ] + ]); + + $mfaSession->markVerifiedForUser($user); + $loginService->reattemptLoginFor($user, 'mfa-totp'); + + return redirect()->intended(); + } } diff --git a/resources/views/mfa/verify.blade.php b/resources/views/mfa/verify.blade.php index 58c0c42de..e818852c2 100644 --- a/resources/views/mfa/verify.blade.php +++ b/resources/views/mfa/verify.blade.php @@ -19,24 +19,15 @@ You'll need to set up at least one method before you gain access. </p> <div> - <a href="{{ url('/mfa/verify/totp') }}" class="button outline">Configure</a> + <a href="{{ url('/mfa/setup') }}" class="button outline">Configure</a> </div> @endif - <div class="setting-list"> - <div class="grid half gap-xl"> - <div> - <div class="setting-list-label">METHOD A</div> - <p class="small"> - ... - </p> - </div> - <div class="pt-m"> - <a href="{{ url('/mfa/verify/totp') }}" class="button outline">BUTTON</a> - </div> - </div> - </div> + @if($method) + <hr class="my-l"> + @include('mfa.verify.' . $method) + @endif @if(count($otherMethods) > 0) <hr class="my-l"> diff --git a/resources/views/mfa/verify/totp.blade.php b/resources/views/mfa/verify/totp.blade.php new file mode 100644 index 000000000..7ff691552 --- /dev/null +++ b/resources/views/mfa/verify/totp.blade.php @@ -0,0 +1,20 @@ +<div class="setting-list-label">Mobile App</div> + +<p class="small mb-m"> + Enter the code, generated using your mobile app, below: +</p> + +<form action="{{ url('/mfa/verify/totp') }}" method="post"> + {{ 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')) + <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/routes/web.php b/routes/web.php index 437b4b03b..029649685 100644 --- a/routes/web.php +++ b/routes/web.php @@ -235,6 +235,7 @@ Route::post('/mfa/totp-confirm', 'Auth\MfaTotpController@confirm'); 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'); // Social auth routes Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login'); diff --git a/tests/Auth/MfaVerificationTest.php b/tests/Auth/MfaVerificationTest.php new file mode 100644 index 000000000..f19700a5a --- /dev/null +++ b/tests/Auth/MfaVerificationTest.php @@ -0,0 +1,75 @@ +<?php + +namespace Tests\Auth; + +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\Access\Mfa\TotpService; +use BookStack\Auth\User; +use Illuminate\Support\Facades\Hash; +use PragmaRX\Google2FA\Google2FA; +use Tests\TestCase; +use Tests\TestResponse; + +class MfaVerificationTest extends TestCase +{ + public function test_totp_verification() + { + [$user, $secret, $loginResp] = $this->startTotpLogin(); + $loginResp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSee('Verify Access'); + $resp->assertSee('Enter the code, generated using your mobile app, below:'); + $resp->assertElementExists('form[action$="/mfa/verify/totp"] input[name="code"]'); + + $google2fa = new Google2FA(); + $resp = $this->post('/mfa/verify/totp', [ + 'code' => $google2fa->getCurrentOtp($secret), + ]); + $resp->assertRedirect('/'); + $this->assertEquals($user->id, auth()->user()->id); + } + + public function test_totp_verification_fails_on_missing_invalid_code() + { + [$user, $secret, $loginResp] = $this->startTotpLogin(); + + $resp = $this->get('/mfa/verify'); + $resp = $this->post('/mfa/verify/totp', [ + '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/totp', [ + 'code' => '123321', + ]); + $resp->assertRedirect('/mfa/verify'); + $resp = $this->get('/mfa/verify'); + + $resp->assertSeeText('The provided code is not valid or has expired.'); + $this->assertNull(auth()->user()); + } + + /** + * @return Array<User, string, TestResponse> + */ + protected function startTotpLogin(): array + { + $secret = $this->app->make(TotpService::class)->generateSecret(); + $user = $this->getEditor(); + $user->password = Hash::make('password'); + $user->save(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, $secret); + $loginResp = $this->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + return [$user, $secret, $loginResp]; + } + +} \ No newline at end of file