Fixed LDAP error thrown by not found user details

- Added testing to cover.

Related to #1876

diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php
index 3c98140f621b560f7f370d5fa0dab08eed29b5d6..84f54ad292c9191479519840dc79b736db40bcb3 100644 (file)
--- a/app/Auth/Access/Guards/LdapSessionGuard.php
+++ b/app/Auth/Access/Guards/LdapSessionGuard.php
@@ -44,11 +44,14 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
     public function validate(array $credentials = [])
     {
         $userDetails = $this->ldapService->getUserDetails($credentials['username']);
     public function validate(array $credentials = [])
     {
         $userDetails = $this->ldapService->getUserDetails($credentials['username']);

-        $this->lastAttempted = $this->provider->retrieveByCredentials([
-            'external_auth_id' => $userDetails['uid']
-        ]);

 
 

-        return $this->ldapService->validateUserCredentials($userDetails, $credentials['username'], $credentials['password']);
+        if (isset($userDetails['uid'])) {
+            $this->lastAttempted = $this->provider->retrieveByCredentials([
+                'external_auth_id' => $userDetails['uid']
+            ]);
+        }
+
+        return $this->ldapService->validateUserCredentials($userDetails, $credentials['password']);

     }
 
     /**
     }
 
     /**


@@ -66,11 +69,15 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
     {
         $username = $credentials['username'];
         $userDetails = $this->ldapService->getUserDetails($username);
     {
         $username = $credentials['username'];
         $userDetails = $this->ldapService->getUserDetails($username);

-        $this->lastAttempted = $user = $this->provider->retrieveByCredentials([
-            'external_auth_id' => $userDetails['uid']
-        ]);

 
 

-        if (!$this->ldapService->validateUserCredentials($userDetails, $username, $credentials['password'])) {
+        $user = null;
+        if (isset($userDetails['uid'])) {
+            $this->lastAttempted = $user = $this->provider->retrieveByCredentials([
+                'external_auth_id' => $userDetails['uid']
+            ]);
+        }
+
+        if (!$this->ldapService->validateUserCredentials($userDetails, $credentials['password'])) {

             return false;
         }
 
             return false;
         }
 

diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index 07e9f7b64e6b4516c53a4aa00f211a56d90598fb..0d6ebfc802aa076d8237fe867afbc5da573f9641 100644 (file)
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -102,9 +102,9 @@ class LdapService extends ExternalAuthService
      * Check if the given credentials are valid for the given user.
      * @throws LdapException
      */
      * Check if the given credentials are valid for the given user.
      * @throws LdapException
      */

-    public function validateUserCredentials(array $ldapUserDetails, string $username, string $password): bool
+    public function validateUserCredentials(?array $ldapUserDetails, string $password): bool

     {
     {

-        if ($ldapUserDetails === null) {
+        if (is_null($ldapUserDetails)) {

             return false;
         }
 
             return false;
         }
 

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index cb1194e22bc3a5c8515ce7acd19c80cba01fa6c1..06f88c222893e7ab3b84e4795609504d65ddd89e 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -166,7 +166,7 @@ class LdapTest extends BrowserKitTest
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => 'cooluser456']);
     }
 
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => 'cooluser456']);
     }
 

-    public function test_initial_incorrect_details()
+    public function test_initial_incorrect_credentials()

     {
         $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
         $this->mockLdap->shouldReceive('setVersion')->once();
     {
         $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
         $this->mockLdap->shouldReceive('setVersion')->once();


@@ -186,6 +186,23 @@ class LdapTest extends BrowserKitTest
             ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
     }
 
             ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
     }
 

+    public function test_login_not_found_username()
+    {
+        $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
+        $this->mockLdap->shouldReceive('setVersion')->once();
+        $this->mockLdap->shouldReceive('setOption')->times(1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->andReturn(['count' => 0]);
+        $this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true, false);
+        $this->mockEscapes(1);
+
+        $this->mockUserLogin()
+            ->seePageIs('/login')->see('These credentials do not match our records.')
+            ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
+    }
+
+

     public function test_create_user_form()
     {
         $this->asAdmin()->visit('/settings/users/create')
     public function test_create_user_form()
     {
         $this->asAdmin()->visit('/settings/users/create')