From 43b8b0c14fd1476ec72295ed3cbb568ebeb10f23 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 24 Jun 2020 23:34:37 +0200 Subject: [PATCH] fix strings being passed where arrays where expected also brought type hints up to internal API level Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 25 +++---------------------- apps/user_ldap/lib/Group_LDAP.php | 7 ++++--- apps/user_ldap/tests/Group_LDAPTest.php | 6 ++++++ apps/user_ldap/tests/Jobs/SyncTest.php | 14 ++++++++++++++ apps/user_ldap/tests/User_LDAPTest.php | 18 ++++++++++++++++++ 5 files changed, 45 insertions(+), 25 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 671f74825b..b93d92b81d 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -847,12 +847,8 @@ class Access extends LDAPUtility { /** * fetches a list of users according to a provided loginName and utilizing * the login filter. - * - * @param string $loginName - * @param array $attributes optional, list of attributes to read - * @return array */ - public function fetchUsersByLoginName($loginName, $attributes = ['dn']) { + public function fetchUsersByLoginName(string $loginName, array $attributes = ['dn']): array { $loginName = $this->escapeFilterPart($loginName); $filter = str_replace('%uid', $loginName, $this->connection->ldapLoginFilter); return $this->fetchListOfUsers($filter, $attributes); @@ -872,15 +868,9 @@ class Access extends LDAPUtility { } /** - * @param string $filter - * @param string|string[] $attr - * @param int $limit - * @param int $offset - * @param bool $forceApplyAttributes - * @return array * @throws \Exception */ - public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null, $forceApplyAttributes = false) { + public function fetchListOfUsers(string $filter, array $attr, int $limit = null, int $offset = null, bool $forceApplyAttributes = false): array { $ldapRecords = $this->searchUsers($filter, $attr, $limit, $offset); $recordsToUpdate = $ldapRecords; if (!$forceApplyAttributes) { @@ -992,18 +982,9 @@ class Access extends LDAPUtility { } /** - * executes an LDAP search, optimized for Users - * - * @param string $filter the LDAP filter for the search - * @param string|string[] $attr optional, when a certain attribute shall be filtered out - * @param integer $limit - * @param integer $offset - * @return array with the search result - * - * Executes an LDAP search * @throws ServerNotAvailableException */ - public function searchUsers($filter, $attr = null, $limit = null, $offset = null) { + public function searchUsers(string $filter, array $attr = null, int $limit = null, int $offset = null): array { $result = []; foreach ($this->connection->ldapBaseUsers as $base) { $result = array_merge($result, $this->search($filter, $base, $attr, $limit, $offset)); diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index a6cd9669c9..6b62f88123 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -140,6 +140,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD //extra work if we don't get back user DNs if (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') { + $requestAttributes = $this->access->userManager->getAttributes(true); $dns = []; $filterParts = []; $bytes = 0; @@ -153,13 +154,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $filter = $this->access->combineFilterWithOr($filterParts); $bytes = 0; $filterParts = []; - $users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts)); + $users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); $dns = array_merge($dns, $users); } } if (count($filterParts) > 0) { $filter = $this->access->combineFilterWithOr($filterParts); - $users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts)); + $users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); $dns = array_merge($dns, $users); } $members = $dns; @@ -989,7 +990,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD str_replace('%uid', $member, $this->access->connection->ldapLoginFilter), $this->access->getFilterPartForUserSearch($search) ]); - $ldap_users = $this->access->fetchListOfUsers($filter, 'dn', 1); + $ldap_users = $this->access->fetchListOfUsers($filter, ['dn'], 1); if (count($ldap_users) < 1) { continue; } diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 6a64a37f8c..60b8b06b3c 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -538,6 +538,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('isDNPartOfBase') ->willReturn(true); + $access->expects($this->any()) + ->method('combineFilterWithAnd') + ->willReturn('pseudo=filter'); $access->userManager = $this->createMock(Manager::class); $groupBackend = new GroupLDAP($access, $pluginManager); @@ -576,6 +579,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('isDNPartOfBase') ->willReturn(true); + $access->expects($this->any()) + ->method('combineFilterWithAnd') + ->willReturn('pseudo=filter'); $access->userManager = $this->createMock(Manager::class); $groupBackend = new GroupLDAP($access, $pluginManager); diff --git a/apps/user_ldap/tests/Jobs/SyncTest.php b/apps/user_ldap/tests/Jobs/SyncTest.php index 5f2adbe7dd..0971e31ae8 100644 --- a/apps/user_ldap/tests/Jobs/SyncTest.php +++ b/apps/user_ldap/tests/Jobs/SyncTest.php @@ -189,9 +189,16 @@ class SyncTest extends TestCase { ->with($connection) ->willReturn($access); + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); + $access->expects($this->once()) ->method('fetchListOfUsers') ->willReturn(array_pad([], $results, 'someUser')); + $access->expects($this->any()) + ->method('combineFilterWithAnd') + ->willReturn('pseudo=filter'); $access->connection = $connection; $access->userManager = $this->userManager; @@ -356,9 +363,16 @@ class SyncTest extends TestCase { ->with($connection) ->willReturn($access); + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); + $access->expects($this->once()) ->method('fetchListOfUsers') ->willReturn(array_pad([], $runData['usersThisCycle'], 'someUser')); + $access->expects($this->any()) + ->method('combineFilterWithAnd') + ->willReturn('pseudo=filter'); $access->connection = $connection; $access->userManager = $this->userManager; diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index 00ca673717..bfdaa99ac0 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -183,6 +183,10 @@ class User_LDAPTest extends TestCase { } return false; }); + + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); } public function testCheckPasswordUidReturn() { @@ -400,6 +404,10 @@ class User_LDAPTest extends TestCase { ->willReturnArgument(0); $this->access->method('fetchUsersByLoginName') ->willReturn([]); + + $this->access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); } public function testGetUsersNoParam() { @@ -1065,6 +1073,9 @@ class User_LDAPTest extends TestCase { ->method('get') ->with($dn) ->willReturn($user); + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); $name = $backend->loginName2UserName($loginName); $this->assertSame($username, $name); @@ -1093,6 +1104,10 @@ class User_LDAPTest extends TestCase { ->method('writeToCache') ->with($this->equalTo('loginName2UserName-'.$loginName), false); + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); + $backend = new UserLDAP($this->access, $this->config, $this->notificationManager, $this->session, $this->pluginManager); $name = $backend->loginName2UserName($loginName); $this->assertSame(false, $name); @@ -1126,6 +1141,9 @@ class User_LDAPTest extends TestCase { ->method('get') ->with($dn) ->willReturn($offlineUser); + $this->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['dn', 'uid', 'mail', 'displayname']); $backend = new UserLDAP($this->access, $this->config, $this->notificationManager, $this->session, $this->pluginManager); $name = $backend->loginName2UserName($loginName);