]> BookStack Code Mirror - bookstack/commitdiff
Worked on MFA setup required flow
authorDan Brown <redacted>
Mon, 2 Aug 2021 21:02:25 +0000 (22:02 +0100)
committerDan Brown <redacted>
Mon, 2 Aug 2021 21:02:25 +0000 (22:02 +0100)
- Restructured some of the route naming to be a little more consistent.
- Moved the routes about to be more logically in one place.
- Created a new middleware to handle the auth of people that should be
  allowed access to mfa setup routes, since these could be used by
  existing logged in users or by people needing to setup MFA on access.
- Added testing to cover MFA setup required flow.
- Added TTL and method tracking to session last-login tracking system.

15 files changed:
app/Auth/Access/LoginService.php
app/Auth/Access/Mfa/MfaSession.php
app/Http/Controllers/Auth/MfaBackupCodesController.php
app/Http/Controllers/Auth/MfaTotpController.php
app/Http/Kernel.php
app/Http/Middleware/Authenticate.php
app/Http/Middleware/AuthenticatedOrPendingMfa.php [new file with mode: 0644]
resources/views/mfa/backup-codes-generate.blade.php
resources/views/mfa/setup.blade.php
resources/views/mfa/totp-generate.blade.php
resources/views/mfa/verify/backup_codes.blade.php
resources/views/mfa/verify/totp.blade.php
routes/web.php
tests/Auth/MfaConfigurationTest.php
tests/Auth/MfaVerificationTest.php

index cc0cb06f330ee3a35e27aa1ff10c1c04dc33c72f..f6ea7517f75c65bd3a66cfe499ec3c02a4344594 100644 (file)
@@ -10,6 +10,7 @@ use BookStack\Facades\Activity;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
 use Exception;
+use phpDocumentor\Reflection\DocBlock\Tags\Method;
 
 class LoginService
 {
@@ -33,7 +34,7 @@ class LoginService
     public function login(User $user, string $method): void
     {
         if ($this->awaitingEmailConfirmation($user) || $this->needsMfaVerification($user)) {
-            $this->setLastLoginAttemptedForUser($user);
+            $this->setLastLoginAttemptedForUser($user, $method);
             throw new StoppedAuthenticationException($user, $this);
             // TODO - Does 'remember' still work? Probably not right now.
 
@@ -41,12 +42,6 @@ class LoginService
 
             // Old MFA middleware todos:
 
-            // TODO - Need to redirect to setup if not configured AND ONLY IF NO OPTIONS CONFIGURED
-            //    Might need to change up such routes to start with /configure/ for such identification.
-            //    (Can't allow access to those if already configured)
-            //    Or, More likely, Need to add defence to those to prevent access unless
-            //    logged in or during partial auth.
-
             // TODO - Handle email confirmation handling
             //  Left BookStack\Http\Middleware\Authenticate@emailConfirmationErrorResponse in which needs
             //  be removed as an example of old behaviour.
@@ -70,13 +65,13 @@ class LoginService
      * Reattempt a system login after a previous stopped attempt.
      * @throws Exception
      */
-    public function reattemptLoginFor(User $user, string $method)
+    public function reattemptLoginFor(User $user)
     {
         if ($user->id !== ($this->getLastLoginAttemptUser()->id ?? null)) {
             throw new Exception('Login reattempt user does align with current session state');
         }
 
-        $this->login($user, $method);
+        $this->login($user, $this->getLastLoginAttemptMethod());
     }
 
     /**
@@ -86,12 +81,38 @@ class LoginService
      */
     public function getLastLoginAttemptUser(): ?User
     {
-        $id = session()->get(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY);
-        if (!$id) {
-            return null;
+        $id = $this->getLastLoginAttemptDetails()['user_id'];
+        return User::query()->where('id', '=', $id)->first();
+    }
+
+    /**
+     * Get the method for the last login attempt.
+     */
+    protected function getLastLoginAttemptMethod(): ?string
+    {
+        return $this->getLastLoginAttemptDetails()['method'];
+    }
+
+    /**
+     * Get the details of the last login attempt.
+     * Checks upon a ttl of about 1 hour since that last attempted login.
+     * @return array{user_id: ?string, method: ?string}
+     */
+    protected function getLastLoginAttemptDetails(): array
+    {
+        $value = session()->get(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY);
+        if (!$value) {
+            return ['user_id' => null, 'method' => null];
         }
 
-        return User::query()->where('id', '=', $id)->first();
+        [$id, $method, $time] = explode(':', $value);
+        $hourAgo = time() - (60*60);
+        if ($time < $hourAgo) {
+            $this->clearLastLoginAttempted();
+            return ['user_id' => null, 'method' => null];
+        }
+
+        return ['user_id' => $id, 'method' => $method];
     }
 
     /**
@@ -99,9 +120,12 @@ class LoginService
      * Must be only used when credentials are correct and a login could be
      * achieved but a secondary factor has stopped the login.
      */
-    protected function setLastLoginAttemptedForUser(User $user)
+    protected function setLastLoginAttemptedForUser(User $user, string $method)
     {
-        session()->put(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY, $user->id);
+        session()->put(
+            self::LAST_LOGIN_ATTEMPTED_SESSION_KEY,
+            implode(':', [$user->id, $method, time()])
+        );
     }
 
     /**
index dabd568f7e80b1941289f178c997b5909dae3bc4..8821dbb9d13522791a9b87271722e61aeb785f76 100644 (file)
@@ -15,6 +15,15 @@ class MfaSession
         return $user->mfaValues()->exists() || $this->userRoleEnforcesMfa($user);
     }
 
+    /**
+     * Check if the given user is pending MFA setup.
+     * (MFA required but not yet configured).
+     */
+    public function isPendingMfaSetup(User $user): bool
+    {
+        return $this->isRequiredForUser($user) && !$user->mfaValues()->exists();
+    }
+
     /**
      * Check if a role of the given user enforces MFA.
      */
index 1353d4562870658fe08845b1ce5deeed4109932b..41c161d7c2145ebb5e28446b3e24f6bd336f37bc 100644 (file)
@@ -78,7 +78,7 @@ class MfaBackupCodesController extends Controller
         MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, $updatedCodes);
 
         $mfaSession->markVerifiedForUser($user);
-        $loginService->reattemptLoginFor($user, 'mfa-backup_codes');
+        $loginService->reattemptLoginFor($user);
 
         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.');
index dd1651970d60c787300fcf4a1e40a2cc88829839..a1701c4cea758e7c8cb8c6ea139668acdc4d3e0e 100644 (file)
@@ -82,7 +82,7 @@ class MfaTotpController extends Controller
         ]);
 
         $mfaSession->markVerifiedForUser($user);
-        $loginService->reattemptLoginFor($user, 'mfa-totp');
+        $loginService->reattemptLoginFor($user);
 
         return redirect()->intended();
     }
index 4f9bfc1e64050dcd7a244c12b453db9ccc04461d..d1f5de917123c7eba976e6533e1f10e65268b52e 100644 (file)
@@ -53,5 +53,6 @@ class Kernel extends HttpKernel
         'throttle'   => \Illuminate\Routing\Middleware\ThrottleRequests::class,
         'perm'       => \BookStack\Http\Middleware\PermissionMiddleware::class,
         'guard'      => \BookStack\Http\Middleware\CheckGuard::class,
+        'mfa-setup'  => \BookStack\Http\Middleware\AuthenticatedOrPendingMfa::class,
     ];
 }
index c687c75a205ca925eaf88f88725498799d74c3bf..b9b5545f2a541a302e6a60b81985009f4dc62723 100644 (file)
@@ -15,9 +15,8 @@ class Authenticate
         if (!hasAppAccess()) {
             if ($request->ajax()) {
                 return response('Unauthorized.', 401);
-            } else {
-                return redirect()->guest(url('/login'));
             }
+            return redirect()->guest(url('/login'));
         }
 
         return $next($request);
diff --git a/app/Http/Middleware/AuthenticatedOrPendingMfa.php b/app/Http/Middleware/AuthenticatedOrPendingMfa.php
new file mode 100644 (file)
index 0000000..2d68a2a
--- /dev/null
@@ -0,0 +1,41 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use BookStack\Auth\Access\LoginService;
+use BookStack\Auth\Access\Mfa\MfaSession;
+use Closure;
+
+class AuthenticatedOrPendingMfa
+{
+
+    protected $loginService;
+    protected $mfaSession;
+
+    public function __construct(LoginService $loginService, MfaSession $mfaSession)
+    {
+        $this->loginService = $loginService;
+        $this->mfaSession = $mfaSession;
+    }
+
+
+    /**
+     * Handle an incoming request.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @param  \Closure  $next
+     * @return mixed
+     */
+    public function handle($request, Closure $next)
+    {
+        $user = auth()->user();
+        $loggedIn = $user !== null;
+        $lastAttemptUser = $this->loginService->getLastLoginAttemptUser();
+
+        if ($loggedIn || ($lastAttemptUser && $this->mfaSession->isPendingMfaSetup($lastAttemptUser))) {
+            return $next($request);
+        }
+
+        return redirect()->guest(url('/login'));
+    }
+}
index 8b437846e93544104908a170f1bfc3632cfffa6e..6a491fc07f68d40ca6209c6e28e66ccdec8df799 100644 (file)
@@ -27,7 +27,7 @@
                 Each code can only be used once
             </p>
 
-            <form action="{{ url('/mfa/backup-codes-confirm') }}" method="POST">
+            <form action="{{ url('/mfa/backup_codes/confirm') }}" method="POST">
                 {{ csrf_field() }}
                 <div class="mt-s text-right">
                     <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.cancel') }}</a>
index 2ec8d0f775254f3a4ad4fff45bbe272eb8e9ff08..f670541bfe61b41fade254eb9bfdc6c3459c2db5 100644 (file)
@@ -25,7 +25,7 @@
                                 @icon('check-circle')
                                 Already configured
                             </div>
-                            <a href="{{ url('/mfa/totp-generate') }}" class="button outline small">Reconfigure</a>
+                            <a href="{{ url('/mfa/totp/generate') }}" class="button outline small">Reconfigure</a>
                             <div component="dropdown" class="inline relative">
                                 <button type="button" refs="dropdown@toggle" class="button outline small">Remove</button>
                                 <div refs="dropdown@menu" class="dropdown-menu">
@@ -38,7 +38,7 @@
                                 </div>
                             </div>
                         @else
-                            <a href="{{ url('/mfa/totp-generate') }}" class="button outline">Setup</a>
+                            <a href="{{ url('/mfa/totp/generate') }}" class="button outline">Setup</a>
                         @endif
                     </div>
                 </div>
@@ -57,7 +57,7 @@
                                 @icon('check-circle')
                                 Already configured
                             </div>
-                            <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline small">Reconfigure</a>
+                            <a href="{{ url('/mfa/backup_codes/generate') }}" class="button outline small">Reconfigure</a>
                             <div component="dropdown" class="inline relative">
                                 <button type="button" refs="dropdown@toggle" class="button outline small">Remove</button>
                                 <div refs="dropdown@menu" class="dropdown-menu">
@@ -70,7 +70,7 @@
                                 </div>
                             </div>
                         @else
-                            <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline">Setup</a>
+                            <a href="{{ url('/mfa/backup_codes/generate') }}" class="button outline">Setup</a>
                         @endif
                     </div>
                 </div>
index c1e7547d5b9be3f49bdc6f2a113d1e70bc126cb8..2ed2677d1b48508c380e3fb35c806507276e8d4d 100644 (file)
@@ -24,7 +24,7 @@
                 Verify that all is working by entering a code, generated within your
                 authentication app, in the input box below:
             </p>
-            <form action="{{ url('/mfa/totp-confirm') }}" method="POST">
+            <form action="{{ url('/mfa/totp/confirm') }}" method="POST">
                 {{ csrf_field() }}
                 <input type="text"
                        name="code"
index abfb8a9d0b0f1b8eabf9713477a5ede0bc657186..7179696cb3e95e2f153017cca6b456ae4770b085 100644 (file)
@@ -4,7 +4,7 @@
     Enter one of your remaining backup codes below:
 </p>
 
-<form action="{{ url('/mfa/verify/backup_codes') }}" method="post">
+<form action="{{ url('/mfa/backup_codes/verify') }}" method="post">
     {{ csrf_field() }}
     <input type="text"
            name="code"
index 55784dc8f9650284a77e3f33245ad6c4728e76f8..fffd3035229a1a935673aa102d796f24927377ac 100644 (file)
@@ -4,7 +4,7 @@
     Enter the code, generated using your mobile app, below:
 </p>
 
-<form action="{{ url('/mfa/verify/totp') }}" method="post">
+<form action="{{ url('/mfa/totp/verify') }}" method="post">
     {{ csrf_field() }}
     <input type="text"
            name="code"
index eadbca5e8fd3bdf3f1432b03205829ae21e0c683..b6590429c3002c6bb088f7e5c962e195f34655bf 100644 (file)
@@ -224,26 +224,27 @@ Route::group(['middleware' => 'auth'], function () {
         Route::put('/roles/{id}', 'RoleController@update');
     });
 
-    // MFA (Auth Mandatory)
-    Route::delete('/mfa/remove/{method}', 'Auth\MfaController@remove');
 });
 
-// MFA (Auth Optional)
-Route::get('/mfa/setup', 'Auth\MfaController@setup');
-Route::get('/mfa/totp-generate', 'Auth\MfaTotpController@generate');
-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');
-Route::post('/mfa/verify/backup_codes', 'Auth\MfaBackupCodesController@verify');
+// MFA routes
+Route::group(['middleware' => 'mfa-setup'], function() {
+    Route::get('/mfa/setup', 'Auth\MfaController@setup');
+    Route::get('/mfa/totp/generate', 'Auth\MfaTotpController@generate');
+    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::group(['middleware' => 'guest'], function() {
+    Route::get('/mfa/verify', 'Auth\MfaController@verify');
+    Route::post('/mfa/totp/verify', 'Auth\MfaTotpController@verify');
+    Route::post('/mfa/backup_codes/verify', 'Auth\MfaBackupCodesController@verify');
+});
+Route::delete('/mfa/remove/{method}', 'Auth\MfaController@remove')->middleware('auth');
 
 // Social auth routes
 Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login');
 Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@callback');
-Route::group(['middleware' => 'auth'], function () {
-    Route::post('/login/service/{socialDriver}/detach', 'Auth\SocialController@detach');
-});
+Route::post('/login/service/{socialDriver}/detach', 'Auth\SocialController@detach')->middleware('auth');
 Route::get('/register/service/{socialDriver}', 'Auth\SocialController@register');
 
 // Login/Logout routes
index a8ca815fbeb0a52ebcf20ba7dd0f59e07c49713e..63a5fa4ddcdaace437216d12b976d0a5bad75db6 100644 (file)
@@ -18,21 +18,21 @@ class MfaConfigurationTest extends TestCase
 
         // Setup page state
         $resp = $this->actingAs($editor)->get('/mfa/setup');
-        $resp->assertElementContains('a[href$="/mfa/totp-generate"]', 'Setup');
+        $resp->assertElementContains('a[href$="/mfa/totp/generate"]', 'Setup');
 
         // Generate page access
-        $resp = $this->get('/mfa/totp-generate');
+        $resp = $this->get('/mfa/totp/generate');
         $resp->assertSee('Mobile App Setup');
         $resp->assertSee('Verify Setup');
-        $resp->assertElementExists('form[action$="/mfa/totp-confirm"] button');
+        $resp->assertElementExists('form[action$="/mfa/totp/confirm"] button');
         $this->assertSessionHas('mfa-setup-totp-secret');
         $svg = $resp->getElementHtml('#main-content .card svg');
 
         // Validation error, code should remain the same
-        $resp = $this->post('/mfa/totp-confirm', [
+        $resp = $this->post('/mfa/totp/confirm', [
             'code' => 'abc123',
         ]);
-        $resp->assertRedirect('/mfa/totp-generate');
+        $resp->assertRedirect('/mfa/totp/generate');
         $resp = $this->followRedirects($resp);
         $resp->assertSee('The provided code is not valid or has expired.');
         $revisitSvg = $resp->getElementHtml('#main-content .card svg');
@@ -42,7 +42,7 @@ class MfaConfigurationTest extends TestCase
         $google2fa = new Google2FA();
         $secret = decrypt(session()->get('mfa-setup-totp-secret'));
         $otp = $google2fa->getCurrentOtp($secret);
-        $resp = $this->post('/mfa/totp-confirm', [
+        $resp = $this->post('/mfa/totp/confirm', [
             'code' => $otp,
         ]);
         $resp->assertRedirect('/mfa/setup');
@@ -50,7 +50,7 @@ class MfaConfigurationTest extends TestCase
         // Confirmation of setup
         $resp = $this->followRedirects($resp);
         $resp->assertSee('Multi-factor method successfully configured');
-        $resp->assertElementContains('a[href$="/mfa/totp-generate"]', 'Reconfigure');
+        $resp->assertElementContains('a[href$="/mfa/totp/generate"]', 'Reconfigure');
 
         $this->assertDatabaseHas('mfa_values', [
             'user_id' => $editor->id,
@@ -69,12 +69,12 @@ class MfaConfigurationTest extends TestCase
 
         // Setup page state
         $resp = $this->actingAs($editor)->get('/mfa/setup');
-        $resp->assertElementContains('a[href$="/mfa/backup-codes-generate"]', 'Setup');
+        $resp->assertElementContains('a[href$="/mfa/backup_codes/generate"]', 'Setup');
 
         // Generate page access
-        $resp = $this->get('/mfa/backup-codes-generate');
+        $resp = $this->get('/mfa/backup_codes/generate');
         $resp->assertSee('Backup Codes');
-        $resp->assertElementContains('form[action$="/mfa/backup-codes-confirm"]', 'Confirm and Enable');
+        $resp->assertElementContains('form[action$="/mfa/backup_codes/confirm"]', 'Confirm and Enable');
         $this->assertSessionHas('mfa-setup-backup-codes');
         $codes = decrypt(session()->get('mfa-setup-backup-codes'));
         // Check code format
@@ -84,13 +84,13 @@ class MfaConfigurationTest extends TestCase
         $resp->assertSee(base64_encode(implode("\n\n", $codes)));
 
         // Confirm submit
-        $resp = $this->post('/mfa/backup-codes-confirm');
+        $resp = $this->post('/mfa/backup_codes/confirm');
         $resp->assertRedirect('/mfa/setup');
 
         // Confirmation of setup
         $resp = $this->followRedirects($resp);
         $resp->assertSee('Multi-factor method successfully configured');
-        $resp->assertElementContains('a[href$="/mfa/backup-codes-generate"]', 'Reconfigure');
+        $resp->assertElementContains('a[href$="/mfa/backup_codes/generate"]', 'Reconfigure');
 
         $this->assertDatabaseHas('mfa_values', [
             'user_id' => $editor->id,
@@ -104,7 +104,7 @@ class MfaConfigurationTest extends TestCase
 
     public function test_backup_codes_cannot_be_confirmed_if_not_previously_generated()
     {
-        $resp = $this->asEditor()->post('/mfa/backup-codes-confirm');
+        $resp = $this->asEditor()->post('/mfa/backup_codes/confirm');
         $resp->assertStatus(500);
     }
 
index 3f272cffbcb043360429041689651518df9e9c2c..d007fa49017b108c3f6ed177595b674e9f607bff 100644 (file)
@@ -2,9 +2,12 @@
 
 namespace Tests\Auth;
 
+use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\Access\Mfa\MfaValue;
 use BookStack\Auth\Access\Mfa\TotpService;
+use BookStack\Auth\Role;
 use BookStack\Auth\User;
+use BookStack\Exceptions\StoppedAuthenticationException;
 use Illuminate\Support\Facades\Hash;
 use PragmaRX\Google2FA\Google2FA;
 use Tests\TestCase;
@@ -20,10 +23,10 @@ class MfaVerificationTest extends TestCase
         $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"]');
+        $resp->assertElementExists('form[action$="/mfa/totp/verify"] input[name="code"]');
 
         $google2fa = new Google2FA();
-        $resp = $this->post('/mfa/verify/totp', [
+        $resp = $this->post('/mfa/totp/verify', [
             'code' => $google2fa->getCurrentOtp($secret),
         ]);
         $resp->assertRedirect('/');
@@ -35,7 +38,7 @@ class MfaVerificationTest extends TestCase
         [$user, $secret, $loginResp] = $this->startTotpLogin();
 
         $resp = $this->get('/mfa/verify');
-        $resp = $this->post('/mfa/verify/totp', [
+        $resp = $this->post('/mfa/totp/verify', [
             'code' => '',
         ]);
         $resp->assertRedirect('/mfa/verify');
@@ -44,7 +47,7 @@ class MfaVerificationTest extends TestCase
         $resp->assertSeeText('The code field is required.');
         $this->assertNull(auth()->user());
 
-        $resp = $this->post('/mfa/verify/totp', [
+        $resp = $this->post('/mfa/totp/verify', [
             'code' => '123321',
         ]);
         $resp->assertRedirect('/mfa/verify');
@@ -63,9 +66,9 @@ class MfaVerificationTest extends TestCase
         $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->assertElementExists('form[action$="/mfa/backup_codes/verify"] input[name="code"]');
 
-        $resp = $this->post('/mfa/verify/backup_codes', [
+        $resp = $this->post('/mfa/backup_codes/verify', [
             'code' => $codes[1],
         ]);
 
@@ -82,7 +85,7 @@ class MfaVerificationTest extends TestCase
         [$user, $codes, $loginResp] = $this->startBackupCodeLogin();
 
         $resp = $this->get('/mfa/verify');
-        $resp = $this->post('/mfa/verify/backup_codes', [
+        $resp = $this->post('/mfa/backup_codes/verify', [
             'code' => '',
         ]);
         $resp->assertRedirect('/mfa/verify');
@@ -91,7 +94,7 @@ class MfaVerificationTest extends TestCase
         $resp->assertSeeText('The code field is required.');
         $this->assertNull(auth()->user());
 
-        $resp = $this->post('/mfa/verify/backup_codes', [
+        $resp = $this->post('/mfa/backup_codes/verify', [
             'code' => 'ab123-ab456',
         ]);
         $resp->assertRedirect('/mfa/verify');
@@ -105,7 +108,7 @@ class MfaVerificationTest extends TestCase
     {
         [$user, $codes, $loginResp] = $this->startBackupCodeLogin();
 
-        $this->post('/mfa/verify/backup_codes', [
+        $this->post('/mfa/backup_codes/verify', [
             'code' => $codes[0],
         ]);
         $this->assertNotNull(auth()->user());
@@ -114,7 +117,7 @@ class MfaVerificationTest extends TestCase
 
         $this->post('/login', ['email' => $user->email, 'password' => 'password']);
         $this->get('/mfa/verify');
-        $resp = $this->post('/mfa/verify/backup_codes', [
+        $resp = $this->post('/mfa/backup_codes/verify', [
             'code' => $codes[0],
         ]);
         $resp->assertRedirect('/mfa/verify');
@@ -128,7 +131,7 @@ class MfaVerificationTest extends TestCase
     {
         [$user, $codes, $loginResp] = $this->startBackupCodeLogin(['abc12-def45', 'abc12-def46']);
 
-        $resp = $this->post('/mfa/verify/backup_codes', [
+        $resp = $this->post('/mfa/backup_codes/verify', [
             'code' => $codes[0],
         ]);
         $resp = $this->followRedirects($resp);
@@ -151,16 +154,88 @@ class MfaVerificationTest extends TestCase
         ]);
 
         // Totp shown by default
-        $mfaView->assertElementExists('form[action$="/mfa/verify/totp"] input[name="code"]');
+        $mfaView->assertElementExists('form[action$="/mfa/totp/verify"] 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->assertElementExists('form[action$="/mfa/backup_codes/verify"] input[name="code"]');
         $resp->assertElementContains('a[href$="/mfa/verify?method=totp"]', 'Verify using a mobile app');
     }
 
-    //    TODO !! - Test no-existing MFA
+    public function test_mfa_required_with_no_methods_leads_to_setup()
+    {
+        $user = $this->getEditor();
+        $user->password = Hash::make('password');
+        $user->save();
+        /** @var Role $role */
+        $role = $user->roles->first();
+        $role->mfa_enforced = true;
+        $role->save();
+
+        $this->assertDatabaseMissing('mfa_values', [
+            'user_id' => $user->id,
+        ]);
+
+        /** @var TestResponse $resp */
+        $resp = $this->followingRedirects()->post('/login', [
+            'email' => $user->email,
+            'password' => 'password',
+        ]);
+
+        $resp->assertSeeText('No Methods Configured');
+        $resp->assertElementContains('a[href$="/mfa/setup"]', 'Configure');
+
+        $this->get('/mfa/backup_codes/generate');
+        $this->followingRedirects()->post('/mfa/backup_codes/confirm');
+        $this->assertDatabaseHas('mfa_values', [
+            'user_id' => $user->id,
+        ]);
+
+        $resp = $this->followingRedirects()->post('/login', [
+            'email' => $user->email,
+            'password' => 'password',
+        ]);
+        $resp->assertSeeText('Enter one of your remaining backup codes below:');
+    }
+
+    public function test_mfa_setup_route_access()
+    {
+        $routes = [
+            ['get', '/mfa/setup'],
+            ['get', '/mfa/totp/generate'],
+            ['post', '/mfa/totp/confirm'],
+            ['get', '/mfa/backup_codes/generate'],
+            ['post', '/mfa/backup_codes/confirm'],
+        ];
+
+        // Non-auth access
+        foreach ($routes as [$method, $path]) {
+            $resp = $this->call($method, $path);
+            $resp->assertRedirect('/login');
+        }
+
+        // Attempted login user, who has configured mfa, access
+        // Sets up user that has MFA required after attempted login.
+        $loginService = $this->app->make(LoginService::class);
+        $user = $this->getEditor();
+        /** @var Role $role */
+        $role = $user->roles->first();
+        $role->mfa_enforced = true;
+        $role->save();
+        try {
+            $loginService->login($user, 'testing');
+        } catch (StoppedAuthenticationException $e) {
+        }
+        $this->assertNotNull($loginService->getLastLoginAttemptUser());
+
+        MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, '[]');
+        foreach ($routes as [$method, $path]) {
+            $resp = $this->call($method, $path);
+            $resp->assertRedirect('/login');
+        }
+
+    }
 
     /**
      * @return Array<User, string, TestResponse>