From 659c33f0e8b909c64f6778af614efd5a79ca224f Mon Sep 17 00:00:00 2001
From: Dane Everitt <dane@daneeveritt.com>
Date: Mon, 7 Nov 2016 15:55:57 -0500
Subject: [PATCH] Fixes a bug that allows a user to bypass 2FA authentication
 requirements

This bug was reported to us by a user (@Ferry#1704) on Discord on
Monday, November 7th, 2016.

It was disclosed that it was possible to bypass the 2FA checkpoint by
clicking outside of the modal which would prompt the modal to close,
but not submit the form. The user could then press the login button
which would trigger an error. Due to this error being triggered the
authentication attempt was not cancelled. On the next page load the
application recognized the user as logged in and continued on to the
panel.

At no time was it possible to login without using the correct email
address and password.

As a result of this bug we have re-factored the Authentication code for
logins to address the persistent session. Previously accounts were
manually logged back out on 2FA failure. However, as this bug
demonstrated, causing a fatal error in the code would prevent the
logout code from firing, thus preserving their session state.

This commit modifies the code to use a non-persistent login to handle
2FA checking. In order for the session to be saved the application must
complete all portions of the login without any errors, at which point
the user is persistently authenticated using Auth::login().

This resolves the ability to cause an exception and bypass 2FA
verification.
---
 app/Http/Controllers/Auth/LoginController.php | 14 ++++++--------
 resources/views/auth/login.blade.php          |  5 ++++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index 197e69592..2a6903144 100644
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -103,7 +103,7 @@ class LoginController extends Controller
         }
 
         // Is the email & password valid?
-        if (!Auth::attempt([
+        if (!Auth::once([
             'email' => $request->input('email'),
             'password' => $request->input('password')
         ], $request->has('remember'))) {
@@ -116,14 +116,10 @@ class LoginController extends Controller
 
         }
 
-        $G2FA = new Google2FA();
-        $user = User::select('use_totp', 'totp_secret')->where('email', $request->input('email'))->first();
-
         // Verify TOTP Token was Valid
-        if($user->use_totp === 1) {
-            if(!$G2FA->verifyKey($user->totp_secret, $request->input('totp_token'))) {
-
-                Auth::logout();
+        if(Auth::user()->use_totp === 1) {
+            $G2FA = new Google2FA();
+            if(is_null($request->input('totp_token')) || !$G2FA->verifyKey(Auth::user()->totp_secret, $request->input('totp_token'))) {
 
                 if (!$lockedOut) {
                     $this->incrementLoginAttempts($request);
@@ -135,6 +131,8 @@ class LoginController extends Controller
             }
         }
 
+        // Successfully Authenticated.
+        Auth::login(Auth::user(), $request->has('remember'));
         return $this->sendLoginResponse($request);
 
     }
diff --git a/resources/views/auth/login.blade.php b/resources/views/auth/login.blade.php
index 2000dad22..9ab5cdbf0 100644
--- a/resources/views/auth/login.blade.php
+++ b/resources/views/auth/login.blade.php
@@ -125,7 +125,10 @@ $(document).ready(function() {
             }
         }).done(function (data) {
             if (typeof data.id !== 'undefined') {
-                $('#openTOTP').modal('show');
+                $('#openTOTP').modal({
+                    backdrop: 'static',
+                    keyboard: false
+                });
                 $('#openTOTP').on('shown.bs.modal', function() {
                     $('#totp_token').focus();
                 });