diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index bda495bc9a852..29d60817c02eb 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -180,7 +180,7 @@ public function getConnection() { * array if $attr is empty, false otherwise * @throws ServerNotAvailableException */ - public function readAttribute($dn, $attr, $filter = 'objectClass=*') { + public function readAttribute(string $dn, string $attr, string $filter = 'objectClass=*') { if (!$this->checkConnection()) { $this->logger->warning( 'No LDAP Connector assigned, access impossible for readAttribute.', @@ -211,7 +211,7 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') { $values = []; $isRangeRequest = false; do { - $result = $this->executeRead($cr, $dn, $attrToRead, $filter, $maxResults); + $result = $this->executeRead($dn, $attrToRead, $filter, $maxResults); if (is_bool($result)) { // when an exists request was run and it was successful, an empty // array must be returned @@ -253,17 +253,12 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') { /** * Runs an read operation against LDAP * - * @param resource|\LDAP\Connection $cr the LDAP connection - * @param string $dn - * @param string $attribute - * @param string $filter - * @param int $maxResults * @return array|bool false if there was any error, true if an exists check * was performed and the requested DN found, array with the * returned data on a successful usual operation * @throws ServerNotAvailableException */ - public function executeRead($cr, $dn, $attribute, $filter, $maxResults) { + public function executeRead(string $dn, string $attribute, string $filter, int $maxResults) { try { $this->initPagedSearch($filter, $dn, [$attribute], $maxResults, 0); } catch (NoMoreResults $e) { @@ -273,7 +268,7 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) { return false; } $dn = $this->helper->DNasBaseParameter($dn); - $rr = @$this->invokeLDAPMethod('read', $cr, $dn, $filter, [$attribute]); + $rr = @$this->invokeLDAPMethod('read', $dn, $filter, [$attribute]); if (!$this->ldap->isResource($rr)) { if ($attribute !== '') { //do not throw this message on userExists check, irritates @@ -282,18 +277,18 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) { //in case an error occurs , e.g. object does not exist return false; } - if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $cr, $rr) === 1)) { + if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $rr) === 1)) { $this->logger->debug('readAttribute: ' . $dn . ' found', ['app' => 'user_ldap']); return true; } - $er = $this->invokeLDAPMethod('firstEntry', $cr, $rr); + $er = $this->invokeLDAPMethod('firstEntry', $rr); if (!$this->ldap->isResource($er)) { //did not match the filter, return false return false; } //LDAP attributes are not case sensitive $result = \OCP\Util::mb_array_change_key_case( - $this->invokeLDAPMethod('getAttributes', $cr, $er), MB_CASE_LOWER, 'UTF-8'); + $this->invokeLDAPMethod('getAttributes', $er), MB_CASE_LOWER, 'UTF-8'); return $result; } @@ -374,10 +369,10 @@ public function setPassword($userDN, $password) { } try { // try PASSWD extended operation first - return @$this->invokeLDAPMethod('exopPasswd', $cr, $userDN, '', $password) || - @$this->invokeLDAPMethod('modReplace', $cr, $userDN, $password); + return @$this->invokeLDAPMethod('exopPasswd', $userDN, '', $password) || + @$this->invokeLDAPMethod('modReplace', $userDN, $password); } catch (ConstraintViolationException $e) { - throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), $e->getCode()); + throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), (int)$e->getCode()); } } @@ -445,7 +440,7 @@ public function getDomainDNFromDN($dn) { * @return string|false LDAP DN on success, otherwise false */ public function groupname2dn($name) { - return $this->groupMapper->getDNByName($name); + return $this->getGroupMapper()->getDNByName($name); } /** @@ -455,7 +450,7 @@ public function groupname2dn($name) { * @return string|false with the LDAP DN on success, otherwise false */ public function username2dn($name) { - $fdn = $this->userMapper->getDNByName($name); + $fdn = $this->getUserMapper()->getDNByName($name); //Check whether the DN belongs to the Base, to avoid issues on multi- //server setups @@ -671,18 +666,12 @@ private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array { $sndAttribute = $this->connection->ldapUserDisplayName2; } else { $nameAttribute = $this->connection->ldapGroupDisplayName; + $sndAttribute = null; } $nextcloudNames = []; foreach ($ldapObjects as $ldapObject) { - $nameByLDAP = null; - if (isset($ldapObject[$nameAttribute]) - && is_array($ldapObject[$nameAttribute]) - && isset($ldapObject[$nameAttribute][0]) - ) { - // might be set, but not necessarily. if so, we use it. - $nameByLDAP = $ldapObject[$nameAttribute][0]; - } + $nameByLDAP = $ldapObject[$nameAttribute][0] ?? null; $ncName = $this->dn2ocname($ldapObject['dn'][0], $nameByLDAP, $isUsers); if ($ncName) { @@ -694,8 +683,7 @@ private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array { if (is_null($nameByLDAP)) { continue; } - $sndName = isset($ldapObject[$sndAttribute][0]) - ? $ldapObject[$sndAttribute][0] : ''; + $sndName = $ldapObject[$sndAttribute][0] ?? ''; $this->cacheUserDisplayName($ncName, $nameByLDAP, $sndName); } elseif ($nameByLDAP !== null) { $this->cacheGroupDisplayName($ncName, $nameByLDAP); @@ -711,7 +699,7 @@ private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array { * @param string $ncname * @throws \Exception */ - public function updateUserState($ncname) { + public function updateUserState($ncname): void { $user = $this->userManager->get($ncname); if ($user instanceof OfflineUser) { $user->unmark(); @@ -724,17 +712,15 @@ public function updateUserState($ncname) { * @param string $ocName the internal Nextcloud username * @param string|false $home the home directory path */ - public function cacheUserHome($ocName, $home) { + public function cacheUserHome(string $ocName, $home): void { $cacheKey = 'getHome' . $ocName; $this->connection->writeToCache($cacheKey, $home); } /** * caches a user as existing - * - * @param string $ocName the internal Nextcloud username */ - public function cacheUserExists($ocName) { + public function cacheUserExists(string $ocName): void { $this->connection->writeToCache('userExists' . $ocName, true); } @@ -753,7 +739,7 @@ public function cacheGroupExists(string $gid): void { * @param string $displayName2 the second display name * @throws \Exception */ - public function cacheUserDisplayName($ocName, $displayName, $displayName2 = '') { + public function cacheUserDisplayName(string $ocName, string $displayName, string $displayName2 = ''): void { $user = $this->userManager->get($ocName); if ($user === null) { return; @@ -806,7 +792,7 @@ private function _createAltInternalOwnCloudNameForUsers(string $name) { */ private function _createAltInternalOwnCloudNameForGroups(string $name) { $usedNames = $this->getGroupMapper()->getNamesBySearch($name, "", '_%'); - if (!$usedNames || count($usedNames) === 0) { + if (count($usedNames) === 0) { $lastNo = 1; //will become name_2 } else { natsort($usedNames); @@ -891,7 +877,7 @@ public function fetchListOfUsers(string $filter, array $attr, int $limit = null, $listOfDNs[] = $entry['dn'][0]; return $listOfDNs; }, []); - $idsByDn = $this->userMapper->getListOfIdsByDn($listOfDNs); + $idsByDn = $this->getUserMapper()->getListOfIdsByDn($listOfDNs); $recordsToUpdate = array_filter($ldapRecords, function ($record) use ($isBackgroundJobModeAjax, $idsByDn) { $newlyMapped = false; $uid = $idsByDn[$record['dn'][0]] ?? null; @@ -913,10 +899,9 @@ public function fetchListOfUsers(string $filter, array $attr, int $limit = null, * user object and requests it to process the freshly fetched attributes and * and their values * - * @param array $ldapRecords * @throws \Exception */ - public function batchApplyUserAttributes(array $ldapRecords) { + public function batchApplyUserAttributes(array $ldapRecords): void { $displayNameAttribute = strtolower((string)$this->connection->ldapUserDisplayName); foreach ($ldapRecords as $userRecord) { if (!isset($userRecord[$displayNameAttribute])) { @@ -941,20 +926,16 @@ public function batchApplyUserAttributes(array $ldapRecords) { } /** - * @param string $filter - * @param string|string[] $attr - * @param int $limit - * @param int $offset * @return array[] */ - public function fetchListOfGroups($filter, $attr, $limit = null, $offset = null) { + public function fetchListOfGroups(string $filter, array $attr, int $limit = null, int $offset = null): array { $groupRecords = $this->searchGroups($filter, $attr, $limit, $offset); $listOfDNs = array_reduce($groupRecords, function ($listOfDNs, $entry) { $listOfDNs[] = $entry['dn'][0]; return $listOfDNs; }, []); - $idsByDn = $this->groupMapper->getListOfIdsByDn($listOfDNs); + $idsByDn = $this->getGroupMapper()->getListOfIdsByDn($listOfDNs); array_walk($groupRecords, function (array $record) use ($idsByDn) { $newlyMapped = false; @@ -994,17 +975,14 @@ public function searchUsers(string $filter, array $attr = null, int $limit = nul } /** - * @param string $filter - * @param string|string[] $attr - * @param int $limit - * @param int $offset + * @param string[] $attr * @return false|int * @throws ServerNotAvailableException */ - public function countUsers($filter, $attr = ['dn'], $limit = null, $offset = null) { + public function countUsers(string $filter, array $attr = ['dn'], int $limit = null, int $offset = null) { $result = false; foreach ($this->connection->ldapBaseUsers as $base) { - $count = $this->count($filter, [$base], $attr, $limit, $offset); + $count = $this->count($filter, [$base], $attr, $limit ?? 0, $offset ?? 0); $result = is_int($count) ? (int)$result + $count : $result; } return $result; @@ -1013,16 +991,12 @@ public function countUsers($filter, $attr = ['dn'], $limit = null, $offset = nul /** * executes an LDAP search, optimized for Groups * - * @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 + * @param ?string[] $attr optional, when certain attributes shall be filtered out * * Executes an LDAP search * @throws ServerNotAvailableException */ - public function searchGroups($filter, $attr = null, $limit = null, $offset = null) { + public function searchGroups(string $filter, array $attr = null, int $limit = null, int $offset = null): array { $result = []; foreach ($this->connection->ldapBaseGroups as $base) { $result = array_merge($result, $this->search($filter, $base, $attr, $limit, $offset)); @@ -1033,17 +1007,13 @@ public function searchGroups($filter, $attr = null, $limit = null, $offset = nul /** * returns the number of available groups * - * @param string $filter the LDAP search filter - * @param string[] $attr optional - * @param int|null $limit - * @param int|null $offset * @return int|bool * @throws ServerNotAvailableException */ - public function countGroups($filter, $attr = ['dn'], $limit = null, $offset = null) { + public function countGroups(string $filter, array $attr = ['dn'], int $limit = null, int $offset = null) { $result = false; foreach ($this->connection->ldapBaseGroups as $base) { - $count = $this->count($filter, [$base], $attr, $limit, $offset); + $count = $this->count($filter, [$base], $attr, $limit ?? 0, $offset ?? 0); $result = is_int($count) ? (int)$result + $count : $result; } return $result; @@ -1052,15 +1022,13 @@ public function countGroups($filter, $attr = ['dn'], $limit = null, $offset = nu /** * returns the number of available objects on the base DN * - * @param int|null $limit - * @param int|null $offset * @return int|bool * @throws ServerNotAvailableException */ - public function countObjects($limit = null, $offset = null) { + public function countObjects(int $limit = null, int $offset = null) { $result = false; foreach ($this->connection->ldapBase as $base) { - $count = $this->count('objectclass=*', [$base], ['dn'], $limit, $offset); + $count = $this->count('objectclass=*', [$base], ['dn'], $limit ?? 0, $offset ?? 0); $result = is_int($count) ? (int)$result + $count : $result; } return $result; @@ -1073,26 +1041,23 @@ public function countObjects($limit = null, $offset = null) { */ /** + * @param mixed[] $arguments * @return mixed * @throws \OC\ServerNotAvailableException */ - private function invokeLDAPMethod() { - $arguments = func_get_args(); - $command = array_shift($arguments); - $cr = array_shift($arguments); + private function invokeLDAPMethod(string $command, ...$arguments) { + if ($command == 'controlPagedResultResponse') { + // php no longer supports call-time pass-by-reference + // thus cannot support controlPagedResultResponse as the third argument + // is a reference + throw new \InvalidArgumentException('Invoker does not support controlPagedResultResponse, call LDAP Wrapper directly instead.'); + } if (!method_exists($this->ldap, $command)) { return null; } - array_unshift($arguments, $cr); - // php no longer supports call-time pass-by-reference - // thus cannot support controlPagedResultResponse as the third argument - // is a reference + array_unshift($arguments, $this->connection->getConnectionResource()); $doMethod = function () use ($command, &$arguments) { - if ($command == 'controlPagedResultResponse') { - throw new \InvalidArgumentException('Invoker does not support controlPagedResultResponse, call LDAP Wrapper directly instead.'); - } else { - return call_user_func_array([$this->ldap, $command], $arguments); - } + return call_user_func_array([$this->ldap, $command], $arguments); }; try { $ret = $doMethod(); @@ -1153,8 +1118,7 @@ private function executeSearch( return false; } - $sr = $this->invokeLDAPMethod('search', $cr, $base, $filter, $attr); - // cannot use $cr anymore, might have changed in the previous call! + $sr = $this->invokeLDAPMethod('search', $base, $filter, $attr); $error = $this->ldap->errno($this->connection->getConnectionResource()); if (!$this->ldap->isResource($sr) || $error !== 0) { $this->logger->error('Attempt for Paging? ' . print_r($pagedSearchOK, true), ['app' => 'user_ldap']); @@ -1183,7 +1147,7 @@ private function processPagedSearchStatus( bool $pagedSearchOK, bool $skipHandling ): bool { - $cookie = null; + $cookie = ''; if ($pagedSearchOK) { $cr = $this->connection->getConnectionResource(); if ($this->ldap->controlPagedResultResponse($cr, $sr, $cookie)) { @@ -1201,7 +1165,7 @@ private function processPagedSearchStatus( $this->pagedSearchedSuccessful = true; } } else { - if (!is_null($limit) && (int)$this->connection->ldapPagingSize !== 0) { + if ((int)$this->connection->ldapPagingSize !== 0) { $this->logger->debug( 'Paged search was not available', ['app' => 'user_ldap'] @@ -1221,10 +1185,10 @@ private function processPagedSearchStatus( * * @param string $filter the LDAP filter for the search * @param array $bases an array containing the LDAP subtree(s) that shall be searched - * @param string|string[] $attr optional, array, one or more attributes that shall be + * @param ?string[] $attr optional, array, one or more attributes that shall be * retrieved. Results will according to the order in the array. - * @param int $limit optional, maximum results to be counted - * @param int $offset optional, a starting point + * @param int $limit maximum results to be counted, 0 means no limit + * @param int $offset a starting point, defaults to 0 * @param bool $skipHandling indicates whether the pages search operation is * completed * @return int|false Integer or false if the search could not be initialized @@ -1233,9 +1197,9 @@ private function processPagedSearchStatus( private function count( string $filter, array $bases, - $attr = null, - ?int $limit = null, - ?int $offset = null, + array $attr = null, + int $limit = 0, + int $offset = 0, bool $skipHandling = false ) { $this->logger->debug('Count filter: {filter}', [ @@ -1243,12 +1207,8 @@ private function count( 'filter' => $filter ]); - if (!is_null($attr) && !is_array($attr)) { - $attr = [mb_strtolower($attr, 'UTF-8')]; - } - $limitPerPage = (int)$this->connection->ldapPagingSize; - if (!is_null($limit) && $limit < $limitPerPage && $limit > 0) { + if ($limit < $limitPerPage && $limit > 0) { $limitPerPage = $limit; } @@ -1277,7 +1237,7 @@ private function count( * Continue now depends on $hasMorePages value */ $continue = $pagedSearchOK && $hasMorePages; - } while ($continue && (is_null($limit) || $limit <= 0 || $limit > $counter)); + } while ($continue && ($limit <= 0 || $limit > $counter)); } return $counter; @@ -1289,7 +1249,7 @@ private function count( * @throws ServerNotAvailableException */ private function countEntriesInSearchResults($sr): int { - return (int)$this->invokeLDAPMethod('countEntries', $this->connection->getConnectionResource(), $sr); + return (int)$this->invokeLDAPMethod('countEntries', $sr); } /** @@ -1310,10 +1270,6 @@ public function search( $limitPerPage = $limit; } - if (!is_null($attr) && !is_array($attr)) { - $attr = [mb_strtolower($attr, 'UTF-8')]; - } - /* ++ Fixing RHDS searches with pages with zero results ++ * As we can have pages with zero results and/or pages with less * than $limit results but with a still valid server 'cookie', @@ -1321,6 +1277,7 @@ public function search( * $findings['count'] < $limit */ $findings = []; + $offset = $offset ?? 0; $savedoffset = $offset; $iFoundItems = 0; @@ -1330,7 +1287,6 @@ public function search( return []; } [$sr, $pagedSearchOK] = $search; - $cr = $this->connection->getConnectionResource(); if ($skipHandling) { //i.e. result do not need to be fetched, we just need the cookie @@ -1340,7 +1296,7 @@ public function search( return []; } - $findings = array_merge($findings, $this->invokeLDAPMethod('getEntries', $cr, $sr)); + $findings = array_merge($findings, $this->invokeLDAPMethod('getEntries', $sr)); $iFoundItems = max($iFoundItems, $findings['count']); unset($findings['count']); @@ -1351,12 +1307,6 @@ public function search( // resetting offset $offset = $savedoffset; - // if we're here, probably no connection resource is returned. - // to make Nextcloud behave nicely, we simply give back an empty array. - if (is_null($findings)) { - return []; - } - if (!is_null($attr)) { $selection = []; $i = 0; @@ -1397,7 +1347,7 @@ public function search( && !is_null($limit) ) ) { - $findings = array_slice($findings, (int)$offset, $limit); + $findings = array_slice($findings, $offset, $limit); } return $findings; } @@ -1517,7 +1467,7 @@ private function combineFilter(array $filters, string $operator): string { * @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); @@ -1529,7 +1479,7 @@ public function getFilterPartForUserSearch($search) { * @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); @@ -1621,10 +1571,8 @@ private function prepareSearchTerm(string $term): string { /** * 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 . '=*' @@ -1695,7 +1643,7 @@ public function getUserDnByUuid($uuid) { $filter = $uuidAttr . '=' . $uuid; $result = $this->searchUsers($filter, ['dn'], 2); - if (is_array($result) && isset($result[0]) && isset($result[0]['dn']) && count($result) === 1) { + if (isset($result[0]['dn']) && count($result) === 1) { // we put the count into account to make sure that this is // really unique return $result[0]['dn'][0]; @@ -1768,13 +1716,11 @@ private function detectUuidAttribute(string $dn, bool $isUser = true, bool $forc } /** - * @param string $dn - * @param bool $isUser * @param array|null $ldapRecord * @return false|string * @throws ServerNotAvailableException */ - public function getUUID($dn, $isUser = true, $ldapRecord = null) { + public function getUUID(string $dn, bool $isUser = true, array $ldapRecord = null) { if ($isUser) { $uuidAttr = 'ldapUuidUserAttribute'; $uuidOverride = $this->connection->ldapExpertUUIDUserAttr; @@ -1794,7 +1740,7 @@ public function getUUID($dn, $isUser = true, $ldapRecord = null) { ? $ldapRecord[$this->connection->$uuidAttr] : $this->readAttribute($dn, $this->connection->$uuidAttr); } - if (is_array($uuid) && isset($uuid[0]) && !empty($uuid[0])) { + if (is_array($uuid) && !empty($uuid[0])) { $uuid = $uuid[0]; } } @@ -1835,14 +1781,8 @@ private function convertObjectGUID2Str(string $oguid): string { * to every two hax figures. * * If an invalid string is passed, it will be returned without change. - * - * @param string $guid - * @return string */ - public function formatGuid2ForFilterUser($guid) { - if (!is_string($guid)) { - throw new \InvalidArgumentException('String expected'); - } + public function formatGuid2ForFilterUser(string $guid): string { $blocks = explode('-', $guid); if (count($blocks) !== 5) { /* @@ -1943,11 +1883,9 @@ public function convertSID2Str($sid) { /** * checks if the given DN is part of the given base DN(s) * - * @param string $dn the DN * @param string[] $bases array containing the allowed base DN or DNs - * @return bool */ - public function isDNPartOfBase($dn, $bases) { + public function isDNPartOfBase(string $dn, array $bases): bool { $belongsToBase = false; $bases = $this->helper->sanitizeDN($bases); @@ -1972,8 +1910,7 @@ private function abandonPagedSearch(): void { if ($this->lastCookie === '') { return; } - $cr = $this->connection->getConnectionResource(); - $this->invokeLDAPMethod('controlPagedResult', $cr, 0, false); + $this->invokeLDAPMethod('controlPagedResult', 0, false); $this->getPagedSearchResultState(); $this->lastCookie = ''; } @@ -2060,7 +1997,7 @@ private function initPagedSearch( $this->abandonPagedSearch(); } $pagedSearchOK = true === $this->invokeLDAPMethod( - 'controlPagedResult', $this->connection->getConnectionResource(), $limit, false + 'controlPagedResult', $limit, false ); if ($pagedSearchOK) { $this->logger->debug('Ready for a paged search', ['app' => 'user_ldap']); @@ -2071,7 +2008,7 @@ private function initPagedSearch( * So we added "&& !empty($this->lastCookie)" to this test to ignore pagination * if we don't have a previous paged search. */ - } elseif ($limit === 0 && !empty($this->lastCookie)) { + } elseif (!empty($this->lastCookie)) { // a search without limit was requested. However, if we do use // Paged Search once, we always must do it. This requires us to // initialize it with the configured page size. @@ -2080,7 +2017,6 @@ private function initPagedSearch( // be returned. $pageSize = (int)$this->connection->ldapPagingSize > 0 ? (int)$this->connection->ldapPagingSize : 500; $pagedSearchOK = $this->invokeLDAPMethod('controlPagedResult', - $this->connection->getConnectionResource(), $pageSize, false); } diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php index ab5aa23f98d88..9ffb2a797814b 100644 --- a/apps/user_ldap/lib/Configuration.php +++ b/apps/user_ldap/lib/Configuration.php @@ -47,7 +47,13 @@ class Configuration { public const LDAP_SERVER_FEATURE_AVAILABLE = 'available'; public const LDAP_SERVER_FEATURE_UNAVAILABLE = 'unavailable'; - protected $configPrefix = null; + /** + * @var string + */ + protected $configPrefix; + /** + * @var bool + */ protected $configRead = false; /** * @var string[] pre-filled with one reference key so that at least one entry is written on save request and @@ -55,7 +61,9 @@ class Configuration { */ protected $unsavedChanges = ['ldapConfigurationActive' => 'ldapConfigurationActive']; - //settings + /** + * @var array settings + */ protected $config = [ 'ldapHost' => null, 'ldapPort' => null, @@ -115,11 +123,7 @@ class Configuration { 'ldapMatchingRuleInChainState' => self::LDAP_SERVER_FEATURE_UNKNOWN, ]; - /** - * @param string $configPrefix - * @param bool $autoRead - */ - public function __construct($configPrefix, $autoRead = true) { + public function __construct(string $configPrefix, bool $autoRead = true) { $this->configPrefix = $configPrefix; if ($autoRead) { $this->readConfiguration(); @@ -145,10 +149,7 @@ public function __set($name, $value) { $this->setConfiguration([$name => $value]); } - /** - * @return array - */ - public function getConfiguration() { + public function getConfiguration(): array { return $this->config; } @@ -159,13 +160,8 @@ public function getConfiguration() { * @param array $config array that holds the config parameters in an associated * array * @param array &$applied optional; array where the set fields will be given to - * @return false|null */ - public function setConfiguration($config, &$applied = null) { - if (!is_array($config)) { - return false; - } - + public function setConfiguration(array $config, array &$applied = null): void { $cta = $this->getConfigTranslationArray(); foreach ($config as $inputKey => $val) { if (strpos($inputKey, '_') !== false && array_key_exists($inputKey, $cta)) { @@ -207,11 +203,10 @@ public function setConfiguration($config, &$applied = null) { } $this->unsavedChanges[$key] = $key; } - return null; } - public function readConfiguration() { - if (!$this->configRead && !is_null($this->configPrefix)) { + public function readConfiguration(): void { + if (!$this->configRead) { $cta = array_flip($this->getConfigTranslationArray()); foreach ($this->config as $key => $val) { if (!isset($cta[$key])) { @@ -260,7 +255,7 @@ public function readConfiguration() { /** * saves the current config changes in the database */ - public function saveConfiguration() { + public function saveConfiguration(): void { $cta = array_flip($this->getConfigTranslationArray()); foreach ($this->unsavedChanges as $key) { $value = $this->config[$key]; @@ -293,7 +288,7 @@ public function saveConfiguration() { } $this->saveValue($cta[$key], $value); } - $this->saveValue('_lastChange', time()); + $this->saveValue('_lastChange', (string)time()); $this->unsavedChanges = []; } @@ -318,7 +313,7 @@ protected function getMultiLine($varName) { * @param string $varName name of config-key * @param array|string $value to set */ - protected function setMultiLine($varName, $value) { + protected function setMultiLine(string $varName, $value): void { if (empty($value)) { $value = ''; } elseif (!is_array($value)) { @@ -349,36 +344,20 @@ protected function setMultiLine($varName, $value) { $this->setRawValue($varName, $finalValue); } - /** - * @param string $varName - * @return string - */ - protected function getPwd($varName) { + protected function getPwd(string $varName): string { return base64_decode($this->getValue($varName)); } - /** - * @param string $varName - * @return string - */ - protected function getLcValue($varName) { + protected function getLcValue(string $varName): string { return mb_strtolower($this->getValue($varName), 'UTF-8'); } - /** - * @param string $varName - * @return string - */ - protected function getSystemValue($varName) { + protected function getSystemValue(string $varName): string { //FIXME: if another system value is added, softcode the default value return \OC::$server->getConfig()->getSystemValue($varName, false); } - /** - * @param string $varName - * @return string - */ - protected function getValue($varName) { + protected function getValue(string $varName): string { static $defaults; if (is_null($defaults)) { $defaults = $this->getDefaults(); @@ -394,7 +373,7 @@ protected function getValue($varName) { * @param string $varName name of config key * @param mixed $value to set */ - protected function setValue($varName, $value) { + protected function setValue(string $varName, $value): void { if (is_string($value)) { $value = trim($value); } @@ -407,16 +386,11 @@ protected function setValue($varName, $value) { * @param string $varName name of config key * @param mixed $value to set */ - protected function setRawValue($varName, $value) { + protected function setRawValue(string $varName, $value): void { $this->config[$varName] = $value; } - /** - * @param string $varName - * @param string $value - * @return bool - */ - protected function saveValue($varName, $value) { + protected function saveValue(string $varName, string $value): bool { \OC::$server->getConfig()->setAppValue( 'user_ldap', $this->configPrefix.$varName, @@ -429,7 +403,7 @@ protected function saveValue($varName, $value) { * @return array an associative array with the default values. Keys are correspond * to config-value entries in the database table */ - public function getDefaults() { + public function getDefaults(): array { return [ 'ldap_host' => '', 'ldap_port' => '', @@ -492,7 +466,7 @@ public function getDefaults() { /** * @return array that maps internal variable names to database fields */ - public function getConfigTranslationArray() { + public function getConfigTranslationArray(): array { //TODO: merge them into one representation static $array = [ 'ldap_host' => 'ldapHost', @@ -554,18 +528,16 @@ public function getConfigTranslationArray() { } /** - * @param string $rule - * @return array * @throws \RuntimeException */ - public function resolveRule($rule) { + public function resolveRule(string $rule): array { if ($rule === 'avatar') { return $this->getAvatarAttributes(); } throw new \RuntimeException('Invalid rule'); } - public function getAvatarAttributes() { + public function getAvatarAttributes(): array { $value = $this->ldapUserAvatarRule ?: self::AVATAR_PREFIX_DEFAULT; $defaultAttributes = ['jpegphoto', 'thumbnailphoto']; diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 3cd6a340a5655..565fb415e5863 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -78,10 +78,25 @@ class Connection extends LDAPUtility { * @var resource|\LDAP\Connection|null */ 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; /** @@ -94,16 +109,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 */ @@ -111,16 +137,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(); @@ -304,9 +328,9 @@ private function readConfiguration($force = false) { * set LDAP configuration with values delivered by an array, not read from configuration * @param array $config array that holds the config parameters in an associated array * @param array &$setParameters optional; array where the set fields will be given to - * @return boolean true if config validates, false otherwise. Check with $setParameters for detailed success on single parameters + * @return bool true if config validates, false otherwise. Check with $setParameters for detailed success on single parameters */ - public function setConfiguration($config, &$setParameters = null) { + public function setConfiguration($config, &$setParameters = null): bool { if (is_null($setParameters)) { $setParameters = []; } diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index f757a8b5e12f5..766b77bf52199 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -863,20 +863,18 @@ private function getGroupsByMember(string $dn, array &$seen = null): array { $groups = $this->access->fetchListOfGroups($filter, [strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']); - if (is_array($groups)) { - $fetcher = function ($dn) use (&$seen) { - if (is_array($dn) && isset($dn['dn'][0])) { - $dn = $dn['dn'][0]; - } - return $this->getGroupsByMember($dn, $seen); - }; - - if (empty($dn)) { - $dn = ""; + $fetcher = function ($dn) use (&$seen) { + if (is_array($dn) && isset($dn['dn'][0])) { + $dn = $dn['dn'][0]; } + return $this->getGroupsByMember($dn, $seen); + }; - $allGroups = $this->walkNestedGroups($dn, $fetcher, $groups, $seen); + if (empty($dn)) { + $dn = ""; } + + $allGroups = $this->walkNestedGroups($dn, $fetcher, $groups, $seen); $visibleGroups = $this->filterValidGroups($allGroups); return array_intersect_key($allGroups, $visibleGroups); } @@ -1349,7 +1347,7 @@ public function getDisplayName(string $gid): string { $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 3f600a40cc0e5..e72d85ac2b938 100644 --- a/apps/user_ldap/lib/ILDAPWrapper.php +++ b/apps/user_ldap/lib/ILDAPWrapper.php @@ -66,7 +66,7 @@ public function controlPagedResult($link, $pageSize, $isCritical); * Retrieve the LDAP pagination cookie * @param resource|\LDAP\Connection $link LDAP link resource * @param resource|\LDAP\Result $result LDAP result resource - * @param string $cookie structure sent by LDAP server + * @param string &$cookie structure sent by LDAP server * @return bool true on success, false otherwise * * Corresponds to ldap_control_paged_result_response @@ -178,8 +178,8 @@ public function modReplace($link, $userDN, $password); /** * Sets the value of the specified option to be $value * @param resource|\LDAP\Connection $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/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index 16973f76ff4d8..1bf5fbc5b78b9 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -280,12 +280,9 @@ public function getListOfIdsByDn(array $fdns): array { /** * Searches mapped names by the giving string in the name column * - * @param string $search - * @param string $prefixMatch - * @param string $postfixMatch * @return string[] */ - public function getNamesBySearch($search, $prefixMatch = "", $postfixMatch = "") { + public function getNamesBySearch(string $search, string $prefixMatch = "", string $postfixMatch = ""): array { $statement = $this->dbc->prepare(' SELECT `owncloud_name` FROM `' . $this->getTableName() . '` diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index ab1500ff3689b..15894ce04b775 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -266,8 +266,8 @@ public function getUsername() { /** * 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) { @@ -278,8 +278,7 @@ public function getHomePath($valueFromLDAP = null) { && 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 16f79d7819d9f..c2902a677669a 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -112,7 +112,7 @@ protected function setUp(): void { 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([ @@ -494,7 +494,7 @@ public function testSetPasswordWithRejectedChange() { ->willReturn(true); $connection = $this->createMock(LDAP::class); $this->connection - ->expects($this->once()) + ->expects($this->any()) ->method('getConnectionResource') ->willReturn($connection); $this->ldap @@ -518,7 +518,7 @@ public function testSetPassword() { ->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 cb637dcc108a0..f8327c0776c33 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -104,14 +104,12 @@ private function getAccessMock() { $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 @@ private function enableGroups(Access $access) { ->willReturnCallback(function ($name) { if ($name === 'ldapDynamicGroupMemberURL') { return ''; + } elseif ($name === 'ldapBaseGroups') { + return []; } return 1; }); @@ -953,6 +953,8 @@ public function testGetGroupsByMember(bool $nestedGroups) { return 'member'; case 'ldapGroupFilter': return $groupFilter; + case 'ldapBaseGroups': + return []; } return 1; }); @@ -1321,16 +1323,16 @@ public function testGroupMembers($groupDN, $expectedMembers, $groupsInfo = null) }); $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 @@ public function testGetDisplayName(string $expected, $ldapResult) { 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 cf3daa9f2c512..e86eb5e9e2ea7 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -1016,6 +1016,10 @@ public function testGetHomePathConfiguredNotAvailableAllowed() { ->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 @@ public function testGetHomePathConfiguredNotAvailableNotAllowed() { ->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 3142a256c9a13..b00c93e79f0c8 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -815,13 +815,15 @@ public function testGetHomeWithPlugin() { 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 ae25aad44ab8e..5382a0c7f6f59 100644 --- a/apps/user_ldap/tests/WizardTest.php +++ b/apps/user_ldap/tests/WizardTest.php @@ -72,7 +72,7 @@ private function getWizardAndMocks() { /** @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 */