diff options
author | blizzz <blizzz@arthur-schiwon.de> | 2020-04-24 12:27:27 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-24 12:27:27 +0200 |
commit | 212138daa181cf1fbcb72b3129f8d3d5f94cf3b2 (patch) | |
tree | 37afc994d574ff437f2b90be3c909b36bc325bca /apps | |
parent | 84a35361599640d838815a4127eab58e49f052fc (diff) | |
parent | 4babdc082b988485608682a6a0bf3cccfd42fe8b (diff) | |
download | nextcloud-server-212138daa181cf1fbcb72b3129f8d3d5f94cf3b2.tar.gz nextcloud-server-212138daa181cf1fbcb72b3129f8d3d5f94cf3b2.zip |
Merge pull request #19919 from nextcloud/enh/noid/ldpa_group_perf
LDAP Group Backend optimizations
Diffstat (limited to 'apps')
-rw-r--r-- | apps/user_ldap/lib/Access.php | 188 | ||||
-rw-r--r-- | apps/user_ldap/lib/Group_LDAP.php | 114 | ||||
-rw-r--r-- | apps/user_ldap/lib/Group_Proxy.php | 24 | ||||
-rw-r--r-- | apps/user_ldap/lib/Helper.php | 43 | ||||
-rw-r--r-- | apps/user_ldap/lib/Mapping/AbstractMapping.php | 84 | ||||
-rw-r--r-- | apps/user_ldap/lib/Mapping/GroupMapping.php | 5 | ||||
-rw-r--r-- | apps/user_ldap/lib/Mapping/UserMapping.php | 5 | ||||
-rw-r--r-- | apps/user_ldap/lib/Proxy.php | 36 | ||||
-rw-r--r-- | apps/user_ldap/lib/User_Proxy.php | 33 | ||||
-rw-r--r-- | apps/user_ldap/tests/Group_LDAPTest.php | 102 |
10 files changed, 428 insertions, 206 deletions
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 1b3a97cef95..251c0eeeaa7 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -61,6 +61,7 @@ use OCP\IUserManager; /** * Class Access + * * @package OCA\User_LDAP */ class Access extends LDAPUtility { @@ -75,6 +76,7 @@ class Access extends LDAPUtility { /** * protected $cookies = []; + * * @var AbstractMapping $userMapper */ protected $userMapper; @@ -114,6 +116,7 @@ class Access extends LDAPUtility { /** * sets the User Mapper + * * @param AbstractMapping $mapper */ public function setUserMapper(AbstractMapping $mapper) { @@ -122,8 +125,9 @@ class Access extends LDAPUtility { /** * returns the User Mapper - * @throws \Exception + * * @return AbstractMapping + * @throws \Exception */ public function getUserMapper() { if (is_null($this->userMapper)) { @@ -134,6 +138,7 @@ class Access extends LDAPUtility { /** * sets the Group Mapper + * * @param AbstractMapping $mapper */ public function setGroupMapper(AbstractMapping $mapper) { @@ -142,8 +147,9 @@ class Access extends LDAPUtility { /** * returns the Group Mapper - * @throws \Exception + * * @return AbstractMapping + * @throws \Exception */ public function getGroupMapper() { if (is_null($this->groupMapper)) { @@ -161,6 +167,7 @@ class Access extends LDAPUtility { /** * returns the Connection instance + * * @return \OCA\User_LDAP\Connection */ public function getConnection() { @@ -226,7 +233,7 @@ class Access extends LDAPUtility { $result = $this->extractRangeData($result, $attr); if (!empty($result)) { $normalizedResult = $this->extractAttributeValuesFromResult( - [ $attr => $result['values'] ], + [$attr => $result['values']], $attr ); $values = array_merge($values, $normalizedResult); @@ -236,14 +243,14 @@ class Access extends LDAPUtility { // no more results left return $values; } else { - $low = $result['rangeHigh'] + 1; + $low = $result['rangeHigh'] + 1; $attrToRead = $result['attributeName'] . ';range=' . $low . '-*'; $isRangeRequest = true; } } } while ($isRangeRequest); - \OCP\Util::writeLog('user_ldap', 'Requested attribute '.$attr.' not found for '.$dn, ILogger::DEBUG); + \OCP\Util::writeLog('user_ldap', 'Requested attribute ' . $attr . ' not found for ' . $dn, ILogger::DEBUG); return false; } @@ -300,7 +307,7 @@ class Access extends LDAPUtility { $values = []; if (isset($result[$attribute]) && $result[$attribute]['count'] > 0) { $lowercaseAttribute = strtolower($attribute); - for ($i=0;$i<$result[$attribute]['count'];$i++) { + for ($i = 0; $i < $result[$attribute]['count']; $i++) { if ($this->resemblesDN($attribute)) { $values[] = $this->helper->sanitizeDN($result[$attribute][$i]); } elseif ($lowercaseAttribute === 'objectguid' || $lowercaseAttribute === 'guid') { @@ -365,14 +372,15 @@ class Access extends LDAPUtility { try { // try PASSWD extended operation first return @$this->invokeLDAPMethod('exopPasswd', $cr, $userDN, '', $password) || - @$this->invokeLDAPMethod('modReplace', $cr, $userDN, $password); + @$this->invokeLDAPMethod('modReplace', $cr, $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(), $e->getCode()); } } /** * checks whether the given attributes value is probably a DN + * * @param string $attr the attribute in question * @return boolean if so true, otherwise false */ @@ -389,6 +397,7 @@ class Access extends LDAPUtility { /** * checks whether the given string is probably a DN + * * @param string $string * @return boolean */ @@ -403,6 +412,7 @@ class Access extends LDAPUtility { * returns a DN-string that is cleaned from not domain parts, e.g. * cn=foo,cn=bar,dc=foobar,dc=server,dc=org * becomes dc=foobar,dc=server,dc=org + * * @param string $dn * @return string */ @@ -427,6 +437,7 @@ class Access extends LDAPUtility { /** * returns the LDAP DN for the given internal Nextcloud name of the group + * * @param string $name the Nextcloud name in question * @return string|false LDAP DN on success, otherwise false */ @@ -436,6 +447,7 @@ class Access extends LDAPUtility { /** * returns the LDAP DN for the given internal Nextcloud name of the user + * * @param string $name the Nextcloud name in question * @return string|false with the LDAP DN on success, otherwise false */ @@ -471,45 +483,6 @@ class Access extends LDAPUtility { } /** - * accepts an array of group DNs and tests whether they match the user - * filter by doing read operations against the group entries. Returns an - * array of DNs that match the filter. - * - * @param string[] $groupDNs - * @return string[] - * @throws ServerNotAvailableException - */ - public function groupsMatchFilter($groupDNs) { - $validGroupDNs = []; - foreach ($groupDNs as $dn) { - $cacheKey = 'groupsMatchFilter-'.$dn; - $groupMatchFilter = $this->connection->getFromCache($cacheKey); - if (!is_null($groupMatchFilter)) { - if ($groupMatchFilter) { - $validGroupDNs[] = $dn; - } - continue; - } - - // Check the base DN first. If this is not met already, we don't - // need to ask the server at all. - if (!$this->isDNPartOfBase($dn, $this->connection->ldapBaseGroups)) { - $this->connection->writeToCache($cacheKey, false); - continue; - } - - $result = $this->readAttribute($dn, '', $this->connection->ldapGroupFilter); - if (is_array($result)) { - $this->connection->writeToCache($cacheKey, true); - $validGroupDNs[] = $dn; - } else { - $this->connection->writeToCache($cacheKey, false); - } - } - return $validGroupDNs; - } - - /** * returns the internal Nextcloud name for the given LDAP DN of the user, false on DN outside of search DN or failure * * @param string $dn the dn of the user object @@ -567,14 +540,14 @@ class Access extends LDAPUtility { } } else { //If the UUID can't be detected something is foul. - \OCP\Util::writeLog('user_ldap', 'Cannot determine UUID for '.$fdn.'. Skipping.', ILogger::INFO); + \OCP\Util::writeLog('user_ldap', 'Cannot determine UUID for ' . $fdn . '. Skipping.', ILogger::INFO); return false; } if (is_null($ldapName)) { $ldapName = $this->readAttribute($fdn, $nameAttribute, $filter); if (!isset($ldapName[0]) && empty($ldapName[0])) { - \OCP\Util::writeLog('user_ldap', 'No or empty name for '.$fdn.' with filter '.$filter.'.', ILogger::INFO); + \OCP\Util::writeLog('user_ldap', 'No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ILogger::INFO); return false; } $ldapName = $ldapName[0]; @@ -633,7 +606,7 @@ class Access extends LDAPUtility { } //if everything else did not help.. - \OCP\Util::writeLog('user_ldap', 'Could not create unique name for '.$fdn.'.', ILogger::INFO); + \OCP\Util::writeLog('user_ldap', 'Could not create unique name for ' . $fdn . '.', ILogger::INFO); return false; } @@ -643,7 +616,7 @@ class Access extends LDAPUtility { string $name, string $uuid, bool $isUser - ) :bool { + ): bool { if ($mapper->map($fdn, $name, $uuid)) { if ($this->ncUserManager instanceof PublicEmitter && $isUser) { $this->cacheUserExists($name); @@ -691,7 +664,7 @@ class Access extends LDAPUtility { private function ldap2NextcloudNames($ldapObjects, $isUsers) { if ($isUsers) { $nameAttribute = $this->connection->ldapUserDisplayName; - $sndAttribute = $this->connection->ldapUserDisplayName2; + $sndAttribute = $this->connection->ldapUserDisplayName2; } else { $nameAttribute = $this->connection->ldapGroupDisplayName; } @@ -743,27 +716,29 @@ class Access extends LDAPUtility { /** * caches the user display name + * * @param string $ocName the internal Nextcloud username * @param string|false $home the home directory path */ public function cacheUserHome($ocName, $home) { - $cacheKey = 'getHome'.$ocName; + $cacheKey = 'getHome' . $ocName; $this->connection->writeToCache($cacheKey, $home); } /** * caches a user as existing + * * @param string $ocName the internal Nextcloud username */ public function cacheUserExists($ocName) { - $this->connection->writeToCache('userExists'.$ocName, true); + $this->connection->writeToCache('userExists' . $ocName, true); } /** * caches a group as existing */ public function cacheGroupExists(string $gid): void { - $this->connection->writeToCache('groupExists'.$gid, true); + $this->connection->writeToCache('groupExists' . $gid, true); } /** @@ -781,7 +756,7 @@ class Access extends LDAPUtility { } $displayName = $user->composeAndStoreDisplayName($displayName, $displayName2); $cacheKeyTrunk = 'getDisplayName'; - $this->connection->writeToCache($cacheKeyTrunk.$ocName, $displayName); + $this->connection->writeToCache($cacheKeyTrunk . $ocName, $displayName); } public function cacheGroupDisplayName(string $ncName, string $displayName): void { @@ -791,6 +766,7 @@ class Access extends LDAPUtility { /** * creates a unique name for internal Nextcloud use for users. Don't call it directly. + * * @param string $name the display name of the object * @return string|false with with the name to use in Nextcloud or false if unsuccessful * @@ -802,7 +778,7 @@ class Access extends LDAPUtility { //while loop is just a precaution. If a name is not generated within //20 attempts, something else is very wrong. Avoids infinite loop. while ($attempts < 20) { - $altName = $name . '_' . rand(1000,9999); + $altName = $name . '_' . rand(1000, 9999); if (!$this->ncUserManager->userExists($altName)) { return $altName; } @@ -813,6 +789,7 @@ class Access extends LDAPUtility { /** * creates a unique name for internal Nextcloud use for groups. Don't call it directly. + * * @param string $name the display name of the object * @return string|false with with the name to use in Nextcloud or false if unsuccessful. * @@ -832,7 +809,7 @@ class Access extends LDAPUtility { $lastName = array_pop($usedNames); $lastNo = (int)substr($lastName, strrpos($lastName, '_') + 1); } - $altName = $name.'_'. (string)($lastNo+1); + $altName = $name . '_' . (string)($lastNo + 1); unset($usedNames); $attempts = 1; @@ -851,6 +828,7 @@ class Access extends LDAPUtility { /** * creates a unique name for internal Nextcloud use. + * * @param string $name the display name of the object * @param boolean $isUser whether name should be created for a user (true) or a group (false) * @return string|false with with the name to use in Nextcloud or false if unsuccessful @@ -910,9 +888,17 @@ class Access extends LDAPUtility { if (!$forceApplyAttributes) { $isBackgroundJobModeAjax = $this->config ->getAppValue('core', 'backgroundjobs_mode', 'ajax') === 'ajax'; - $recordsToUpdate = array_filter($ldapRecords, function ($record) use ($isBackgroundJobModeAjax) { + $listOfDNs = array_reduce($ldapRecords, function ($listOfDNs, $entry) { + $listOfDNs[] = $entry['dn'][0]; + return $listOfDNs; + }, []); + $idsByDn = $this->userMapper->getListOfIdsByDn($listOfDNs); + $recordsToUpdate = array_filter($ldapRecords, function ($record) use ($isBackgroundJobModeAjax, $idsByDn) { $newlyMapped = false; - $uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record); + $uid = $idsByDn[$record['dn'][0]] ?? null; + if ($uid === null) { + $uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record); + } if (is_string($uid)) { $this->cacheUserExists($uid); } @@ -938,7 +924,7 @@ class Access extends LDAPUtility { // displayName is obligatory continue; } - $ocName = $this->dn2ocname($userRecord['dn'][0], null, true); + $ocName = $this->dn2ocname($userRecord['dn'][0], null, true); if ($ocName === false) { continue; } @@ -949,7 +935,7 @@ class Access extends LDAPUtility { } else { \OC::$server->getLogger()->debug( "The ldap user manager returned null for $ocName", - ['app'=>'user_ldap'] + ['app' => 'user_ldap'] ); } } @@ -964,9 +950,19 @@ class Access extends LDAPUtility { */ public function fetchListOfGroups($filter, $attr, $limit = null, $offset = null) { $groupRecords = $this->searchGroups($filter, $attr, $limit, $offset); - array_walk($groupRecords, function ($record) { + + $listOfDNs = array_reduce($groupRecords, function ($listOfDNs, $entry) { + $listOfDNs[] = $entry['dn'][0]; + return $listOfDNs; + }, []); + $idsByDn = $this->groupMapper->getListOfIdsByDn($listOfDNs); + + array_walk($groupRecords, function ($record) use ($idsByDn) { $newlyMapped = false; - $gid = $this->dn2ocname($record['dn'][0], null, false, $newlyMapped, $record); + $gid = $uidsByDn[$record['dn'][0]] ?? null; + if ($gid === null) { + $gid = $this->dn2ocname($record['dn'][0], null, false, $newlyMapped, $record); + } if (!$newlyMapped && is_string($gid)) { $this->cacheGroupExists($gid); } @@ -1092,6 +1088,7 @@ class Access extends LDAPUtility { /** * Returns the LDAP handler + * * @throws \OC\ServerNotAvailableException */ @@ -1175,7 +1172,7 @@ class Access extends LDAPUtility { // cannot use $cr anymore, might have changed in the previous call! $error = $this->ldap->errno($this->connection->getConnectionResource()); if (!$this->ldap->isResource($sr) || $error !== 0) { - \OCP\Util::writeLog('user_ldap', 'Attempt for Paging? '.print_r($pagedSearchOK, true), ILogger::ERROR); + \OCP\Util::writeLog('user_ldap', 'Attempt for Paging? ' . print_r($pagedSearchOK, true), ILogger::ERROR); return false; } @@ -1222,7 +1219,7 @@ class Access extends LDAPUtility { if (!is_null($limit) && (int)$this->connection->ldapPagingSize !== 0) { \OC::$server->getLogger()->debug( 'Paged search was not available', - [ 'app' => 'user_ldap' ] + ['app' => 'user_ldap'] ); } } @@ -1409,7 +1406,7 @@ class Access extends LDAPUtility { //a) paged search unsuccessful, though attempted //b) no paged search, but limit set if ((!$this->getPagedSearchResultState() - && $pagedSearchOK) + && $pagedSearchOK) || ( !$pagedSearchOK && !is_null($limit) @@ -1454,7 +1451,8 @@ class Access extends LDAPUtility { /** * escapes (user provided) parts for LDAP filter - * @param string $input, the provided value + * + * @param string $input , the provided value * @param bool $allowAsterisk whether in * at the beginning should be preserved * @return string the escaped string */ @@ -1464,13 +1462,14 @@ class Access extends LDAPUtility { $asterisk = '*'; $input = mb_substr($input, 1, null, 'UTF-8'); } - $search = ['*', '\\', '(', ')']; + $search = ['*', '\\', '(', ')']; $replace = ['\\*', '\\\\', '\\(', '\\)']; return $asterisk . str_replace($search, $replace, $input); } /** * combines the input filters with AND + * * @param string[] $filters the filters to connect * @return string the combined filter */ @@ -1480,6 +1479,7 @@ class Access extends LDAPUtility { /** * combines the input filters with OR + * * @param string[] $filters the filters to connect * @return string the combined filter * Combines Filter arguments with OR @@ -1490,24 +1490,26 @@ class Access extends LDAPUtility { /** * combines the input filters with given operator + * * @param string[] $filters the filters to connect * @param string $operator either & or | * @return string the combined filter */ private function combineFilter($filters, $operator) { - $combinedFilter = '('.$operator; + $combinedFilter = '(' . $operator; foreach ($filters as $filter) { if ($filter !== '' && $filter[0] !== '(') { - $filter = '('.$filter.')'; + $filter = '(' . $filter . ')'; } - $combinedFilter.=$filter; + $combinedFilter .= $filter; } - $combinedFilter.=')'; + $combinedFilter .= ')'; return $combinedFilter; } /** * creates a filter part for to perform search for users + * * @param string $search the search term * @return string the final filter part to use in LDAP searches */ @@ -1519,6 +1521,7 @@ class Access extends LDAPUtility { /** * creates a filter part for to perform search for groups + * * @param string $search the search term * @return string the final filter part to use in LDAP searches */ @@ -1531,6 +1534,7 @@ class Access extends LDAPUtility { /** * creates a filter part for searches by splitting up the given search * string into single words + * * @param string $search the search term * @param string[] $searchAttributes needs to have at least two attributes, * otherwise it does not make sense :) @@ -1557,6 +1561,7 @@ class Access extends LDAPUtility { /** * creates a filter part for searches + * * @param string $search the search term * @param string[]|null $searchAttributes * @param string $fallbackAttribute a fallback attribute in case the user @@ -1590,7 +1595,7 @@ class Access extends LDAPUtility { } } if (count($filter) === 1) { - return '('.$filter[0].')'; + return '(' . $filter[0] . ')'; } return $this->combineFilterWithOr($filter); } @@ -1599,6 +1604,7 @@ class Access extends LDAPUtility { * returns the search term depending on whether we are allowed * list users found by ldap with the current input appended by * a * + * * @return string */ private function prepareSearchTerm($term) { @@ -1617,6 +1623,7 @@ class Access extends LDAPUtility { /** * returns the filter used for counting users + * * @return string */ public function getFilterForUserCount() { @@ -1655,8 +1662,8 @@ class Access extends LDAPUtility { */ public function getUserDnByUuid($uuid) { $uuidOverride = $this->connection->ldapExpertUUIDUserAttr; - $filter = $this->connection->ldapUserFilter; - $bases = $this->connection->ldapBaseUsers; + $filter = $this->connection->ldapUserFilter; + $bases = $this->connection->ldapBaseUsers; if ($this->connection->ldapUuidUserAttribute === 'auto' && $uuidOverride === '') { // Sacrebleu! The UUID attribute is unknown :( We need first an @@ -1711,10 +1718,10 @@ class Access extends LDAPUtility { */ private function detectUuidAttribute($dn, $isUser = true, $force = false, array $ldapRecord = null) { if ($isUser) { - $uuidAttr = 'ldapUuidUserAttribute'; + $uuidAttr = 'ldapUuidUserAttribute'; $uuidOverride = $this->connection->ldapExpertUUIDUserAttr; } else { - $uuidAttr = 'ldapUuidGroupAttribute'; + $uuidAttr = 'ldapUuidGroupAttribute'; $uuidOverride = $this->connection->ldapExpertUUIDGroupAttr; } @@ -1771,10 +1778,10 @@ class Access extends LDAPUtility { */ public function getUUID($dn, $isUser = true, $ldapRecord = null) { if ($isUser) { - $uuidAttr = 'ldapUuidUserAttribute'; + $uuidAttr = 'ldapUuidUserAttribute'; $uuidOverride = $this->connection->ldapExpertUUIDUserAttr; } else { - $uuidAttr = 'ldapUuidGroupAttribute'; + $uuidAttr = 'ldapUuidGroupAttribute'; $uuidOverride = $this->connection->ldapExpertUUIDGroupAttr; } @@ -1799,6 +1806,7 @@ class Access extends LDAPUtility { /** * converts a binary ObjectGUID into a string representation + * * @param string $oguid the ObjectGUID in it's binary form as retrieved from AD * @return string * @link http://www.php.net/manual/en/function.ldap-get-values-len.php#73198 @@ -1854,16 +1862,16 @@ class Access extends LDAPUtility { \OC::$server->getLogger()->info( 'Passed string does not resemble a valid GUID. Known UUID ' . '({uuid}) probably does not match UUID configuration.', - [ 'app' => 'user_ldap', 'uuid' => $guid ] + ['app' => 'user_ldap', 'uuid' => $guid] ); return $guid; } - for ($i=0; $i < 3; $i++) { + for ($i = 0; $i < 3; $i++) { $pairs = str_split($blocks[$i], 2); $pairs = array_reverse($pairs); $blocks[$i] = implode('', $pairs); } - for ($i=0; $i < 5; $i++) { + for ($i = 0; $i < 5; $i++) { $pairs = str_split($blocks[$i], 2); $blocks[$i] = '\\' . implode('\\', $pairs); } @@ -1879,7 +1887,7 @@ class Access extends LDAPUtility { */ public function getSID($dn) { $domainDN = $this->getDomainDNFromDN($dn); - $cacheKey = 'getSID-'.$domainDN; + $cacheKey = 'getSID-' . $domainDN; $sid = $this->connection->getFromCache($cacheKey); if (!is_null($sid)) { return $sid; @@ -1898,6 +1906,7 @@ class Access extends LDAPUtility { /** * converts a binary SID into a string representation + * * @param string $sid * @return string */ @@ -1936,6 +1945,7 @@ class Access extends LDAPUtility { /** * 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 @@ -1946,7 +1956,7 @@ class Access extends LDAPUtility { foreach ($bases as $base) { $belongsToBase = true; - if (mb_strripos($dn, $base, 0, 'UTF-8') !== (mb_strlen($dn, 'UTF-8')-mb_strlen($base, 'UTF-8'))) { + if (mb_strripos($dn, $base, 0, 'UTF-8') !== (mb_strlen($dn, 'UTF-8') - mb_strlen($base, 'UTF-8'))) { $belongsToBase = false; } if ($belongsToBase) { @@ -1979,6 +1989,7 @@ class Access extends LDAPUtility { * be reset by other operations. Best, call it immediately after a search(), * searchUsers() or searchGroups() call. count-methods are probably safe as * well. Don't rely on it with any fetchList-method. + * * @return bool */ public function hasMoreResults() { @@ -1993,6 +2004,7 @@ class Access extends LDAPUtility { /** * Check whether the most recent paged search was successful. It flushed the state var. Use it always after a possible paged search. + * * @return boolean|null true on success, null or false otherwise */ public function getPagedSearchResultState() { @@ -2045,10 +2057,10 @@ class Access extends LDAPUtility { $this->abandonPagedSearch(); } $pagedSearchOK = true === $this->invokeLDAPMethod( - 'controlPagedResult', $this->connection->getConnectionResource(), $limit, false - ); + 'controlPagedResult', $this->connection->getConnectionResource(), $limit, false + ); if ($pagedSearchOK) { - \OC::$server->getLogger()->debug('Ready for a paged search',['app' => 'user_ldap']); + \OC::$server->getLogger()->debug('Ready for a paged search', ['app' => 'user_ldap']); } /* ++ Fixing RHDS searches with pages with zero results ++ * We coudn't get paged searches working with our RHDS for login ($limit = 0), diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 85d9e38e03e..f05c8fb8ca4 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -85,6 +85,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * is user in group? + * * @param string $uid uid of the user * @param string $gid gid of the group * @return bool @@ -95,7 +96,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if (!$this->enabled) { return false; } - $cacheKey = 'inGroup'.$uid.':'.$gid; + $cacheKey = 'inGroup' . $uid . ':' . $gid; $inGroup = $this->access->connection->getFromCache($cacheKey); if (!is_null($inGroup)) { return (bool)$inGroup; @@ -108,7 +109,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return $isInGroup; } - $cacheKeyMembers = 'inGroup-members:'.$gid; + $cacheKeyMembers = 'inGroup-members:' . $gid; $members = $this->access->connection->getFromCache($cacheKeyMembers); if (!is_null($members)) { $this->cachedGroupMembers[$gid] = $members; @@ -199,13 +200,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $pos = strpos($memberURLs[0], '('); if ($pos !== false) { $memberUrlFilter = substr($memberURLs[0], $pos); - $foundMembers = $this->access->searchUsers($memberUrlFilter,'dn'); + $foundMembers = $this->access->searchUsers($memberUrlFilter, 'dn'); $dynamicMembers = []; foreach ($foundMembers as $value) { $dynamicMembers[$value['dn'][0]] = 1; } } else { - \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. + \OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' . 'of group ' . $dnGroup, ILogger::DEBUG); } } @@ -228,7 +229,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return []; } // used extensively in cron job, caching makes sense for nested groups - $cacheKey = '_groupMembers'.$dnGroup; + $cacheKey = '_groupMembers' . $dnGroup; $groupMembers = $this->access->connection->getFromCache($cacheKey); if ($groupMembers !== null) { return $groupMembers; @@ -274,7 +275,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD }; $groups = $this->walkNestedGroups($DN, $fetcher, $groups); - return $this->access->groupsMatchFilter($groups); + return $this->filterValidGroups($groups); } /** @@ -284,7 +285,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @return array */ private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): array { - $nesting = (int) $this->access->connection->ldapNestedGroups; + $nesting = (int)$this->access->connection->ldapNestedGroups; // depending on the input, we either have a list of DNs or a list of LDAP records // also, the output expects either DNs or records. Testing the first element should suffice. $recordMode = is_array($list) && isset($list[0]) && is_array($list[0]) && isset($list[0]['dn'][0]); @@ -319,6 +320,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * translates a gidNumber into an ownCloud internal name + * * @param string $gid as given by gidNumber on POSIX LDAP * @param string $dn a DN that belongs to the same domain as the group * @return string|bool @@ -354,6 +356,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the entry's gidNumber + * * @param string $dn * @param string $attribute * @return string|bool @@ -368,6 +371,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the group's primary ID + * * @param string $dn * @return string|bool */ @@ -377,6 +381,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the user's gidNumber + * * @param string $dn * @return string|bool */ @@ -410,7 +415,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if ($search !== '') { $filterParts[] = $this->access->getFilterPartForUserSearch($search); } - $filterParts[] = $this->access->connection->ldapGidNumber .'=' . $groupID; + $filterParts[] = $this->access->connection->ldapGidNumber . '=' . $groupID; return $this->access->combineFilterWithAnd($filterParts); } @@ -460,6 +465,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * gets the gidNumber of a user + * * @param string $dn * @return string */ @@ -477,6 +483,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * translates a primary group ID into an Nextcloud internal name + * * @param string $gid as given by primaryGroupID on AD * @param string $dn a DN that belongs to the same domain as the group * @return string|bool @@ -516,6 +523,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the entry's primary group ID + * * @param string $dn * @param string $attribute * @return string|bool @@ -530,6 +538,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the group's primary ID + * * @param string $dn * @return string|bool */ @@ -539,6 +548,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the user's primary group ID + * * @param string $dn * @return string|bool */ @@ -622,6 +632,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * gets the primary group of a user + * * @param string $dn * @return string */ @@ -639,6 +650,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * Get all groups a user belongs to + * * @param string $uid Name of the user * @return array with group names * @@ -651,7 +663,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if (!$this->enabled) { return []; } - $cacheKey = 'getUserGroups'.$uid; + $cacheKey = 'getUserGroups' . $uid; $userGroups = $this->access->connection->getFromCache($cacheKey); if (!is_null($userGroups)) { return $userGroups; @@ -671,14 +683,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if (!empty($dynamicGroupMemberURL)) { // look through dynamic groups to add them to the result array if needed $groupsToMatch = $this->access->fetchListOfGroups( - $this->access->connection->ldapGroupFilter,['dn',$dynamicGroupMemberURL]); + $this->access->connection->ldapGroupFilter, ['dn', $dynamicGroupMemberURL]); foreach ($groupsToMatch as $dynamicGroup) { if (!array_key_exists($dynamicGroupMemberURL, $dynamicGroup)) { continue; } $pos = strpos($dynamicGroup[$dynamicGroupMemberURL][0], '('); if ($pos !== false) { - $memberUrlFilter = substr($dynamicGroup[$dynamicGroupMemberURL][0],$pos); + $memberUrlFilter = substr($dynamicGroup[$dynamicGroupMemberURL][0], $pos); // apply filter via ldap search to see if this user is in this // dynamic group $userMatch = $this->access->readAttribute( @@ -696,7 +708,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } } } else { - \OCP\Util::writeLog('user_ldap', 'No search filter found on member url '. + \OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' . 'of group ' . print_r($dynamicGroup, true), ILogger::DEBUG); } } @@ -708,7 +720,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if ((int)$this->access->connection->hasMemberOfFilterSupport === 1 && (int)$this->access->connection->useMemberOfToDetectMembership === 1 && strtolower($this->access->connection->ldapGroupMemberAssocAttr) !== 'memberuid' - ) { + ) { $groupDNs = $this->_getGroupDNsFromMemberOf($userDN); if (is_array($groupDNs)) { foreach ($groupDNs as $dn) { @@ -739,7 +751,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } elseif (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') { $result = $this->access->readAttribute($userDN, 'uid'); if ($result === false) { - \OCP\Util::writeLog('user_ldap', 'No uid attribute found for DN ' . $userDN . ' on '. + \OCP\Util::writeLog('user_ldap', 'No uid attribute found for DN ' . $userDN . ' on ' . $this->access->connection->ldapHost, ILogger::DEBUG); $uid = false; } else { @@ -789,9 +801,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } $allGroups = []; $seen[$dn] = true; - $filter = $this->access->connection->ldapGroupMemberAssocAttr.'='.$dn; + $filter = $this->access->connection->ldapGroupMemberAssocAttr . '=' . $dn; $groups = $this->access->fetchListOfGroups($filter, - [$this->access->connection->ldapGroupDisplayName, 'dn']); + [strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']); if (is_array($groups)) { $fetcher = function ($dn, &$seen) { if (is_array($dn) && isset($dn['dn'][0])) { @@ -801,8 +813,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD }; $allGroups = $this->walkNestedGroups($dn, $fetcher, $groups); } - $visibleGroups = $this->access->groupsMatchFilter(array_keys($allGroups)); - return array_intersect_key($allGroups, array_flip($visibleGroups)); + $visibleGroups = $this->filterValidGroups($allGroups); + return array_intersect_key($allGroups, $visibleGroups); } /** @@ -823,7 +835,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return []; } $search = $this->access->escapeFilterPart($search, true); - $cacheKey = 'usersInGroup-'.$gid.'-'.$search.'-'.$limit.'-'.$offset; + $cacheKey = 'usersInGroup-' . $gid . '-' . $search . '-' . $limit . '-' . $offset; // check for cache of the exact query $groupUsers = $this->access->connection->getFromCache($cacheKey); if (!is_null($groupUsers)) { @@ -831,7 +843,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } // check for cache of the query without limit and offset - $groupUsers = $this->access->connection->getFromCache('usersInGroup-'.$gid.'-'.$search); + $groupUsers = $this->access->connection->getFromCache('usersInGroup-' . $gid . '-' . $search); if (!is_null($groupUsers)) { $groupUsers = array_slice($groupUsers, $offset, $limit); $this->access->connection->writeToCache($cacheKey, $groupUsers); @@ -907,7 +919,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $groupUsers = array_unique(array_merge($groupUsers, $primaryUsers, $posixGroupUsers)); natsort($groupUsers); - $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); + $this->access->connection->writeToCache('usersInGroup-' . $gid . '-' . $search, $groupUsers); $groupUsers = array_slice($groupUsers, $offset, $limit); $this->access->connection->writeToCache($cacheKey, $groupUsers); @@ -917,6 +929,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the number of users in a group, who match the search term + * * @param string $gid the internal group name * @param string $search optional, a search string * @return int|bool @@ -926,7 +939,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return $this->groupPluginManager->countUsersInGroup($gid, $search); } - $cacheKey = 'countUsersInGroup-'.$gid.'-'.$search; + $cacheKey = 'countUsersInGroup-' . $gid . '-' . $search; if (!$this->enabled || !$this->groupExists($gid)) { return false; } @@ -958,7 +971,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $search = $this->access->escapeFilterPart($search, true); $isMemberUid = (strtolower($this->access->connection->ldapGroupMemberAssocAttr) - === 'memberuid'); + === 'memberuid'); //we need to apply the search filter //alternatives that need to be checked: @@ -1015,10 +1028,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if (!$this->enabled) { return []; } - $cacheKey = 'getGroups-'.$search.'-'.$limit.'-'.$offset; + $cacheKey = 'getGroups-' . $search . '-' . $limit . '-' . $offset; //Check cache before driving unnecessary searches - \OCP\Util::writeLog('user_ldap', 'getGroups '.$cacheKey, ILogger::DEBUG); + \OCP\Util::writeLog('user_ldap', 'getGroups ' . $cacheKey, ILogger::DEBUG); $ldap_groups = $this->access->connection->getFromCache($cacheKey); if (!is_null($ldap_groups)) { return $ldap_groups; @@ -1033,11 +1046,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $this->access->connection->ldapGroupFilter, $this->access->getFilterPartForGroupSearch($search) ]); - \OCP\Util::writeLog('user_ldap', 'getGroups Filter '.$filter, ILogger::DEBUG); + \OCP\Util::writeLog('user_ldap', 'getGroups Filter ' . $filter, ILogger::DEBUG); $ldap_groups = $this->access->fetchListOfGroups($filter, - [$this->access->connection->ldapGroupDisplayName, 'dn'], - $limit, - $offset); + [$this->access->connection->ldapGroupDisplayName, 'dn'], + $limit, + $offset); $ldap_groups = $this->access->nextcloudGroupNames($ldap_groups); $this->access->connection->writeToCache($cacheKey, $ldap_groups); @@ -1078,7 +1091,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $chunkLimit = min($pagingSize, $overallLimit - $chunkOffset); $ldapGroups = $this->getGroupsChunk($search, $chunkLimit, $chunkOffset); $nread = count($ldapGroups); - \OCP\Util::writeLog('user_ldap', 'getGroups('.$search.'): read '.$nread.' at offset '.$chunkOffset.' (limit: '.$chunkLimit.')', ILogger::DEBUG); + \OCP\Util::writeLog('user_ldap', 'getGroups(' . $search . '): read ' . $nread . ' at offset ' . $chunkOffset . ' (limit: ' . $chunkLimit . ')', ILogger::DEBUG); if ($nread) { $allGroups = array_merge($allGroups, $ldapGroups); $chunkOffset += $nread; @@ -1100,11 +1113,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * check if a group exists + * * @param string $gid * @return bool */ public function groupExists($gid) { - $groupExists = $this->access->connection->getFromCache('groupExists'.$gid); + $groupExists = $this->access->connection->getFromCache('groupExists' . $gid); if (!is_null($groupExists)) { return (bool)$groupExists; } @@ -1113,22 +1127,43 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD //only, requires more checking. $dn = $this->access->groupname2dn($gid); if (!$dn) { - $this->access->connection->writeToCache('groupExists'.$gid, false); + $this->access->connection->writeToCache('groupExists' . $gid, false); + return false; + } + + if (!$this->access->isDNPartOfBase($dn, $this->access->connection->ldapBaseGroups)) { + $this->access->connection->writeToCache('groupExists' . $gid, false); return false; } //if group really still exists, we will be able to read its objectclass - if (!is_array($this->access->readAttribute($dn, ''))) { - $this->access->connection->writeToCache('groupExists'.$gid, false); + if (!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapGroupFilter))) { + $this->access->connection->writeToCache('groupExists' . $gid, false); return false; } - $this->access->connection->writeToCache('groupExists'.$gid, true); + $this->access->connection->writeToCache('groupExists' . $gid, true); return true; } + protected function filterValidGroups(array $listOfGroups): array { + $validGroupDNs = []; + foreach ($listOfGroups as $key => $item) { + $dn = is_string($item) ? $item : $item['dn'][0]; + $gid = $this->access->dn2groupname($dn); + if (!$gid) { + continue; + } + if ($this->groupExists($gid)) { + $validGroupDNs[$key] = $item; + } + } + return $validGroupDNs; + } + /** * Check if backend implements actions + * * @param int $actions bitwise-or'ed actions * @return boolean * @@ -1142,6 +1177,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * Return access for LDAP interaction. + * * @return Access instance of Access for LDAP interaction */ public function getLDAPAccess($gid) { @@ -1150,6 +1186,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * create a group + * * @param string $gid * @return bool * @throws \Exception @@ -1177,6 +1214,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * delete a group + * * @param string $gid gid of the group to delete * @return bool * @throws \Exception @@ -1186,7 +1224,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD if ($ret = $this->groupPluginManager->deleteGroup($gid)) { #delete group in nextcloud internal db $this->access->getGroupMapper()->unmap($gid); - $this->access->connection->writeToCache("groupExists".$gid, false); + $this->access->connection->writeToCache("groupExists" . $gid, false); } return $ret; } @@ -1195,6 +1233,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * Add a user to a group + * * @param string $uid Name of the user to add to group * @param string $gid Name of the group in which add the user * @return bool @@ -1213,6 +1252,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * Removes a user from a group + * * @param string $uid Name of the user to remove from group * @param string $gid Name of the group from which remove the user * @return bool @@ -1231,6 +1271,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * Gets group details + * * @param string $gid Name of the group * @return array | false * @throws \Exception @@ -1246,6 +1287,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * Return LDAP connection resource from a cloned connection. * The cloned connection needs to be closed manually. * of the current access. + * * @param string $gid * @return resource of the LDAP connection */ diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php index 3bd0cc4c400..c6a2c7fb6fb 100644 --- a/apps/user_ldap/lib/Group_Proxy.php +++ b/apps/user_ldap/lib/Group_Proxy.php @@ -36,6 +36,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Constructor + * * @param string[] $serverConfigPrefixes array containing the config Prefixes */ public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap, GroupPluginManager $groupPluginManager) { @@ -51,6 +52,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Tries the backends one after the other until a positive result is returned from the specified method + * * @param string $gid the gid connected to the request * @param string $method the method of the group backend that shall be called * @param array $parameters an array of parameters to be passed @@ -60,7 +62,9 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet $cacheKey = $this->getGroupCacheKey($gid); foreach ($this->backends as $configPrefix => $backend) { if ($result = call_user_func_array([$backend, $method], $parameters)) { - $this->writeToCache($cacheKey, $configPrefix); + if (!$this->isSingleBackend()) { + $this->writeToCache($cacheKey, $configPrefix); + } return $result; } } @@ -69,6 +73,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Asks the backend connected to the server that supposely takes care of the gid from the request. + * * @param string $gid the gid connected to the request * @param string $method the method of the group backend that shall be called * @param array $parameters an array of parameters to be passed @@ -99,8 +104,13 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet return false; } + protected function activeBackends(): int { + return count($this->backends); + } + /** * is user in group? + * * @param string $uid uid of the user * @param string $gid gid of the group * @return bool @@ -113,6 +123,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Get all groups a user belongs to + * * @param string $uid Name of the user * @return string[] with group names * @@ -134,6 +145,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * get a list of all users in a group + * * @return string[] with user ids */ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { @@ -160,6 +172,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * delete a group + * * @param string $gid gid of the group to delete * @return bool */ @@ -170,6 +183,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Add a user to a group + * * @param string $uid Name of the user to add to group * @param string $gid Name of the group in which add the user * @return bool @@ -183,6 +197,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Removes a user from a group + * * @param string $uid Name of the user to remove from group * @param string $gid Name of the group from which remove the user * @return bool @@ -196,6 +211,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * returns the number of users in a group, who match the search term + * * @param string $gid the internal group name * @param string $search optional, a search string * @return int|bool @@ -207,6 +223,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * get an array with group details + * * @param string $gid * @return array|false */ @@ -217,6 +234,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * get a list of all groups + * * @return string[] with group names * * Returns a list with all groups @@ -236,6 +254,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * check if a group exists + * * @param string $gid * @return bool */ @@ -245,6 +264,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Check if backend implements actions + * * @param int $actions bitwise-or'ed actions * @return boolean * @@ -258,6 +278,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Return access for LDAP interaction. + * * @param string $gid * @return Access instance of Access for LDAP interaction */ @@ -268,6 +289,7 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet /** * Return a new LDAP connection for the specified group. * The connection needs to be closed manually. + * * @param string $gid * @return resource of the LDAP connection */ diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php index 90fa3d05892..05e56c318af 100644 --- a/apps/user_ldap/lib/Helper.php +++ b/apps/user_ldap/lib/Helper.php @@ -34,6 +34,7 @@ namespace OCA\User_LDAP; +use OC\Cache\CappedMemoryCache; use OCP\IConfig; class Helper { @@ -41,6 +42,9 @@ class Helper { /** @var IConfig */ private $config; + /** @var CappedMemoryCache */ + protected $sanitizeDnCache; + /** * Helper constructor. * @@ -48,10 +52,12 @@ class Helper { */ public function __construct(IConfig $config) { $this->config = $config; + $this->sanitizeDnCache = new CappedMemoryCache(10000); } /** * returns prefixes for each saved LDAP/AD server configuration. + * * @param bool $activeConfigurations optional, whether only active configuration shall be * retrieved, defaults to false * @return array with a list of the available prefixes @@ -92,6 +98,7 @@ class Helper { /** * * determines the host for every configured connection + * * @return array an array with configprefix as keys * */ @@ -144,6 +151,7 @@ class Helper { /** * deletes a given saved LDAP/AD server configuration. + * * @param string $prefix the configuration prefix of the config to delete * @return bool true on success, false otherwise */ @@ -161,11 +169,11 @@ class Helper { DELETE FROM `*PREFIX*appconfig` WHERE `configkey` LIKE ? - '.$saveOtherConfigurations.' + ' . $saveOtherConfigurations . ' AND `appid` = \'user_ldap\' AND `configkey` NOT IN (\'enabled\', \'installed_version\', \'types\', \'bgjUpdateGroupsLastRun\') '); - $delRows = $query->execute([$prefix.'%']); + $delRows = $query->execute([$prefix . '%']); if ($delRows === null) { return false; @@ -180,8 +188,9 @@ class Helper { /** * checks whether there is one or more disabled LDAP configurations - * @throws \Exception + * * @return bool + * @throws \Exception */ public function haveDisabledConfigurations() { $all = $this->getServerConfigurationPrefixes(false); @@ -196,6 +205,7 @@ class Helper { /** * extracts the domain from a given URL + * * @param string $url the URL * @return string|false domain as string on success, false otherwise */ @@ -229,6 +239,7 @@ class Helper { /** * sanitizes a DN received from the LDAP server + * * @param array $dn the DN in question * @return array|string the sanitized DN */ @@ -242,12 +253,20 @@ class Helper { return $result; } + if (!is_string($dn)) { + throw new \LogicException('String expected ' . \gettype($dn) . ' given'); + } + + if (($sanitizedDn = $this->sanitizeDnCache->get($dn)) !== null) { + return $sanitizedDn; + } + //OID sometimes gives back DNs with whitespace after the comma // a la "uid=foo, cn=bar, dn=..." We need to tackle this! - $dn = preg_replace('/([^\\\]),(\s+)/u', '\1,', $dn); + $sanitizedDn = preg_replace('/([^\\\]),(\s+)/u', '\1,', $dn); //make comparisons and everything work - $dn = mb_strtolower($dn, 'UTF-8'); + $sanitizedDn = mb_strtolower($sanitizedDn, 'UTF-8'); //escape DN values according to RFC 2253 – this is already done by ldap_explode_dn //to use the DN in search filters, \ needs to be escaped to \5c additionally @@ -261,17 +280,19 @@ class Helper { '\;' => '\5c3B', '\"' => '\5c22', '\#' => '\5c23', - '(' => '\28', - ')' => '\29', - '*' => '\2A', + '(' => '\28', + ')' => '\29', + '*' => '\2A', ]; - $dn = str_replace(array_keys($replacements), array_values($replacements), $dn); + $sanitizedDn = str_replace(array_keys($replacements), array_values($replacements), $sanitizedDn); + $this->sanitizeDnCache->set($dn, $sanitizedDn); - return $dn; + return $sanitizedDn; } /** * converts a stored DN so it can be used as base parameter for LDAP queries, internally we store them for usage in LDAP filters + * * @param string $dn the DN * @return string */ @@ -302,7 +323,7 @@ class Helper { $userSession = \OC::$server->getUserSession(); $userPluginManager = \OC::$server->query('LDAPUserPluginManager'); - $userBackend = new User_Proxy( + $userBackend = new User_Proxy( $configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession, $userPluginManager ); $uid = $userBackend->loginName2UserName($param['uid']); diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index e14c9a572de..6fd07f5f483 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -26,8 +26,11 @@ namespace OCA\User_LDAP\Mapping; +use OC\DB\QueryBuilder\QueryBuilder; + /** * Class AbstractMapping + * * @package OCA\User_LDAP\Mapping */ abstract class AbstractMapping { @@ -38,9 +41,10 @@ abstract class AbstractMapping { /** * returns the DB table name which holds the mappings + * * @return string */ - abstract protected function getTableName(); + abstract protected function getTableName(bool $includePrefix = true); /** * @param \OCP\IDBConnection $dbc @@ -49,8 +53,12 @@ abstract class AbstractMapping { $this->dbc = $dbc; } + /** @var array caches Names (value) by DN (key) */ + protected $cache = []; + /** * checks whether a provided string represents an existing table col + * * @param string $col * @return bool */ @@ -67,11 +75,12 @@ abstract class AbstractMapping { /** * Gets the value of one column based on a provided value of another column + * * @param string $fetchCol * @param string $compareCol * @param string $search - * @throws \Exception * @return string|false + * @throws \Exception */ protected function getXbyY($fetchCol, $compareCol, $search) { if (!$this->isColNameValid($fetchCol)) { @@ -81,7 +90,7 @@ abstract class AbstractMapping { } $query = $this->dbc->prepare(' SELECT `' . $fetchCol . '` - FROM `'. $this->getTableName() .'` + FROM `' . $this->getTableName() . '` WHERE `' . $compareCol . '` = ? '); @@ -95,6 +104,7 @@ abstract class AbstractMapping { /** * Performs a DELETE or UPDATE query to the database. + * * @param \Doctrine\DBAL\Driver\Statement $query * @param array $parameters * @return bool true if at least one row was modified, false otherwise @@ -107,27 +117,42 @@ abstract class AbstractMapping { /** * Gets the LDAP DN based on the provided name. * Replaces Access::ocname2dn + * * @param string $name * @return string|false */ public function getDNByName($name) { - return $this->getXbyY('ldap_dn', 'owncloud_name', $name); + $dn = array_search($name, $this->cache); + if ($dn === false) { + $dn = $this->getXbyY('ldap_dn', 'owncloud_name', $name); + $this->cache[$dn] = $name; + } + return $dn; } /** * Updates the DN based on the given UUID + * * @param string $fdn * @param string $uuid * @return bool */ public function setDNbyUUID($fdn, $uuid) { + $oldDn = $this->getDnByUUID($uuid); $query = $this->dbc->prepare(' UPDATE `' . $this->getTableName() . '` SET `ldap_dn` = ? WHERE `directory_uuid` = ? '); - return $this->modify($query, [$fdn, $uuid]); + $r = $this->modify($query, [$fdn, $uuid]); + + if ($r && is_string($oldDn) && isset($this->cache[$oldDn])) { + $this->cache[$fdn] = $this->cache[$oldDn]; + unset($this->cache[$oldDn]); + } + + return $r; } /** @@ -146,20 +171,44 @@ abstract class AbstractMapping { WHERE `ldap_dn` = ? '); + unset($this->cache[$fdn]); + return $this->modify($query, [$uuid, $fdn]); } /** * Gets the name based on the provided LDAP DN. + * * @param string $fdn * @return string|false */ public function getNameByDN($fdn) { - return $this->getXbyY('owncloud_name', 'ldap_dn', $fdn); + if (!isset($this->cache[$fdn])) { + $this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn', $fdn); + } + return $this->cache[$fdn]; + } + + public function getListOfIdsByDn(array $fdns): array { + $qb = $this->dbc->getQueryBuilder(); + $qb->select('owncloud_name', 'ldap_dn') + ->from($this->getTableName(false)) + ->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($fdns, QueryBuilder::PARAM_STR_ARRAY))); + $stmt = $qb->execute(); + + $results = $stmt->fetchAll(\Doctrine\DBAL\FetchMode::ASSOCIATIVE); + foreach ($results as $key => $entry) { + unset($results[$key]); + $results[$entry['ldap_dn']] = $entry['owncloud_name']; + $this->cache[$entry['ldap_dn']] = $entry['owncloud_name']; + } + + return $results; } /** * Searches mapped names by the giving string in the name column + * * @param string $search * @param string $prefixMatch * @param string $postfixMatch @@ -168,11 +217,11 @@ abstract class AbstractMapping { public function getNamesBySearch($search, $prefixMatch = "", $postfixMatch = "") { $query = $this->dbc->prepare(' SELECT `owncloud_name` - FROM `'. $this->getTableName() .'` + FROM `' . $this->getTableName() . '` WHERE `owncloud_name` LIKE ? '); - $res = $query->execute([$prefixMatch.$this->dbc->escapeLikeParameter($search).$postfixMatch]); + $res = $query->execute([$prefixMatch . $this->dbc->escapeLikeParameter($search) . $postfixMatch]); $names = []; if ($res !== false) { while ($row = $query->fetch()) { @@ -184,6 +233,7 @@ abstract class AbstractMapping { /** * Gets the name based on the provided LDAP UUID. + * * @param string $uuid * @return string|false */ @@ -191,8 +241,13 @@ abstract class AbstractMapping { return $this->getXbyY('owncloud_name', 'directory_uuid', $uuid); } + public function getDnByUUID($uuid) { + return $this->getXbyY('ldap_dn', 'directory_uuid', $uuid); + } + /** * Gets the UUID based on the provided LDAP DN + * * @param string $dn * @return false|string * @throws \Exception @@ -203,6 +258,7 @@ abstract class AbstractMapping { /** * gets a piece of the mapping list + * * @param int $offset * @param int $limit * @return array @@ -224,6 +280,7 @@ abstract class AbstractMapping { /** * attempts to map the given entry + * * @param string $fdn fully distinguished name (from LDAP) * @param string $name * @param string $uuid a unique identifier as used in LDAP @@ -242,13 +299,16 @@ abstract class AbstractMapping { } $row = [ - 'ldap_dn' => $fdn, - 'owncloud_name' => $name, + 'ldap_dn' => $fdn, + 'owncloud_name' => $name, 'directory_uuid' => $uuid ]; try { $result = $this->dbc->insertIfNotExist($this->getTableName(), $row); + if ((bool)$result === true) { + $this->cache[$fdn] = $name; + } // insertIfNotExist returns values as int return (bool)$result; } catch (\Exception $e) { @@ -258,12 +318,13 @@ abstract class AbstractMapping { /** * removes a mapping based on the owncloud_name of the entry + * * @param string $name * @return bool */ public function unmap($name) { $query = $this->dbc->prepare(' - DELETE FROM `'. $this->getTableName() .'` + DELETE FROM `' . $this->getTableName() . '` WHERE `owncloud_name` = ?'); return $this->modify($query, [$name]); @@ -271,6 +332,7 @@ abstract class AbstractMapping { /** * Truncate's the mapping table + * * @return bool */ public function clear() { diff --git a/apps/user_ldap/lib/Mapping/GroupMapping.php b/apps/user_ldap/lib/Mapping/GroupMapping.php index b2c1b9c99af..703cc56a02a 100644 --- a/apps/user_ldap/lib/Mapping/GroupMapping.php +++ b/apps/user_ldap/lib/Mapping/GroupMapping.php @@ -33,7 +33,8 @@ class GroupMapping extends AbstractMapping { * returns the DB table name which holds the mappings * @return string */ - protected function getTableName() { - return '*PREFIX*ldap_group_mapping'; + protected function getTableName(bool $includePrefix = true) { + $p = $includePrefix ? '*PREFIX*' : ''; + return $p . 'ldap_group_mapping'; } } diff --git a/apps/user_ldap/lib/Mapping/UserMapping.php b/apps/user_ldap/lib/Mapping/UserMapping.php index 556f7ecf1a4..b70cb866904 100644 --- a/apps/user_ldap/lib/Mapping/UserMapping.php +++ b/apps/user_ldap/lib/Mapping/UserMapping.php @@ -33,7 +33,8 @@ class UserMapping extends AbstractMapping { * returns the DB table name which holds the mappings * @return string */ - protected function getTableName() { - return '*PREFIX*ldap_user_mapping'; + protected function getTableName(bool $includePrefix = true) { + $p = $includePrefix ? '*PREFIX*' : ''; + return $p . 'ldap_user_mapping'; } } diff --git a/apps/user_ldap/lib/Proxy.php b/apps/user_ldap/lib/Proxy.php index 3cf55f8cd58..7bcbd19ff1c 100644 --- a/apps/user_ldap/lib/Proxy.php +++ b/apps/user_ldap/lib/Proxy.php @@ -40,6 +40,8 @@ use OCA\User_LDAP\User\Manager; abstract class Proxy { private static $accesses = []; private $ldap = null; + /** @var bool */ + private $isSingleBackend; /** @var \OCP\ICache|null */ private $cache; @@ -70,11 +72,11 @@ abstract class Proxy { static $coreNotificationManager; if ($fs === null) { $ocConfig = \OC::$server->getConfig(); - $fs = new FilesystemHelper(); - $log = new LogWrapper(); - $avatarM = \OC::$server->getAvatarManager(); - $db = \OC::$server->getDatabaseConnection(); - $userMap = new UserMapping($db); + $fs = new FilesystemHelper(); + $log = new LogWrapper(); + $avatarM = \OC::$server->getAvatarManager(); + $db = \OC::$server->getDatabaseConnection(); + $userMap = new UserMapping($db); $groupMap = new GroupMapping($db); $coreUserManager = \OC::$server->getUserManager(); $coreNotificationManager = \OC::$server->getNotificationManager(); @@ -105,7 +107,7 @@ abstract class Proxy { * @return string */ protected function getUserCacheKey($uid) { - return 'user-'.$uid.'-lastSeenOn'; + return 'user-' . $uid . '-lastSeenOn'; } /** @@ -113,7 +115,7 @@ abstract class Proxy { * @return string */ protected function getGroupCacheKey($gid) { - return 'group-'.$gid.'-lastSeenOn'; + return 'group-' . $gid . '-lastSeenOn'; } /** @@ -139,8 +141,18 @@ abstract class Proxy { */ abstract public function getLDAPAccess($id); + abstract protected function activeBackends(): int; + + protected function isSingleBackend(): bool { + if ($this->isSingleBackend === null) { + $this->isSingleBackend = $this->activeBackends() === 1; + } + return $this->isSingleBackend; + } + /** * Takes care of the request to the User backend + * * @param string $id * @param string $method string, the method of the user backend that shall be called * @param array $parameters an array of parameters to be passed @@ -148,8 +160,10 @@ abstract class Proxy { * @return mixed, the result of the specified method */ protected function handleRequest($id, $method, $parameters, $passOnWhen = false) { - $result = $this->callOnLastSeenOn($id, $method, $parameters, $passOnWhen); - if ($result === $passOnWhen) { + if (!$this->isSingleBackend()) { + $result = $this->callOnLastSeenOn($id, $method, $parameters, $passOnWhen); + } + if (!isset($result) || $result === $passOnWhen) { $result = $this->walkBackends($id, $method, $parameters); } return $result; @@ -164,7 +178,7 @@ abstract class Proxy { if ($key === null) { return $prefix; } - return $prefix.hash('sha256', $key); + return $prefix . hash('sha256', $key); } /** @@ -193,7 +207,7 @@ abstract class Proxy { if ($this->cache === null) { return; } - $key = $this->getCacheKey($key); + $key = $this->getCacheKey($key); $value = base64_encode(json_encode($value)); $this->cache->set($key, $value, 2592000); } diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php index e9ff92d03eb..a81e4668347 100644 --- a/apps/user_ldap/lib/User_Proxy.php +++ b/apps/user_ldap/lib/User_Proxy.php @@ -72,6 +72,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Tries the backends one after the other until a positive result is returned from the specified method + * * @param string $uid the uid connected to the request * @param string $method the method of the user backend that shall be called * @param array $parameters an array of parameters to be passed @@ -86,7 +87,9 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, $instance = $this->getAccess($configPrefix); } if ($result = call_user_func_array([$instance, $method], $parameters)) { - $this->writeToCache($cacheKey, $configPrefix); + if (!$this->isSingleBackend()) { + $this->writeToCache($cacheKey, $configPrefix); + } return $result; } } @@ -95,6 +98,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Asks the backend connected to the server that supposely takes care of the uid from the request. + * * @param string $uid the uid connected to the request * @param string $method the method of the user backend that shall be called * @param array $parameters an array of parameters to be passed @@ -130,8 +134,13 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, return false; } + protected function activeBackends(): int { + return count($this->backends); + } + /** * Check if backend implements actions + * * @param int $actions bitwise-or'ed actions * @return boolean * @@ -145,6 +154,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Backend name to be shown in user management + * * @return string the name of the backend to be shown */ public function getBackendName() { @@ -173,6 +183,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * check if a user exists + * * @param string $uid the username * @return boolean */ @@ -197,6 +208,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * check if a user exists on LDAP + * * @param string|\OCA\User_LDAP\User\User $user either the Nextcloud user * name or an instance of that user * @return boolean @@ -208,6 +220,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Check if the password is correct + * * @param string $uid The username * @param string $password The password * @return bool @@ -228,7 +241,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, $id = 'LOGINNAME,' . $loginName; return $this->handleRequest($id, 'loginName2UserName', [$loginName]); } - + /** * returns the username for the given LDAP DN, if available * @@ -242,6 +255,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * get the user's home directory + * * @param string $uid the username * @return boolean */ @@ -251,6 +265,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * get display name of the user + * * @param string $uid user ID of the user * @return string display name */ @@ -271,6 +286,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * checks whether the user is allowed to change his avatar in Nextcloud + * * @param string $uid the Nextcloud user name * @return boolean either the user can or cannot */ @@ -280,6 +296,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Get a list of all display names and user ids. + * * @param string $search * @param string|null $limit * @param string|null $offset @@ -299,6 +316,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * delete a user + * * @param string $uid The username of the user to delete * @return bool * @@ -307,9 +325,10 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, public function deleteUser($uid) { return $this->handleRequest($uid, 'deleteUser', [$uid]); } - + /** * Set password + * * @param string $uid The username * @param string $password The new password * @return bool @@ -328,6 +347,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Count the number of users + * * @return int|bool */ public function countUsers() { @@ -343,16 +363,18 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Return access for LDAP interaction. + * * @param string $uid * @return Access instance of Access for LDAP interaction */ public function getLDAPAccess($uid) { return $this->handleRequest($uid, 'getLDAPAccess', [$uid]); } - + /** * Return a new LDAP connection for the specified user. * The connection needs to be closed manually. + * * @param string $uid * @return resource of the LDAP connection */ @@ -362,11 +384,12 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** * Creates a new user in LDAP + * * @param $username * @param $password * @return bool */ public function createUser($username, $password) { - return $this->handleRequest($username, 'createUser', [$username,$password]); + return $this->handleRequest($username, 'createUser', [$username, $password]); } } diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index ae637e0e584..64160e203a0 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -61,7 +61,7 @@ class Group_LDAPTest extends TestCase { $conMethods = get_class_methods('\OCA\User_LDAP\Connection'); $accMethods = get_class_methods('\OCA\User_LDAP\Access'); } - $lw = $this->createMock(ILDAPWrapper::class); + $lw = $this->createMock(ILDAPWrapper::class); $connector = $this->getMockBuilder('\OCA\User_LDAP\Connection') ->setMethods($conMethods) ->setConstructorArgs([$lw, null, null]) @@ -79,7 +79,7 @@ class Group_LDAPTest extends TestCase { private function getPluginManagerMock() { return $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')->getMock(); } - + /** * @param Access|\PHPUnit_Framework_MockObject_MockObject $access */ @@ -120,6 +120,9 @@ class Group_LDAPTest extends TestCase { } return []; }); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); // for primary groups $access->expects($this->once()) @@ -141,11 +144,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('groupname2dn') ->willReturn('cn=group,dc=foo,dc=bar'); - $access->expects($this->any()) ->method('fetchListOfUsers') ->willReturn([]); - $access->expects($this->any()) ->method('readAttribute') ->willReturnCallback(function ($name) { @@ -159,14 +160,16 @@ class Group_LDAPTest extends TestCase { } return ['u11', 'u22', 'u33', 'u34']; }); - $access->expects($this->any()) ->method('dn2username') ->willReturnCallback(function () { return 'foobar' . \OC::$server->getSecureRandom()->generate(7); }); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); - $groupBackend = new GroupLDAP($access,$pluginManager); + $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('group', '3'); $this->assertSame(2, $users); @@ -175,7 +178,7 @@ class Group_LDAPTest extends TestCase { public function testCountUsersWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','countUsersInGroup']) + ->setMethods(['implementsActions', 'countUsersInGroup']) ->getMock(); $pluginManager->expects($this->once()) @@ -193,7 +196,7 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->countUsersInGroup('gid', 'search'),42); + $this->assertEquals($ldap->countUsersInGroup('gid', 'search'), 42); } public function testGidNumber2NameSuccess() { @@ -475,7 +478,7 @@ class Group_LDAPTest extends TestCase { $uid = 'someUser'; $gid = 'someGroup'; - $cacheKey = 'inGroup'.$uid.':'.$gid; + $cacheKey = 'inGroup' . $uid . ':' . $gid; $access->connection->expects($this->once()) ->method('getFromCache') @@ -534,6 +537,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->exactly(2)) ->method('nextcloudUserNames') ->willReturnOnConsecutiveCalls(['lisa', 'bart', 'kira', 'brad'], ['walle', 'dino', 'xenia']); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $access->userManager = $this->createMock(Manager::class); $groupBackend = new GroupLDAP($access, $pluginManager); @@ -569,6 +575,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->once()) ->method('nextcloudUserNames') ->willReturn(['lisa', 'bart', 'kira', 'brad']); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $access->userManager = $this->createMock(Manager::class); $groupBackend = new GroupLDAP($access, $pluginManager); @@ -599,14 +608,15 @@ class Group_LDAPTest extends TestCase { } return []; }); - $access->expects($this->any()) ->method('groupname2dn') ->willReturn('cn=foobar,dc=foo,dc=bar'); - $access->expects($this->once()) ->method('countUsers') ->willReturn(4); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('foobar'); @@ -629,17 +639,19 @@ class Group_LDAPTest extends TestCase { ->method('username2dn') ->willReturn($dn); - $access->expects($this->exactly(3)) + $access->expects($this->exactly(5)) ->method('readAttribute') - ->will($this->onConsecutiveCalls(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar'], [], [])); + ->will($this->onConsecutiveCalls(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar'], [], [], [], [])); - $access->expects($this->exactly(2)) + $access->expects($this->any()) ->method('dn2groupname') ->willReturnArgument(0); - - $access->expects($this->exactly(1)) - ->method('groupsMatchFilter') + $access->expects($this->any()) + ->method('groupname2dn') ->willReturnArgument(0); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $groupBackend = new GroupLDAP($access, $pluginManager); $groups = $groupBackend->getUserGroups('userX'); @@ -677,9 +689,6 @@ class Group_LDAPTest extends TestCase { $access->expects($this->once()) ->method('nextcloudGroupNames') ->willReturn([]); - $access->expects($this->any()) - ->method('groupsMatchFilter') - ->willReturnArgument(0); $groupBackend = new GroupLDAP($access, $pluginManager); $groupBackend->getUserGroups('userX'); @@ -715,9 +724,9 @@ class Group_LDAPTest extends TestCase { ->method('username2dn') ->willReturn($dn); - $access->expects($this->never()) + $access->expects($this->any()) ->method('readAttribute') - ->with($dn, 'memberOf'); + ->willReturn([]); $group1 = [ 'cn' => 'group1', @@ -736,8 +745,23 @@ class Group_LDAPTest extends TestCase { ->method('fetchListOfGroups') ->willReturn([$group1, $group2]); $access->expects($this->any()) - ->method('groupsMatchFilter') - ->willReturnArgument(0); + ->method('dn2groupname') + ->willReturnCallback(function (string $dn) { + return ldap_explode_dn($dn, 1)[0]; + }); + $access->expects($this->any()) + ->method('groupname2dn') + ->willReturnCallback(function (string $gid) use ($group1, $group2) { + if ($gid === $group1['cn']) { + return $group1['dn'][0]; + } + if ($gid === $group2['cn']) { + return $group2['dn'][0]; + } + }); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $groupBackend = new GroupLDAP($access, $pluginManager); $groups = $groupBackend->getUserGroups('userX'); @@ -750,7 +774,7 @@ class Group_LDAPTest extends TestCase { public function testCreateGroupWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','createGroup']) + ->setMethods(['implementsActions', 'createGroup']) ->getMock(); $pluginManager->expects($this->once()) @@ -768,10 +792,10 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->createGroup('gid'),true); + $this->assertEquals($ldap->createGroup('gid'), true); } - + public function testCreateGroupFailing() { $this->expectException(\Exception::class); @@ -796,7 +820,7 @@ class Group_LDAPTest extends TestCase { public function testDeleteGroupWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','deleteGroup']) + ->setMethods(['implementsActions', 'deleteGroup']) ->getMock(); $pluginManager->expects($this->once()) @@ -823,10 +847,10 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->deleteGroup('gid'),'result'); + $this->assertEquals($ldap->deleteGroup('gid'), 'result'); } - + public function testDeleteGroupFailing() { $this->expectException(\Exception::class); @@ -851,7 +875,7 @@ class Group_LDAPTest extends TestCase { public function testAddToGroupWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','addToGroup']) + ->setMethods(['implementsActions', 'addToGroup']) ->getMock(); $pluginManager->expects($this->once()) @@ -869,10 +893,10 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->addToGroup('uid', 'gid'),'result'); + $this->assertEquals($ldap->addToGroup('uid', 'gid'), 'result'); } - + public function testAddToGroupFailing() { $this->expectException(\Exception::class); @@ -897,7 +921,7 @@ class Group_LDAPTest extends TestCase { public function testRemoveFromGroupWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','removeFromGroup']) + ->setMethods(['implementsActions', 'removeFromGroup']) ->getMock(); $pluginManager->expects($this->once()) @@ -915,10 +939,10 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->removeFromGroup('uid', 'gid'),'result'); + $this->assertEquals($ldap->removeFromGroup('uid', 'gid'), 'result'); } - + public function testRemoveFromGroupFailing() { $this->expectException(\Exception::class); @@ -943,7 +967,7 @@ class Group_LDAPTest extends TestCase { public function testGetGroupDetailsWithPlugin() { /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') - ->setMethods(['implementsActions','getGroupDetails']) + ->setMethods(['implementsActions', 'getGroupDetails']) ->getMock(); $pluginManager->expects($this->once()) @@ -961,10 +985,10 @@ class Group_LDAPTest extends TestCase { $ldap = new GroupLDAP($access, $pluginManager); - $this->assertEquals($ldap->getGroupDetails('gid'),'result'); + $this->assertEquals($ldap->getGroupDetails('gid'), 'result'); } - + public function testGetGroupDetailsFailing() { $this->expectException(\Exception::class); |