diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index ef6d33f4db1..e5037ad2f95 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -72,25 +72,23 @@ private function getUserWithAttributes(string $userName, array $attributes): ?ar } /** - * Calculate the display name. + * Build the user display name from the (potentially multiple) attributes defined by the configuration. */ - protected function getUserDisplayName(array $displayNameAttr, array $userDetails, string $defaultValue): string + protected function getUserDisplayName(array $userDetails, array $displayNameAttrs, string $defaultValue): string { - $displayName = []; - foreach ($displayNameAttr as $dnAttr) { + $displayNameParts = []; + foreach ($displayNameAttrs as $dnAttr) { $dnComponent = $this->getUserResponseProperty($userDetails, $dnAttr, null); - if ($dnComponent !== null) { - $displayName[] = $dnComponent; + if ($dnComponent) { + $displayNameParts[] = $dnComponent; } } - if (count($displayName) == 0) { - $displayName = $defaultValue; - } else { - $displayName = implode(' ', $displayName); + if (empty($displayNameParts)) { + return $defaultValue; } - return $displayName; + return implode(' ', $displayNameParts); } /** @@ -103,12 +101,12 @@ public function getUserDetails(string $userName): ?array { $idAttr = $this->config['id_attribute']; $emailAttr = $this->config['email_attribute']; - $displayNameAttr = $this->config['display_name_attribute']; + $displayNameAttrs = explode('|', $this->config['display_name_attribute']); $thumbnailAttr = $this->config['thumbnail_attribute']; - $user = $this->getUserWithAttributes($userName, array_filter(array_merge($displayNameAttr, [ - 'cn', 'dn', $idAttr, $emailAttr, $thumbnailAttr, - ]))); + $user = $this->getUserWithAttributes($userName, array_filter([ + 'cn', 'dn', $idAttr, $emailAttr, ...$displayNameAttrs, $thumbnailAttr, + ])); if (is_null($user)) { return null; @@ -117,7 +115,7 @@ public function getUserDetails(string $userName): ?array $userCn = $this->getUserResponseProperty($user, 'cn', null); $formatted = [ 'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']), - 'name' => $this->getUserDisplayName($displayNameAttr, $user, $userCn), + 'name' => $this->getUserDisplayName($user, $displayNameAttrs, $userCn), 'dn' => $user['dn'], 'email' => $this->getUserResponseProperty($user, $emailAttr, null), 'avatar' => $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null, diff --git a/app/Config/services.php b/app/Config/services.php index 4e27896870a..d7345823150 100644 --- a/app/Config/services.php +++ b/app/Config/services.php @@ -127,7 +127,7 @@ 'version' => env('LDAP_VERSION', false), 'id_attribute' => env('LDAP_ID_ATTRIBUTE', 'uid'), 'email_attribute' => env('LDAP_EMAIL_ATTRIBUTE', 'mail'), - 'display_name_attribute' => explode('|', env('LDAP_DISPLAY_NAME_ATTRIBUTE', 'cn')), + 'display_name_attribute' => env('LDAP_DISPLAY_NAME_ATTRIBUTE', 'cn'), 'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false), 'user_to_groups' => env('LDAP_USER_TO_GROUPS', false), 'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'), diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 27169a2becf..9a00c983a50 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' => 'cn', 'services.ldap.id_attribute' => 'uid', 'services.ldap.user_to_groups' => false, 'services.ldap.version' => '3', @@ -581,7 +581,7 @@ public function test_login_group_mapping_does_not_conflict_with_default_role() public function test_login_uses_specified_display_name_attribute() { app('config')->set([ - 'services.ldap.display_name_attribute' => ['displayName'], + 'services.ldap.display_name_attribute' => 'displayName', ]); $this->commonLdapMocks(1, 1, 2, 4, 2); @@ -603,10 +603,37 @@ public function test_login_uses_specified_display_name_attribute() $this->assertDatabaseHas('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']); } + public function test_login_uses_multiple_display_properties_if_defined() + { + app('config')->set([ + 'services.ldap.display_name_attribute' => 'firstname|middlename|noname|lastname', + ]); + + $this->commonLdapMocks(1, 1, 1, 2, 1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) + ->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], + 'dn' => 'dc=test' . config('services.ldap.base_dn'), + 'firstname' => ['Barry'], + 'middlename' => ['Elliott'], + 'lastname' => ['Chuckle'], + 'mail' => [$this->mockUser->email], + ]]); + + $this->mockUserLogin(); + + $this->assertDatabaseHas('users', [ + 'email' => $this->mockUser->email, + 'name' => 'Barry Elliott Chuckle', + ]); + } + public function test_login_uses_default_display_name_attribute_if_specified_not_present() { app('config')->set([ - 'services.ldap.display_name_attribute' => ['displayName'], + 'services.ldap.display_name_attribute' => 'displayName', ]); $this->commonLdapMocks(1, 1, 2, 4, 2);