From 7728931f150cb9f80a98cf6a2f947d7f25532cc4 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 1 Feb 2020 14:30:23 +0000
Subject: [PATCH] Set more appropriate login validation and broken up LDAP
 guide a bit

---
 app/Auth/Access/Guards/LdapSessionGuard.php   | 65 ++++++++++++-------
 app/Http/Controllers/Auth/LoginController.php | 37 +++++++++++
 2 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php
index ad173cf73..223088d05 100644
--- a/app/Auth/Access/Guards/LdapSessionGuard.php
+++ b/app/Auth/Access/Guards/LdapSessionGuard.php
@@ -75,29 +75,8 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
             $user = $this->freshUserInstanceFromLdapUserDetails($userDetails);
         }
 
-        $providedEmail = ($credentials['email'] ?? false);
-
-        // Request email if missing from LDAP and model and missing from request
-        if (is_null($user->email) && !$providedEmail) {
-            throw new LoginAttemptEmailNeededException();
-        }
-
-        // Add email to model if non-existing and email provided in request
-        if (!$user->exists && $user->email === null && $providedEmail) {
-            $user->email = $providedEmail;
-        }
-
-        if (!$user->exists) {
-            // Check for existing users with same email
-            $alreadyUser = $user->newQuery()->where('email', '=', $user->email)->count() > 0;
-            if ($alreadyUser) {
-                throw new LoginAttemptException(trans('errors.error_user_exists_different_creds', ['email' => $user->email]));
-            }
-
-            $user->save();
-            $this->userRepo->attachDefaultRole($user);
-            $this->userRepo->downloadAndAssignUserAvatar($user);
-        }
+        $this->checkForUserEmail($user, $credentials['email'] ?? '');
+        $this->saveIfNew($user);
 
         // Sync LDAP groups if required
         if ($this->ldapService->shouldSyncGroups()) {
@@ -108,6 +87,46 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
         return true;
     }
 
+    /**
+     * Save the give user if they don't yet existing in the system.
+     * @throws LoginAttemptException
+     */
+    protected function saveIfNew(User $user)
+    {
+        if ($user->exists) {
+            return;
+        }
+
+        // Check for existing users with same email
+        $alreadyUser = $user->newQuery()->where('email', '=', $user->email)->count() > 0;
+        if ($alreadyUser) {
+            throw new LoginAttemptException(trans('errors.error_user_exists_different_creds', ['email' => $user->email]));
+        }
+
+        $user->save();
+        $this->userRepo->attachDefaultRole($user);
+        $this->userRepo->downloadAndAssignUserAvatar($user);
+    }
+
+    /**
+     * Ensure the given user has an email.
+     * Takes the provided email in the request if a value is provided
+     * and the user does not have an existing email.
+     * @throws LoginAttemptEmailNeededException
+     */
+    protected function checkForUserEmail(User $user, string $providedEmail)
+    {
+        // Request email if missing from user and missing from request
+        if (is_null($user->email) && !$providedEmail) {
+            throw new LoginAttemptEmailNeededException();
+        }
+
+        // Add email to model if non-existing and email provided in request
+        if (!$user->exists && is_null($user->email) && $providedEmail) {
+            $user->email = $providedEmail;
+        }
+    }
+
     /**
      * Create a fresh user instance from details provided by a LDAP lookup.
      */
diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index 1ff86fff6..2302937cb 100644
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -119,6 +119,43 @@ class LoginController extends Controller
         return $this->sendFailedLoginResponse($request);
     }
 
+    /**
+     * Validate the user login request.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @return void
+     *
+     * @throws \Illuminate\Validation\ValidationException
+     */
+    protected function validateLogin(Request $request)
+    {
+        $rules = [];
+        $authMethod = config('auth.method');
+
+        if ($authMethod === 'standard') {
+            $rules = [
+                'email' => 'required|string|email',
+                'password' => 'required|string'
+            ];
+        }
+
+        if ($authMethod === 'ldap') {
+            $rules = [
+                'username' => 'required|string',
+                'password' => 'required|string',
+                'email' => 'email',
+            ];
+        }
+
+        if ($authMethod === 'saml2') {
+            $rules = [
+                'email' => 'email',
+            ];
+        }
+
+        $request->validate($rules);
+    }
+
     /**
      * Send a response when a login attempt exception occurs.
      */