Added login redirect system to confirm/mfa

Also continued a bit on the MFA verification system. Moved some MFA routes to public space using updated login service to get the current user that is either logged in or last attempted login (With correct creds).
2024-10-01 05:36:00 +00:00 · 2021-07-18 16:52:31 +01:00 · 2021-07-18 16:52:31 +01:00 · 1af5bbf3f7
commit 1af5bbf3f7
parent 1278fb4969
11 changed files with 186 additions and 37 deletions
--- a/app/Auth/Access/LoginService.php
+++ b/app/Auth/Access/LoginService.php
@ -5,13 +5,17 @@ namespace BookStack\Auth\Access;
 use BookStack\Actions\ActivityType;
 use BookStack\Auth\Access\Mfa\MfaSession;
 use BookStack\Auth\User;
+use BookStack\Exceptions\StoppedAuthenticationException;
 use BookStack\Facades\Activity;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
+use Exception;

 class LoginService
 {

+    protected const LAST_LOGIN_ATTEMPTED_SESSION_KEY = 'auth-login-last-attempted';
+
    protected $mfaSession;

    public function __construct(MfaSession $mfaSession)
@ -19,24 +23,18 @@ class LoginService
        $this->mfaSession = $mfaSession;
    }

-
    /**
     * Log the given user into the system.
     * Will start a login of the given user but will prevent if there's
     * a reason to (MFA or Unconfirmed Email).
     * Returns a boolean to indicate the current login result.
+     * @throws StoppedAuthenticationException
     */
-    public function login(User $user, string $method): bool
+    public function login(User $user, string $method): void
    {
        if ($this->awaitingEmailConfirmation($user) || $this->needsMfaVerification($user)) {
-            // TODO - Remember who last attempted a login so we can use them after such
-            //  a email confirmation or mfa verification step.
-            //  Create a method to fetch that attempted user for use by the email confirmation
-            //  or MFA verification services.
-            //  Also will need a method to 'reattemptLastAttempted' login for once
-            //  the email confirmation of MFA verification steps have passed.
-            //  Must ensure this remembered last attempted login is cleared upon successful login.
-
+            $this->setLastLoginAttemptedForUser($user);
+            throw new StoppedAuthenticationException($user, $this);
            // TODO - Does 'remember' still work? Probably not right now.

            // Old MFA middleware todos:
@ -44,15 +42,15 @@ class LoginService
            // 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?
+            //    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.
-
-            return false;
        }

+        $this->clearLastLoginAttempted();
        auth()->login($user);
        Activity::add(ActivityType::AUTH_LOGIN, "{$method}; {$user->logDescriptor()}");
        Theme::dispatch(ThemeEvents::AUTH_LOGIN, $method, $user);
@ -64,14 +62,58 @@ class LoginService
                auth($guard)->login($user);
            }
        }
+    }

-        return true;
+    /**
+     * Reattempt a system login after a previous stopped attempt.
+     * @throws Exception
+     */
+    public function reattemptLoginFor(User $user, string $method)
+    {
+        if ($user->id !== $this->getLastLoginAttemptUser()) {
+            throw new Exception('Login reattempt user does align with current session state');
+        }
+
+        $this->login($user, $method);
+    }
+
+    /**
+     * Get the last user that was attempted to be logged in.
+     * Only exists if the last login attempt had correct credentials
+     * but had been prevented by a secondary factor.
+     */
+    public function getLastLoginAttemptUser(): ?User
+    {
+        $id = session()->get(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY);
+        if (!$id) {
+            return null;
+        }
+
+        return User::query()->where('id', '=', $id)->first();
+    }
+
+    /**
+     * Set the last login attempted user.
+     * 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)
+    {
+        session()->put(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY, $user->id);
+    }
+
+    /**
+     * Clear the last login attempted session value.
+     */
+    protected function clearLastLoginAttempted(): void
+    {
+        session()->remove(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY);
    }

    /**
     * Check if MFA verification is needed.
     */
-    protected function needsMfaVerification(User $user): bool
+    public function needsMfaVerification(User $user): bool
    {
        return !$this->mfaSession->isVerifiedForUser($user) && $this->mfaSession->isRequiredForUser($user);
    }
@ -79,7 +121,7 @@ class LoginService
    /**
     * Check if the given user is awaiting email confirmation.
     */
-    protected function awaitingEmailConfirmation(User $user): bool
+    public function awaitingEmailConfirmation(User $user): bool
    {
        $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict'));
        return $requireConfirmation && !$user->email_confirmed;
@ -89,6 +131,9 @@ class LoginService
     * Attempt the login of a user using the given credentials.
     * Meant to mirror Laravel's default guard 'attempt' method
     * but in a manner that always routes through our login system.
+     * May interrupt the flow if extra authentication requirements are imposed.
+     *
+     * @throws StoppedAuthenticationException
     */
    public function attempt(array $credentials, string $method, bool $remember = false): bool
    {
@ -96,7 +141,7 @@ class LoginService
        if ($result) {
            $user = auth()->user();
            auth()->logout();
-            $result = $this->login($user, $method);
+            $this->login($user, $method);
        }

        return $result;
--- a/app/Exceptions/StoppedAuthenticationException.php
+++ b/app/Exceptions/StoppedAuthenticationException.php
@ -0,0 +1,40 @@
+<?php
+
+namespace BookStack\Exceptions;
+
+use BookStack\Auth\Access\LoginService;
+use BookStack\Auth\User;
+use Illuminate\Contracts\Support\Responsable;
+
+class StoppedAuthenticationException extends \Exception implements Responsable
+{
+
+    protected $user;
+    protected $loginService;
+
+    /**
+     * StoppedAuthenticationException constructor.
+     */
+    public function __construct(User $user, LoginService $loginService)
+    {
+        $this->user = $user;
+        $this->loginService = $loginService;
+        parent::__construct();
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function toResponse($request)
+    {
+        $redirect = '/login';
+
+        if ($this->loginService->awaitingEmailConfirmation($this->user)) {
+            $redirect = '/register/confirm/awaiting';
+        } else if  ($this->loginService->needsMfaVerification($this->user)) {
+            $redirect = '/mfa/verify';
+        }
+
+        return redirect($redirect);
+    }
+}
--- a/app/Http/Controllers/Auth/ConfirmEmailController.php
+++ b/app/Http/Controllers/Auth/ConfirmEmailController.php
@ -47,12 +47,11 @@ class ConfirmEmailController extends Controller
    /**
     * Shows a notice that a user's email address has not been confirmed,
     * Also has the option to re-send the confirmation email.
-     *
-     * @return View
     */
    public function showAwaiting()
    {
-        return view('auth.user-unconfirmed');
+        $user = $this->loginService->getLastLoginAttemptUser();
+        return view('auth.user-unconfirmed', ['user' => $user]);
    }

    /**
--- a/app/Http/Controllers/Auth/HandlesPartialLogins.php
+++ b/app/Http/Controllers/Auth/HandlesPartialLogins.php
@ -0,0 +1,22 @@
+<?php
+
+namespace BookStack\Http\Controllers\Auth;
+
+use BookStack\Auth\Access\LoginService;
+use BookStack\Auth\User;
+use BookStack\Exceptions\NotFoundException;
+
+trait HandlesPartialLogins
+{
+    protected function currentOrLastAttemptedUser(): User
+    {
+        $loginService = app()->make(LoginService::class);
+        $user = auth()->user() ?? $loginService->getLastLoginAttemptUser();
+
+        if (!$user) {
+            throw new NotFoundException('A user for this action could not be found');
+        }
+
+        return $user;
+    }
+}
--- a/app/Http/Controllers/Auth/MfaBackupCodesController.php
+++ b/app/Http/Controllers/Auth/MfaBackupCodesController.php
@ -10,6 +10,8 @@ use Exception;

 class MfaBackupCodesController extends Controller
 {
+    use HandlesPartialLogins;
+
    protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-backup-codes';

    /**
@ -39,7 +41,7 @@ class MfaBackupCodesController extends Controller
        }

        $codes = decrypt(session()->pull(self::SETUP_SECRET_SESSION_KEY));
-        MfaValue::upsertWithValue(user(), MfaValue::METHOD_BACKUP_CODES, json_encode($codes));
+        MfaValue::upsertWithValue($this->currentOrLastAttemptedUser(), MfaValue::METHOD_BACKUP_CODES, json_encode($codes));

        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'backup-codes');
        return redirect('/mfa/setup');
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@ -5,15 +5,19 @@ namespace BookStack\Http\Controllers\Auth;
 use BookStack\Actions\ActivityType;
 use BookStack\Auth\Access\Mfa\MfaValue;
 use BookStack\Http\Controllers\Controller;
+use BookStack\Http\Request;

 class MfaController extends Controller
 {
+    use HandlesPartialLogins;
+
    /**
     * Show the view to setup MFA for the current user.
     */
    public function setup()
    {
-        $userMethods = user()->mfaValues()
+        $userMethods = $this->currentOrLastAttemptedUser()
+            ->mfaValues()
            ->get(['id', 'method'])
            ->groupBy('method');
        return view('mfa.setup', [
@ -41,14 +45,26 @@ class MfaController extends Controller
    /**
     * Show the page to start an MFA verification.
     */
-    public function verify()
+    public function verify(Request $request)
    {
-        $userMethods = user()->mfaValues()
+        // TODO - Test this
+        $desiredMethod = $request->get('method');
+        $userMethods = $this->currentOrLastAttemptedUser()
+            ->mfaValues()
            ->get(['id', 'method'])
            ->groupBy('method');

+        // Basic search for the default option for a user.
+        // (Prioritises totp over backup codes)
+        $method = $userMethods->has($desiredMethod) ? $desiredMethod : $userMethods->keys()->sort()->reverse()->first();
+        $otherMethods = $userMethods->keys()->filter(function($userMethod) use ($method) {
+            return $method !== $userMethod;
+        })->all();
+
        return view('mfa.verify', [
            'userMethods' => $userMethods,
+            'method' => $method,
+            'otherMethods' => $otherMethods,
        ]);
    }
 }
--- a/app/Http/Controllers/Auth/MfaTotpController.php
+++ b/app/Http/Controllers/Auth/MfaTotpController.php
@ -6,12 +6,15 @@ use BookStack\Actions\ActivityType;
 use BookStack\Auth\Access\Mfa\MfaValue;
 use BookStack\Auth\Access\Mfa\TotpService;
 use BookStack\Auth\Access\Mfa\TotpValidationRule;
+use BookStack\Exceptions\NotFoundException;
 use BookStack\Http\Controllers\Controller;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;

 class MfaTotpController extends Controller
 {
+    use HandlesPartialLogins;
+
    protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-totp-secret';

    /**
@ -39,6 +42,7 @@ class MfaTotpController extends Controller
     * Confirm the setup of TOTP and save the auth method secret
     * against the current user.
     * @throws ValidationException
+     * @throws NotFoundException
     */
    public function confirm(Request $request)
    {
@ -51,7 +55,7 @@ class MfaTotpController extends Controller
            ]
        ]);

-        MfaValue::upsertWithValue(user(), MfaValue::METHOD_TOTP, $totpSecret);
+        MfaValue::upsertWithValue($this->currentOrLastAttemptedUser(), MfaValue::METHOD_TOTP, $totpSecret);
        session()->remove(static::SETUP_SECRET_SESSION_KEY);
        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'totp');

--- a/app/Http/Controllers/Auth/SocialController.php
+++ b/app/Http/Controllers/Auth/SocialController.php
@ -140,9 +140,9 @@ class SocialController extends Controller
        }

        $user = $this->registrationService->registerUser($userData, $socialAccount, $emailVerified);
+        $this->showSuccessNotification(trans('auth.register_success'));
        $this->loginService->login($user, $socialDriver);

-        $this->showSuccessNotification(trans('auth.register_success'));
        return redirect('/');
    }
 }
--- a/resources/views/auth/user-unconfirmed.blade.php
+++ b/resources/views/auth/user-unconfirmed.blade.php
@ -17,8 +17,8 @@
                {!! csrf_field() !!}
                <div class="form-group">
                    <label for="email">{{ trans('auth.email') }}</label>
-                    @if(auth()->check())
-                        @include('form.text', ['name' => 'email', 'model' => auth()->user()])
+                    @if($user)
+                        @include('form.text', ['name' => 'email', 'model' => $user])
                    @else
                        @include('form.text', ['name' => 'email'])
                    @endif
--- a/resources/views/mfa/verify.blade.php
+++ b/resources/views/mfa/verify.blade.php
@ -1,16 +1,28 @@
@extends('simple-layout')

@section('body')
-    <div class="container small py-xl">
+    <div class="container very-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.
+                Verify using one of your configured methods to continue.
            </p>

+            @if(!$method)
+                <hr class="my-l">
+                <h5>No Methods Configured</h5>
+                <p class="small">
+                    No multi-factor authentication methods could be found for your account.
+                    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>
+                </div>
+            @endif
+
            <div class="setting-list">
                <div class="grid half gap-xl">
                    <div>
@ -26,6 +38,13 @@

            </div>

+            @if(count($otherMethods) > 0)
+                <hr class="my-l">
+                @foreach($otherMethods as $otherMethod)
+                    <a href="{{ url("/mfa/verify?method={$otherMethod}") }}">Use {{$otherMethod}}</a>
+                @endforeach
+            @endif
+
        </div>
    </div>
@stop
--- a/routes/web.php
+++ b/routes/web.php
@ -224,16 +224,18 @@ Route::group(['middleware' => 'auth'], function () {
        Route::put('/roles/{id}', 'RoleController@update');
    });

-    // 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');
+    // MFA (Auth Mandatory)
    Route::delete('/mfa/remove/{method}', 'Auth\MfaController@remove');
-    Route::get('/mfa/verify', 'Auth\MfaController@verify');
 });

+// 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');
+
 // Social auth routes
 Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login');
 Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@callback');