diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php index 3c98140f6..84f54ad29 100644 --- 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']); - $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); - $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; } diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index 07e9f7b64..0d6ebfc80 100644 --- 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 */ - 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; } diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index cb1194e22..06f88c222 100644 --- 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']); } - 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(); @@ -186,6 +186,23 @@ class LdapTest extends BrowserKitTest ->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')