From c3e462ab2f508952bbf7ca6f18ef02bae84e5063 Mon Sep 17 00:00:00 2001
From: Dane Everitt <dane@daneeveritt.com>
Date: Sat, 7 Apr 2018 16:17:51 -0500
Subject: [PATCH] Cleanup login/reset functionality, address security issue
 with 2FA pathways

---
 .../Auth/AbstractLoginController.php          |  4 ++
 .../Auth/ForgotPasswordController.php         | 10 ++--
 .../Auth/LoginCheckpointController.php        |  9 ++-
 app/Http/Controllers/Auth/LoginController.php | 16 ++++--
 .../components/auth/ForgotPassword.vue        | 55 ++++++++++++++++---
 .../scripts/components/auth/LoginForm.vue     | 26 +++++----
 .../pterodactyl/styles/components/buttons.css |  7 ++-
 .../styles/components/spinners.css            | 52 ++++++++++++++++++
 resources/assets/pterodactyl/styles/main.css  |  1 +
 resources/lang/en/auth.php                    |  3 +-
 routes/auth.php                               | 14 +++--
 11 files changed, 158 insertions(+), 39 deletions(-)
 create mode 100644 resources/assets/pterodactyl/styles/components/spinners.css

diff --git a/app/Http/Controllers/Auth/AbstractLoginController.php b/app/Http/Controllers/Auth/AbstractLoginController.php
index eac305315..f36a8c231 100644
--- a/app/Http/Controllers/Auth/AbstractLoginController.php
+++ b/app/Http/Controllers/Auth/AbstractLoginController.php
@@ -106,6 +106,10 @@ abstract class AbstractLoginController extends Controller
             $this->getField($request->input('user')) => $request->input('user'),
         ]);
 
+        if ($request->route()->named('auth.checkpoint')) {
+            throw new DisplayException(trans('auth.checkpoint_failed'));
+        }
+
         throw new DisplayException(trans('auth.failed'));
     }
 
diff --git a/app/Http/Controllers/Auth/ForgotPasswordController.php b/app/Http/Controllers/Auth/ForgotPasswordController.php
index f35be6d37..edec72307 100644
--- a/app/Http/Controllers/Auth/ForgotPasswordController.php
+++ b/app/Http/Controllers/Auth/ForgotPasswordController.php
@@ -3,7 +3,7 @@
 namespace Pterodactyl\Http\Controllers\Auth;
 
 use Illuminate\Http\Request;
-use Illuminate\Http\RedirectResponse;
+use Illuminate\Http\JsonResponse;
 use Illuminate\Support\Facades\Password;
 use Pterodactyl\Http\Controllers\Controller;
 use Pterodactyl\Events\Auth\FailedPasswordReset;
@@ -18,9 +18,9 @@ class ForgotPasswordController extends Controller
      *
      * @param  \Illuminate\Http\Request
      * @param string $response
-     * @return \Illuminate\Http\RedirectResponse
+     * @return \Illuminate\Http\JsonResponse
      */
-    protected function sendResetLinkFailedResponse(Request $request, $response): RedirectResponse
+    protected function sendResetLinkFailedResponse(Request $request, $response): JsonResponse
     {
         // As noted in #358 we will return success even if it failed
         // to avoid pointing out that an account does or does not
@@ -34,9 +34,9 @@ class ForgotPasswordController extends Controller
      * Get the response for a successful password reset link.
      *
      * @param string $response
-     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
+     * @return \Illuminate\Http\JsonResponse
      */
-    protected function sendResetLinkResponse($response)
+    protected function sendResetLinkResponse($response): JsonResponse
     {
         return response()->json([
             'status' => trans($response),
diff --git a/app/Http/Controllers/Auth/LoginCheckpointController.php b/app/Http/Controllers/Auth/LoginCheckpointController.php
index cacd0da1f..be6357516 100644
--- a/app/Http/Controllers/Auth/LoginCheckpointController.php
+++ b/app/Http/Controllers/Auth/LoginCheckpointController.php
@@ -10,9 +10,8 @@ class LoginCheckpointController extends AbstractLoginController
 {
     /**
      * Handle a login where the user is required to provide a TOTP authentication
-     * token. In order to add additional layers of security, users are not
-     * informed of an incorrect password until this stage, forcing them to
-     * provide a token on each login attempt.
+     * token. Once a user has reached this stage it is assumed that they have already
+     * provided a valid username and password.
      *
      * @param \Pterodactyl\Http\Requests\Auth\LoginCheckpointRequest $request
      * @return \Illuminate\Http\JsonResponse
@@ -28,7 +27,7 @@ class LoginCheckpointController extends AbstractLoginController
             return $this->sendFailedLoginResponse($request);
         }
 
-        if (! array_get($cache, 'valid_credentials') || array_get($cache, 'request_ip') !== $request->ip()) {
+        if (array_get($cache, 'request_ip') !== $request->ip()) {
             return $this->sendFailedLoginResponse($request, $user);
         }
 
@@ -40,7 +39,7 @@ class LoginCheckpointController extends AbstractLoginController
             return $this->sendFailedLoginResponse($request, $user);
         }
 
-        $this->authManager->guard()->login($user, true);
+        $this->auth->guard()->login($user, true);
 
         return $this->sendLoginResponse($request);
     }
diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index 22d6f4686..91c49e481 100644
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -33,22 +33,26 @@ class LoginController extends AbstractLoginController
             return $this->sendFailedLoginResponse($request);
         }
 
-        $validCredentials = password_verify($request->input('password'), $user->password);
+        // Ensure that the account is using a valid username and password before trying to
+        // continue. Previously this was handled in the 2FA checkpoint, however that has
+        // a flaw in which you can discover if an account exists simply by seeing if you
+        // can proceede to the next step in the login process.
+        if (! password_verify($request->input('password'), $user->password)) {
+            return $this->sendFailedLoginResponse($request, $user);
+        }
+
+        // If the user is using 2FA we do not actually log them in at this step, we return
+        // a one-time token to link the 2FA credentials to this account via the UI.
         if ($user->use_totp) {
             $token = str_random(128);
             $this->cache->put($token, [
                 'user_id' => $user->id,
-                'valid_credentials' => $validCredentials,
                 'request_ip' => $request->ip(),
             ], 5);
 
             return response()->json(['complete' => false, 'token' => $token]);
         }
 
-        if (! $validCredentials) {
-            return $this->sendFailedLoginResponse($request, $user);
-        }
-
         $this->auth->guard()->login($user, true);
 
         return response()->json(['complete' => true]);
diff --git a/resources/assets/pterodactyl/scripts/components/auth/ForgotPassword.vue b/resources/assets/pterodactyl/scripts/components/auth/ForgotPassword.vue
index a252dab94..71b058dd8 100644
--- a/resources/assets/pterodactyl/scripts/components/auth/ForgotPassword.vue
+++ b/resources/assets/pterodactyl/scripts/components/auth/ForgotPassword.vue
@@ -1,9 +1,19 @@
 <template>
     <div>
-        <form class="bg-white shadow-lg rounded-lg pt-10 px-8 pb-6 mb-4 animate fadein" method="post">
+        <div class="pb-4" v-for="error in errors">
+            <div class="p-2 bg-red-dark border-red-darker border items-center text-red-lightest leading-normal rounded flex lg:inline-flex w-full text-sm"
+                 role="alert">
+                <span class="flex rounded-full bg-red uppercase px-2 py-1 text-xs font-bold mr-3 leading-none">Error</span>
+                <span class="mr-2 text-left flex-auto">{{ error }}</span>
+            </div>
+        </div>
+        <form class="bg-white shadow-lg rounded-lg pt-10 px-8 pb-6 mb-4 animate fadein" method="post"
+              v-on:submit.prevent="submitForm"
+        >
             <div class="flex flex-wrap -mx-3 mb-6">
                 <div class="input-open">
                     <input class="input" id="grid-email" type="email" aria-labelledby="grid-email" ref="email" required
+                           v-bind:readonly="showSpinner"
                            v-bind:value="email"
                            v-on:input="updateEmail($event)"
                     />
@@ -12,9 +22,11 @@
                 </div>
             </div>
             <div>
-                <csrf/>
-                <button class="btn btn-blue btn-jumbo" type="submit">
-                    {{ $t('auth.recover_account') }}
+                <button class="btn btn-blue btn-jumbo" type="submit" v-bind:disabled="submitDisabled">
+                    <span class="spinner white" v-bind:class="{ hidden: ! showSpinner }">&nbsp;</span>
+                    <span v-bind:class="{ hidden: showSpinner }">
+                        {{ $t('auth.recover_account') }}
+                    </span>
                 </button>
             </div>
             <div class="pt-6 text-center">
@@ -27,10 +39,7 @@
 </template>
 
 <script>
-    import Csrf from "../forms/CSRF";
-
     export default {
-        components: {Csrf},
         name: 'forgot-password',
         props: {
             email: {type: String, required: true},
@@ -41,11 +50,43 @@
         data: function () {
             return {
                 X_CSRF_TOKEN: window.X_CSRF_TOKEN,
+                errors: [],
+                submitDisabled: false,
+                showSpinner: false,
             };
         },
         methods: {
             updateEmail: function (event) {
+                this.$data.submitDisabled = false;
                 this.$emit('update-email', event.target.value);
+            },
+
+            submitForm: function () {
+                const self = this;
+                this.$data.submitDisabled = true;
+                this.$data.showSpinner = true;
+                this.$data.errors = [];
+
+                window.axios.post(this.route('auth.forgot-password.send-link'), {
+                    email: this.$props.email,
+                })
+                    .then(function (response) {
+                        self.$data.submitDisabled = false;
+                        self.$data.showSpinner = false;
+                        self.flash({message: response.data.status, variant: 'success'});
+                        self.$router.push({name: 'login'});
+                    })
+                    .catch(function (err) {
+                        self.$data.showSpinner = false;
+                        if (!err.response) {
+                            return console.error(err);
+                        }
+
+                        const response = err.response;
+                        if (response.data && _.isObject(response.data.errors)) {
+                            self.$data.errors.push(response.data.errors[0].detail);
+                        }
+                    });
             }
         }
     }
diff --git a/resources/assets/pterodactyl/scripts/components/auth/LoginForm.vue b/resources/assets/pterodactyl/scripts/components/auth/LoginForm.vue
index d5201e6c1..523bc10cf 100644
--- a/resources/assets/pterodactyl/scripts/components/auth/LoginForm.vue
+++ b/resources/assets/pterodactyl/scripts/components/auth/LoginForm.vue
@@ -2,9 +2,7 @@
     <div>
         <flash-message variant="danger" />
         <flash-message variant="success" />
-        <flash-message variant="warning" />
-        <flash-message variant="info" />
-        <div class="py-4" v-if="errors && errors.length === 1">
+        <div class="pb-4" v-if="errors && errors.length === 1">
             <div class="p-2 bg-red-dark border-red-darker border items-center text-red-lightest leading-normal rounded flex lg:inline-flex w-full text-sm"
                  role="alert">
                 <span class="flex rounded-full bg-red uppercase px-2 py-1 text-xs font-bold mr-3 leading-none">Error</span>
@@ -12,13 +10,12 @@
             </div>
         </div>
         <form class="bg-white shadow-lg rounded-lg pt-10 px-8 pb-6 mb-4 animate fadein" method="post"
-              v-on:submit.prevent="handleLogin"
+              v-on:submit.prevent="submitForm"
         >
             <div class="flex flex-wrap -mx-3 mb-6">
                 <div class="input-open">
-                    <input class="input" id="grid-username" type="text" name="user" aria-labelledby="grid-username"
+                    <input class="input" id="grid-username" type="text" name="user" aria-labelledby="grid-username" required
                            ref="email"
-                           required
                            v-bind:value="user.email"
                            v-on:input="updateEmail($event)"
                     />
@@ -28,6 +25,7 @@
             <div class="flex flex-wrap -mx-3 mb-6">
                 <div class="input-open">
                     <input class="input" id="grid-password" type="password" name="password"
+                           ref="password"
                            aria-labelledby="grid-password" required
                            v-model="user.password"
                     />
@@ -35,8 +33,11 @@
                 </div>
             </div>
             <div>
-                <button class="btn btn-blue btn-jumbo" type="submit">
-                    {{ $t('auth.sign_in') }}
+                <button class="btn btn-blue btn-jumbo" type="submit" v-bind:disabled="showSpinner">
+                    <span class="spinner white" v-bind:class="{ hidden: ! showSpinner }">&nbsp;</span>
+                    <span v-bind:class="{ hidden: showSpinner }">
+                        {{ $t('auth.sign_in') }}
+                    </span>
                 </button>
             </div>
             <div class="pt-6 text-center">
@@ -67,6 +68,7 @@
         data: function () {
             return {
                 errors: [],
+                showSpinner: false,
             }
         },
         mounted: function () {
@@ -75,8 +77,9 @@
         methods: {
             // Handle a login request eminating from the form. If 2FA is required the
             // user will be presented with the 2FA modal window.
-            handleLogin: function () {
+            submitForm: function () {
                 const self = this;
+                this.$data.showSpinner = true;
 
                 axios.post(this.route('auth.login'), {
                     user: this.$props.user.email,
@@ -88,17 +91,20 @@
                         }
 
                         self.$props.user.password = '';
+                        self.$data.showSpinner = false;
                         self.$router.push({name: 'checkpoint', query: {token: response.data.token}});
                     })
                     .catch(function (err) {
                         self.$props.user.password = '';
+                        self.$data.showSpinner = false;
                         if (!err.response) {
                             return console.error(err);
                         }
 
                         const response = err.response;
                         if (response.data && _.isObject(response.data.errors)) {
-                            self.$data.errors.push(response.data.errors[0].detail);
+                            self.$data.errors = [response.data.errors[0].detail];
+                            self.$refs.password.focus();
                         }
                     });
             },
diff --git a/resources/assets/pterodactyl/styles/components/buttons.css b/resources/assets/pterodactyl/styles/components/buttons.css
index f9cf3f7c1..08cf2a519 100644
--- a/resources/assets/pterodactyl/styles/components/buttons.css
+++ b/resources/assets/pterodactyl/styles/components/buttons.css
@@ -7,7 +7,7 @@
   &.btn-blue {
     @apply .bg-blue .border-blue-dark .border .text-white;
 
-    &:hover {
+    &:hover:enabled {
       @apply .bg-blue-dark .border-blue-darker;
     }
   }
@@ -18,4 +18,9 @@
   &.btn-jumbo {
     @apply .p-4 .w-full .uppercase .tracking-wide .text-sm;
   }
+
+  &:disabled {
+    opacity: 0.55;
+    cursor: default;
+  }
 }
diff --git a/resources/assets/pterodactyl/styles/components/spinners.css b/resources/assets/pterodactyl/styles/components/spinners.css
new file mode 100644
index 000000000..e81725f55
--- /dev/null
+++ b/resources/assets/pterodactyl/styles/components/spinners.css
@@ -0,0 +1,52 @@
+.spinner {
+  color: transparent;
+  pointer-events: none;
+  position: relative;
+
+  &:after {
+    animation: spinners--spin 500ms infinite linear;
+    border-radius: 9999px;
+    @apply .border-2 .border-grey-light;
+    border-top-color: transparent !important;
+    border-right-color: transparent !important;
+    content: '';
+    display: block;
+    width: 1em;
+    height: 1em;
+    left: calc(50% - (1em / 2));
+    top: calc(50% - (1em / 2));
+    position: absolute !important;
+  }
+
+  /**
+   * Speeds
+   */
+  &.spin-slow:after {
+    animation: spinners--spin 1200ms infinite linear;
+  }
+
+  /**
+   * Spinner Colors
+   */
+  &.blue:after {
+    @apply .border-blue;
+  }
+
+  &.white:after {
+    @apply .border-white;
+  }
+
+  &.spinner-thick:after {
+    @apply .border-4;
+  }
+}
+
+@keyframes spinners--spin {
+  from {
+    transform: rotate(0deg);
+  }
+
+  to {
+    transform: rotate(360deg);
+  }
+}
diff --git a/resources/assets/pterodactyl/styles/main.css b/resources/assets/pterodactyl/styles/main.css
index 0a7a5a792..a9a309f9d 100644
--- a/resources/assets/pterodactyl/styles/main.css
+++ b/resources/assets/pterodactyl/styles/main.css
@@ -10,6 +10,7 @@
 @import "components/animations.css";
 @import "components/authentication.css";
 @import "components/buttons.css";
+@import "components/spinners.css";
 
 /**
  * Tailwind Utilities
diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php
index a1a553074..c94227b8c 100644
--- a/resources/lang/en/auth.php
+++ b/resources/lang/en/auth.php
@@ -19,7 +19,8 @@ return [
     'reset_password_text' => 'Reset your account password.',
     'reset_password' => 'Reset Account Password',
     'email_sent' => 'An email has been sent to you with further instructions for resetting your password.',
-    'failed' => 'The credentials provided do not match those we have on record, or the 2FA token provided was invalid.',
+    'failed' => 'No account matching those credentials could be found.',
+    'checkpoint_failed' => 'The two-factor authentication token was invalid.',
     'throttle' => 'Too many login attempts. Please try again in :seconds seconds.',
     'password_requirements' => 'Passwords must contain at least one uppercase, lowercase, and numeric character and must be at least 8 characters in length.',
     'request_reset' => 'Locate Account',
diff --git a/routes/auth.php b/routes/auth.php
index 57bcfe98a..fe1e65f0c 100644
--- a/routes/auth.php
+++ b/routes/auth.php
@@ -9,14 +9,20 @@
 |
 */
 Route::group(['middleware' => 'guest'], function () {
+    // Login specific routes
     Route::get('/login', 'LoginController@showLoginForm')->name('auth.login');
-    Route::get('/password/reset/{token}', 'ResetPasswordController@showResetForm')->name('auth.reset');
-
     Route::post('/login', 'LoginController@login')->middleware('recaptcha');
     Route::post('/login/checkpoint', 'LoginCheckpointController@index')->name('auth.checkpoint');
-    Route::post('/password', 'ForgotPasswordController@sendResetLinkEmail')->middleware('recaptcha');
+
+    // Forgot password route. A post to this endpoint will trigger an
+    // email to be sent containing a reset token.
+    Route::post('/password', 'ForgotPasswordController@sendResetLinkEmail')->name('auth.forgot-password.send-link')->middleware('recaptcha');
+
+    // Password reset routes. This endpoint is hit after going through
+    // the forgot password routes to acquire a token (or after an account
+    // is created).
+    Route::get('/password/reset/{token}', 'ResetPasswordController@showResetForm')->name('auth.reset-password');
     Route::post('/password/reset', 'ResetPasswordController@reset')->name('auth.reset.post')->middleware('recaptcha');
-    Route::post('/password/reset/{token}', 'ForgotPasswordController@sendResetLinkEmail')->middleware('recaptcha');
 });
 
 /*