From 4db523c3faffe2e0503fd72e18cb7a5faba31926 Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=B4me=20Chilliet?= Date: Thu, 3 Mar 2022 16:35:06 +0100 Subject: [PATCH] Fix unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/Access.php | 8 ++---- apps/user_ldap/lib/Connection.php | 38 ++++++++++++++++++++----- apps/user_ldap/lib/Group_LDAP.php | 2 +- apps/user_ldap/lib/ILDAPWrapper.php | 4 +-- apps/user_ldap/lib/User/User.php | 7 ++--- apps/user_ldap/tests/AccessTest.php | 6 ++-- apps/user_ldap/tests/Group_LDAPTest.php | 34 +++++++++++++--------- apps/user_ldap/tests/User/UserTest.php | 8 ++++++ apps/user_ldap/tests/User_LDAPTest.php | 16 ++++++----- apps/user_ldap/tests/WizardTest.php | 2 +- 10 files changed, 81 insertions(+), 44 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 67a80dcbba7..0fea6ace996 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -1526,7 +1526,7 @@ class Access extends LDAPUtility { * @param string $search the search term * @return string the final filter part to use in LDAP searches */ - public function getFilterPartForUserSearch($search) { + public function getFilterPartForUserSearch($search): string { return $this->getFilterPartForSearch($search, $this->connection->ldapAttributesForUserSearch, $this->connection->ldapUserDisplayName); @@ -1538,7 +1538,7 @@ class Access extends LDAPUtility { * @param string $search the search term * @return string the final filter part to use in LDAP searches */ - public function getFilterPartForGroupSearch($search) { + public function getFilterPartForGroupSearch($search): string { return $this->getFilterPartForSearch($search, $this->connection->ldapAttributesForGroupSearch, $this->connection->ldapGroupDisplayName); @@ -1632,10 +1632,8 @@ class Access extends LDAPUtility { /** * returns the filter used for counting users - * - * @return string */ - public function getFilterForUserCount() { + public function getFilterForUserCount(): string { $filter = $this->combineFilterWithAnd([ $this->connection->ldapUserFilter, $this->connection->ldapUserDisplayName . '=*' diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 77ae34f9f6c..4abea708a0d 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -75,10 +75,25 @@ use Psr\Log\LoggerInterface; */ class Connection extends LDAPUtility { private $ldapConnectionRes = null; + + /** + * @var string + */ private $configPrefix; + + /** + * @var ?string + */ private $configID; + + /** + * @var bool + */ private $configured = false; - //whether connection should be kept on __destruct + + /** + * @var bool whether connection should be kept on __destruct + */ private $dontDestruct = false; /** @@ -91,16 +106,27 @@ class Connection extends LDAPUtility { */ public $hasGidNumber = true; - //cache handler - protected $cache; + /** + * @var \OCP\ICache|null + */ + protected $cache = null; /** @var Configuration settings handler **/ protected $configuration; + /** + * @var bool + */ protected $doNotValidate = false; + /** + * @var bool + */ protected $ignoreValidation = false; + /** + * @var array{dn?: mixed, hash?: string, result?: bool} + */ protected $bindResult = []; /** @var LoggerInterface */ @@ -108,16 +134,14 @@ class Connection extends LDAPUtility { /** * Constructor - * @param ILDAPWrapper $ldap * @param string $configPrefix a string with the prefix for the configkey column (appconfig table) * @param string|null $configID a string with the value for the appid column (appconfig table) or null for on-the-fly connections */ - public function __construct(ILDAPWrapper $ldap, $configPrefix = '', $configID = 'user_ldap') { + public function __construct(ILDAPWrapper $ldap, string $configPrefix = '', ?string $configID = 'user_ldap') { parent::__construct($ldap); $this->configPrefix = $configPrefix; $this->configID = $configID; - $this->configuration = new Configuration($configPrefix, - !is_null($configID)); + $this->configuration = new Configuration($configPrefix, !is_null($configID)); $memcache = \OC::$server->getMemCacheFactory(); if ($memcache->isAvailable()) { $this->cache = $memcache->createDistributed(); diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 29ef8958293..45478c00973 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -1349,7 +1349,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I $this->access->groupname2dn($gid), $this->access->connection->ldapGroupDisplayName); - if ($displayName && (count($displayName) > 0)) { + if (($displayName !== false) && (count($displayName) > 0)) { $displayName = $displayName[0]; $this->access->connection->writeToCache($cacheKey, $displayName); return $displayName; diff --git a/apps/user_ldap/lib/ILDAPWrapper.php b/apps/user_ldap/lib/ILDAPWrapper.php index c82df09d234..695667ee41f 100644 --- a/apps/user_ldap/lib/ILDAPWrapper.php +++ b/apps/user_ldap/lib/ILDAPWrapper.php @@ -178,8 +178,8 @@ interface ILDAPWrapper { /** * Sets the value of the specified option to be $value * @param resource $link LDAP link resource - * @param string $option a defined LDAP Server option - * @param int $value the new value for the option + * @param int $option a defined LDAP Server option + * @param mixed $value the new value for the option * @return bool true on success, false otherwise */ public function setOption($link, $option, $value); diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index b09fbd18327..2c81c87fd47 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -274,8 +274,8 @@ class User { /** * returns the home directory of the user if specified by LDAP settings - * @param string $valueFromLDAP - * @return bool|string + * @param ?string $valueFromLDAP + * @return false|string * @throws \Exception */ public function getHomePath($valueFromLDAP = null) { @@ -286,8 +286,7 @@ class User { && strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0 && $this->access->connection->homeFolderNamingRule !== 'attr:') { $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); - $homedir = $this->access->readAttribute( - $this->access->username2dn($this->getUsername()), $attr); + $homedir = $this->access->readAttribute($this->access->username2dn($this->getUsername()), $attr); if ($homedir && isset($homedir[0])) { $path = $homedir[0]; } diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 4547e7e824e..402e7d1e8e4 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -113,7 +113,7 @@ class AccessTest extends TestCase { private function getConnectorAndLdapMock() { $lw = $this->createMock(ILDAPWrapper::class); $connector = $this->getMockBuilder(Connection::class) - ->setConstructorArgs([$lw, null, null]) + ->setConstructorArgs([$lw, '', null]) ->getMock(); $um = $this->getMockBuilder(Manager::class) ->setConstructorArgs([ @@ -495,7 +495,7 @@ class AccessTest extends TestCase { ->willReturn(true); $connection = $this->createMock(LDAP::class); $this->connection - ->expects($this->once()) + ->expects($this->any()) ->method('getConnectionResource') ->willReturn($connection); $this->ldap @@ -519,7 +519,7 @@ class AccessTest extends TestCase { ->willReturn(true); $connection = $this->createMock(LDAP::class); $this->connection - ->expects($this->once()) + ->expects($this->any()) ->method('getConnectionResource') ->willReturn($connection); $this->ldap diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index cb637dcc108..f8327c0776c 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -104,14 +104,12 @@ class Group_LDAPTest extends TestCase { $lw = $this->createMock(ILDAPWrapper::class); $connector = $this->getMockBuilder(Connection::class) ->setMethods($conMethods) - ->setConstructorArgs([$lw, null, null]) + ->setConstructorArgs([$lw, '', null]) ->getMock(); $access = $this->createMock(Access::class); - $access->expects($this->any()) - ->method('getConnection') - ->willReturn($connector); + $access->connection = $connector; $access->userManager = $this->createMock(Manager::class); @@ -133,6 +131,8 @@ class Group_LDAPTest extends TestCase { ->willReturnCallback(function ($name) { if ($name === 'ldapDynamicGroupMemberURL') { return ''; + } elseif ($name === 'ldapBaseGroups') { + return []; } return 1; }); @@ -953,6 +953,8 @@ class Group_LDAPTest extends TestCase { return 'member'; case 'ldapGroupFilter': return $groupFilter; + case 'ldapBaseGroups': + return []; } return 1; }); @@ -1321,16 +1323,16 @@ class Group_LDAPTest extends TestCase { }); $access->connection = $this->createMock(Connection::class); - if (count($groupsInfo) > 1) { - $access->connection->expects($this->any()) - ->method('__get') - ->willReturnCallback(function ($name) { - if ($name === 'ldapNestedGroups') { - return 1; - } - return null; - }); - } + $access->connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($name) { + if ($name === 'ldapNestedGroups') { + return 1; + } elseif ($name === 'ldapGroupMemberAssocAttr') { + return 'attr'; + } + return null; + }); /** @var GroupPluginManager $pluginManager */ $pluginManager = $this->createMock(GroupPluginManager::class); @@ -1373,6 +1375,10 @@ class Group_LDAPTest extends TestCase { return null; }); + $access->expects($this->any()) + ->method('groupname2dn') + ->willReturn('fakedn'); + /** @var GroupPluginManager $pluginManager */ $pluginManager = $this->createMock(GroupPluginManager::class); diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index b1b2d9ac391..eb5abdb5c8e 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -1016,6 +1016,10 @@ class UserTest extends \Test\TestCase { ->method('readAttribute') ->willReturn(false); + $this->access->expects($this->once()) + ->method('username2dn') + ->willReturn($this->dn); + // asks for "enforce_home_folder_naming_rule" $this->config->expects($this->once()) ->method('getAppValue') @@ -1038,6 +1042,10 @@ class UserTest extends \Test\TestCase { ->method('readAttribute') ->willReturn(false); + $this->access->expects($this->once()) + ->method('username2dn') + ->willReturn($this->dn); + // asks for "enforce_home_folder_naming_rule" $this->config->expects($this->once()) ->method('getAppValue') diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index 78767ff6760..24ebb97b790 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -815,13 +815,15 @@ class User_LDAPTest extends TestCase { private function prepareAccessForGetDisplayName() { $this->connection->expects($this->any()) - ->method('__get') - ->willReturnCallback(function ($name) { - if ($name === 'ldapUserDisplayName') { - return 'displayname'; - } - return null; - }); + ->method('__get') + ->willReturnCallback(function ($name) { + if ($name === 'ldapUserDisplayName') { + return 'displayname'; + } elseif ($name === 'ldapUserDisplayName2') { + return 'displayname2'; + } + return null; + }); $this->access->expects($this->any()) ->method('readAttribute') diff --git a/apps/user_ldap/tests/WizardTest.php b/apps/user_ldap/tests/WizardTest.php index ae25aad44ab..5382a0c7f6f 100644 --- a/apps/user_ldap/tests/WizardTest.php +++ b/apps/user_ldap/tests/WizardTest.php @@ -72,7 +72,7 @@ class WizardTest extends TestCase { /** @var Configuration|\PHPUnit\Framework\MockObject\MockObject $conf */ $conf = $this->getMockBuilder(Configuration::class) ->setMethods($confMethods) - ->setConstructorArgs([$lw, null, null]) + ->setConstructorArgs(['', true]) ->getMock(); /** @var Access|\PHPUnit\Framework\MockObject\MockObject $access */ -- 2.39.5