Started on some MFA access-time checks

Discovered some difficult edge cases:
- User image loading in header bar when using local_secure storage
- 404s showing user-specific visible content due to content listing on
  404 page since user is in semi-logged in state. Maybe need to go
  through and change up how logins are handled to centralise and
  provide us better control at login time to prevent any auth level.

diff --git a/app/Auth/Access/Mfa/MfaSession.php b/app/Auth/Access/Mfa/MfaSession.php
new file mode 100644 (file)
index 0000000..67574cb
--- /dev/null
+++ b/app/Auth/Access/Mfa/MfaSession.php
@@ -0,0 +1,44 @@
+<?php
+
+namespace BookStack\Auth\Access\Mfa;
+
+class MfaSession
+{
+    private const MFA_VERIFIED_SESSION_KEY = 'mfa-verification-passed';
+
+    /**
+     * Check if MFA is required for the current user.
+     */
+    public function requiredForCurrentUser(): bool
+    {
+        // TODO - Test both these cases
+        return user()->mfaValues()->exists() || $this->currentUserRoleEnforcesMfa();
+    }
+
+    /**
+     * Check if a role of the current user enforces MFA.
+     */
+    protected function currentUserRoleEnforcesMfa(): bool
+    {
+        return user()->roles()
+            ->where('mfa_enforced', '=', true)
+            ->exists();
+    }
+
+    /**
+     * Check if the current MFA session has already been verified.
+     */
+    public function isVerified(): bool
+    {
+        return session()->get(self::MFA_VERIFIED_SESSION_KEY) === 'true';
+    }
+
+    /**
+     * Mark the current session as MFA-verified.
+     */
+    public function markVerified(): void
+    {
+        session()->put(self::MFA_VERIFIED_SESSION_KEY, 'true');
+    }
+
+}
\ No newline at end of file

diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php
index 9feda9433aefda07d09d6203c31371da88f06b7a..39a4e852f774dafcbbd2425620de155a9e616eda 100644 (file)
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@@ -37,4 +37,18 @@ class MfaController extends Controller
 
         return redirect('/mfa/setup');
     }
+
+    /**
+     * Show the page to start an MFA verification.
+     */
+    public function verify()
+    {
+        $userMethods = user()->mfaValues()
+            ->get(['id', 'method'])
+            ->groupBy('method');
+
+        return view('mfa.verify', [
+            'userMethods' => $userMethods,
+        ]);
+    }
 }

diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index 4f9bfc1e64050dcd7a244c12b453db9ccc04461d..c9e59ed3e30863b5bb5eb3826c6f7f45d92fd56d 100644 (file)
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -48,6 +48,7 @@ class Kernel extends HttpKernel
      */
     protected $routeMiddleware = [
         'auth'       => \BookStack\Http\Middleware\Authenticate::class,
+        'mfa'        => \BookStack\Http\Middleware\EnforceMfaRequirements::class,
         'can'        => \Illuminate\Auth\Middleware\Authorize::class,
         'guest'      => \BookStack\Http\Middleware\RedirectIfAuthenticated::class,
         'throttle'   => \Illuminate\Routing\Middleware\ThrottleRequests::class,

diff --git a/app/Http/Middleware/EnforceMfaRequirements.php b/app/Http/Middleware/EnforceMfaRequirements.php
index 957b42ae15a8e174ea0707c5a67b38b44e51a864..ac3c9609bceefba2b709f875f6d20fd2df10af17 100644 (file)
--- a/app/Http/Middleware/EnforceMfaRequirements.php
+++ b/app/Http/Middleware/EnforceMfaRequirements.php
@@ -2,10 +2,21 @@
 
 namespace BookStack\Http\Middleware;
 
+use BookStack\Auth\Access\Mfa\MfaSession;
 use Closure;
 
 class EnforceMfaRequirements
 {
+    protected $mfaSession;
+
+    /**
+     * EnforceMfaRequirements constructor.
+     */
+    public function __construct(MfaSession $mfaSession)
+    {
+        $this->mfaSession = $mfaSession;
+    }
+
     /**
      * Handle an incoming request.
      *
@@ -15,10 +26,23 @@ class EnforceMfaRequirements
      */
     public function handle($request, Closure $next)
     {
-        $mfaRequired = user()->roles()->where('mfa_enforced', '=', true)->exists();
-        // TODO - Run this after auth (If authenticated)
-        // TODO - Redirect user to setup MFA or verify via MFA.
+        if (
+            !$this->mfaSession->isVerified()
+            && !$request->is('mfa/verify*', 'uploads/images/user/*')
+            && $this->mfaSession->requiredForCurrentUser()
+        ) {
+            return redirect('/mfa/verify');
+        }
+
+        // TODO - URI wildcard exceptions above allow access to the 404 page of this user
+        //  which could then expose content. Either need to lock that down (Tricky to do image thing)
+        //  or prevent any level of auth until verified.
+
+        // 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)
         // TODO - Store mfa_pass into session for future requests?
+
         return $next($request);
     }
 }

diff --git a/resources/views/mfa/verify.blade.php b/resources/views/mfa/verify.blade.php
new file mode 100644 (file)
index 0000000..4ff0e6c
--- /dev/null
+++ b/resources/views/mfa/verify.blade.php
@@ -0,0 +1,31 @@
+@extends('simple-layout')
+
+@section('body')
+    <div class="container small py-xl">
+
+        <div class="card content-wrap auto-height">
+            <h1 class="list-heading">Verify Access</h1>
+            <p class="mb-none">
+                Your user account requires you to confirm your identity via an additional level
+                of verification before you're granted access.
+                Verify using one of your configure methods to continue.
+            </p>
+
+            <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>
+
+        </div>
+    </div>
+@stop

diff --git a/routes/web.php b/routes/web.php
index e3329edc48ca2d3b37eb03f03f14e141f2053daa..406cfd767e23dabc02f35f713bc73884b0de6026 100644 (file)
--- a/routes/web.php
+++ b/routes/web.php
@@ -4,7 +4,7 @@ Route::get('/status', 'StatusController@show');
 Route::get('/robots.txt', 'HomeController@getRobots');
 
 // Authenticated routes...
-Route::group(['middleware' => 'auth'], function () {
+Route::group(['middleware' => ['auth', 'mfa']], function () {
 
     // Secure images routing
     Route::get('/uploads/images/{path}', 'Images\ImageController@showImage')
@@ -224,13 +224,14 @@ Route::group(['middleware' => 'auth'], function () {
         Route::put('/roles/{id}', 'RoleController@update');
     });
 
-    // MFA Setup Routes
+    // MFA Routes
     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::delete('/mfa/remove/{method}', 'Auth\MfaController@remove');
+    Route::get('/mfa/verify', 'Auth\MfaController@verify');
 });
 
 // Social auth routes