From 83da17715235776aacdd3cbdeeb8bc1ef936504c Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 11:59:28 +0530 Subject: [PATCH 1/7] Add: Ability to merge firstname and lastname when fullname is not there in ldap and add firstname and lastname in testcase --- app/Access/LdapService.php | 12 ++++++++++-- tests/Auth/LdapTest.php | 4 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index 365cb1db015..6db00433ace 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -82,17 +82,25 @@ public function getUserDetails(string $userName): ?array $idAttr = $this->config['id_attribute']; $emailAttr = $this->config['email_attribute']; $displayNameAttr = $this->config['display_name_attribute']; + $firstName = explode(',',$displayNameAttr)[0]; + $lastName = explode(',',$displayNameAttr)[0]; $thumbnailAttr = $this->config['thumbnail_attribute']; $user = $this->getUserWithAttributes($userName, array_filter([ - 'cn', 'dn', $idAttr, $emailAttr, $displayNameAttr, $thumbnailAttr, + 'cn', 'dn', $idAttr, $emailAttr, $firstName,$lastName, $thumbnailAttr, ])); if (is_null($user)) { return null; } - + $userCn = $this->getUserResponseProperty($user, 'cn', null); + + if($userCn === null) + { + $userCn = $this->getUserResponseProperty($user, $firstName, null) . ' ' . $this->getUserResponseProperty($user, $lastName, null); + } + $formatted = [ 'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']), 'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn), diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index ef95bc2e8f4..40850d63796 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -29,7 +29,7 @@ protected function setUp(): void 'auth.defaults.guard' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'services.ldap.email_attribute' => 'mail', - 'services.ldap.display_name_attribute' => 'cn', + 'services.ldap.display_name_attribute' => 'givenName', 'services.ldap.id_attribute' => 'uid', 'services.ldap.user_to_groups' => false, 'services.ldap.version' => '3', @@ -590,6 +590,8 @@ public function test_login_uses_specified_display_name_attribute() ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], 'cn' => [$this->mockUser->name], + 'givenName' => [explode(" ",$this->mockUser)[0]], + 'sn' => [explode(" ",$this->mockUser)[1]], 'dn' => 'dc=test' . config('services.ldap.base_dn'), 'displayname' => 'displayNameAttribute', ]]); From 291bb01507ed0aca197c75aa1764c678b17b29ed Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 12:19:17 +0530 Subject: [PATCH 2/7] fix: lint-php --- app/Access/LdapService.php | 10 +++++----- tests/Auth/LdapTest.php | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index 6db00433ace..3596b6f3945 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -82,8 +82,8 @@ public function getUserDetails(string $userName): ?array $idAttr = $this->config['id_attribute']; $emailAttr = $this->config['email_attribute']; $displayNameAttr = $this->config['display_name_attribute']; - $firstName = explode(',',$displayNameAttr)[0]; - $lastName = explode(',',$displayNameAttr)[0]; + $firstName = explode(',', $displayNameAttr)[0]; + $lastName = explode(',', $displayNameAttr)[0]; $thumbnailAttr = $this->config['thumbnail_attribute']; $user = $this->getUserWithAttributes($userName, array_filter([ @@ -93,13 +93,13 @@ public function getUserDetails(string $userName): ?array if (is_null($user)) { return null; } - + $userCn = $this->getUserResponseProperty($user, 'cn', null); - if($userCn === null) + if ($userCn === null) { $userCn = $this->getUserResponseProperty($user, $firstName, null) . ' ' . $this->getUserResponseProperty($user, $lastName, null); - } + } $formatted = [ 'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']), diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 40850d63796..6ddfe2584ca 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -590,8 +590,8 @@ public function test_login_uses_specified_display_name_attribute() ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], 'cn' => [$this->mockUser->name], - 'givenName' => [explode(" ",$this->mockUser)[0]], - 'sn' => [explode(" ",$this->mockUser)[1]], + 'givenName' => [explode(" ", $this->mockUser)[0]], + 'sn' => [explode(" ", $this->mockUser)[1]], 'dn' => 'dc=test' . config('services.ldap.base_dn'), 'displayname' => 'displayNameAttribute', ]]); From 3d0147a54a4c621d8d10250fb159397452e9b7d0 Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 12:22:30 +0530 Subject: [PATCH 3/7] fix: lint-php --- app/Access/LdapService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index 3596b6f3945..19dfefec0e7 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -96,10 +96,10 @@ public function getUserDetails(string $userName): ?array $userCn = $this->getUserResponseProperty($user, 'cn', null); - if ($userCn === null) + if ($userCn === null) { $userCn = $this->getUserResponseProperty($user, $firstName, null) . ' ' . $this->getUserResponseProperty($user, $lastName, null); - } + } $formatted = [ 'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']), From c7b1439dc7544e82e6d42bbbf06d108cbcbdcd45 Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 12:25:56 +0530 Subject: [PATCH 4/7] fix: lint-php --- app/Access/LdapService.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index 19dfefec0e7..a38997447a2 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -96,8 +96,7 @@ public function getUserDetails(string $userName): ?array $userCn = $this->getUserResponseProperty($user, 'cn', null); - if ($userCn === null) - { + if ($userCn === null) { $userCn = $this->getUserResponseProperty($user, $firstName, null) . ' ' . $this->getUserResponseProperty($user, $lastName, null); } From 21e28695b036582afbf2609ae75ffd6fad9135a6 Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 16:10:42 +0530 Subject: [PATCH 5/7] add: test case if cn(common name is not there then givenName and sn will merge and use as display name --- tests/Auth/LdapTest.php | 42 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 6ddfe2584ca..55019f7b6a9 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -29,7 +29,7 @@ protected function setUp(): void 'auth.defaults.guard' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'services.ldap.email_attribute' => 'mail', - 'services.ldap.display_name_attribute' => 'givenName', + 'services.ldap.display_name_attribute' => 'cn', 'services.ldap.id_attribute' => 'uid', 'services.ldap.user_to_groups' => false, 'services.ldap.version' => '3', @@ -634,6 +634,46 @@ public function test_login_uses_default_display_name_attribute_if_specified_not_ ]); } + public function test_login_uses_givenName_and_sn_merge_if_display_name_is_not_present() + { + app('config')->set([ + 'services.ldap.display_name_attribute' => 'displayName', + ]); + + $this->mockUser->firstName = explode(" ", $this->mockUser->name)[0]; + $this->mockUser->lastName = explode(" ", $this->mockUser->name)[1]; + + $this->commonLdapMocks(1, 1, 2, 4, 2); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) + ->andReturn(['count' => 1, 0 => [ + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'givenName' => [$this->mockUser->firstName], + 'sn' => [$this->mockUser->lastName], + 'dn' => 'dc=test' . config('services.ldap.base_dn'), + ]]); + + $this->mockUserLogin()->assertRedirect('/login'); + $this->get('/login')->assertSee('Please enter an email to use for this account.'); + + $resp = $this->mockUserLogin($this->mockUser->email); + $resp->assertRedirect('/'); + + $expectedDisplayName = $this->mockUser->firstName . ' ' . $this->mockUser->lastName; + + $this->get('/')->assertSee($expectedDisplayName); + + $this->assertDatabaseHas('users', [ + 'email' => $this->mockUser->email, + 'email_confirmed' => false, + 'external_auth_id' => $this->mockUser->name, + 'name' => $this->mockUser->name, + ]); + } + + + protected function checkLdapReceivesCorrectDetails($serverString, $expectedHostString): void { app('config')->set(['services.ldap.server' => $serverString]); From b891b90dac7914caf4ad133b5d73895236c6d0d9 Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 16:35:38 +0530 Subject: [PATCH 6/7] fix: lint-php --- tests/Auth/LdapTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 55019f7b6a9..7b396263233 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -653,13 +653,13 @@ public function test_login_uses_givenName_and_sn_merge_if_display_name_is_not_pr 'sn' => [$this->mockUser->lastName], 'dn' => 'dc=test' . config('services.ldap.base_dn'), ]]); - + $this->mockUserLogin()->assertRedirect('/login'); $this->get('/login')->assertSee('Please enter an email to use for this account.'); - + $resp = $this->mockUserLogin($this->mockUser->email); $resp->assertRedirect('/'); - + $expectedDisplayName = $this->mockUser->firstName . ' ' . $this->mockUser->lastName; $this->get('/')->assertSee($expectedDisplayName); From d3268076ee32d5bbc7a1692dbdb15a2d2a738d67 Mon Sep 17 00:00:00 2001 From: Brij Mandaliya Date: Tue, 15 Oct 2024 19:30:08 +0530 Subject: [PATCH 7/7] correct: lastname value is not properly assign for displayname --- app/Access/LdapService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index a38997447a2..1d64ce3fd57 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -83,7 +83,7 @@ public function getUserDetails(string $userName): ?array $emailAttr = $this->config['email_attribute']; $displayNameAttr = $this->config['display_name_attribute']; $firstName = explode(',', $displayNameAttr)[0]; - $lastName = explode(',', $displayNameAttr)[0]; + $lastName = explode(',', $displayNameAttr)[1]; $thumbnailAttr = $this->config['thumbnail_attribute']; $user = $this->getUserWithAttributes($userName, array_filter([