diff options
author | Morris Jobke <hey@morrisjobke.de> | 2020-07-06 15:00:27 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-07-06 15:00:27 +0300 |
commit | d72d9ff1f48eb282101c29aeea2a13c7f1368f8f (patch) | |
tree | 88889d2c9e7509b4867d4075f31741cfb2be2b8b /apps/user_ldap | |
parent | db782fefa13c75586c00bdd8b913253f92f70c1e (diff) | |
parent | 64fe042b0dea30587dd044c6d90220fc7de593b6 (diff) |
Merge pull request #21171 from nextcloud/enh/noid/tidy-up-group-ldap
tidy up Group_LDAP
Diffstat (limited to 'apps/user_ldap')
-rw-r--r-- | apps/user_ldap/lib/Access.php | 4 | ||||
-rw-r--r-- | apps/user_ldap/lib/Connection.php | 5 | ||||
-rw-r--r-- | apps/user_ldap/lib/Group_LDAP.php | 351 | ||||
-rw-r--r-- | apps/user_ldap/tests/Group_LDAPTest.php | 73 |
4 files changed, 198 insertions, 235 deletions
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index b93d92b81dd..ef4b5c9434e 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -1435,7 +1435,7 @@ class Access extends LDAPUtility { * @param bool $allowAsterisk whether in * at the beginning should be preserved * @return string the escaped string */ - public function escapeFilterPart($input, $allowAsterisk = false) { + public function escapeFilterPart($input, $allowAsterisk = false): string { $asterisk = ''; if ($allowAsterisk && strlen($input) > 0 && $input[0] === '*') { $asterisk = '*'; @@ -1452,7 +1452,7 @@ class Access extends LDAPUtility { * @param string[] $filters the filters to connect * @return string the combined filter */ - public function combineFilterWithAnd($filters) { + public function combineFilterWithAnd($filters): string { return $this->combineFilter($filters, '&'); } diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 0cc93a08e6f..079429027c5 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -67,6 +67,11 @@ use OCP\ILogger; * @property string[] ldapBaseGroups * @property string ldapGroupFilter * @property string ldapGroupDisplayName + * @property string ldapLoginFilter + * @property string ldapDynamicGroupMemberURL + * @property string ldapGidNumber + * @property int hasMemberOfFilterSupport + * @property int useMemberOfToDetectMembership */ class Connection extends LDAPUtility { private $ldapConnectionRes = null; diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 6b62f88123e..6f6e7362e97 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -43,37 +43,34 @@ namespace OCA\User_LDAP; +use Closure; +use Exception; +use OC; use OC\Cache\CappedMemoryCache; +use OC\ServerNotAvailableException; use OCP\Group\Backend\IGetDisplayNameBackend; use OCP\GroupInterface; use OCP\ILogger; -class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend { +class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend { protected $enabled = false; - /** - * @var string[] $cachedGroupMembers array of users with gid as key - */ + /** @var string[] $cachedGroupMembers array of users with gid as key */ protected $cachedGroupMembers; - - /** - * @var string[] $cachedGroupsByMember array of groups with uid as key - */ + /** @var string[] $cachedGroupsByMember array of groups with uid as key */ protected $cachedGroupsByMember; - - /** - * @var string[] $cachedNestedGroups array of groups with gid (DN) as key - */ + /** @var string[] $cachedNestedGroups array of groups with gid (DN) as key */ protected $cachedNestedGroups; - /** @var GroupPluginManager */ protected $groupPluginManager; + /** @var ILogger */ + protected $logger; public function __construct(Access $access, GroupPluginManager $groupPluginManager) { parent::__construct($access); $filter = $this->access->connection->ldapGroupFilter; - $gassoc = $this->access->connection->ldapGroupMemberAssocAttr; - if (!empty($filter) && !empty($gassoc)) { + $gAssoc = $this->access->connection->ldapGroupMemberAssocAttr; + if (!empty($filter) && !empty($gAssoc)) { $this->enabled = true; } @@ -81,6 +78,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $this->cachedGroupsByMember = new CappedMemoryCache(); $this->cachedNestedGroups = new CappedMemoryCache(); $this->groupPluginManager = $groupPluginManager; + $this->logger = OC::$server->getLogger(); } /** @@ -89,8 +87,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @param string $uid uid of the user * @param string $gid gid of the group * @return bool - * - * Checks whether the user is member of a group or not. + * @throws Exception + * @throws ServerNotAvailableException */ public function inGroup($uid, $gid) { if (!$this->enabled) { @@ -105,8 +103,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $userDN = $this->access->username2dn($uid); if (isset($this->cachedGroupMembers[$gid])) { - $isInGroup = in_array($userDN, $this->cachedGroupMembers[$gid]); - return $isInGroup; + return in_array($userDN, $this->cachedGroupMembers[$gid]); } $cacheKeyMembers = 'inGroup-members:' . $gid; @@ -175,13 +172,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * @param string $dnGroup - * @return array + * For a group that has user membership defined by an LDAP search url + * attribute returns the users that match the search url otherwise returns + * an empty array. * - * For a group that has user membership defined by an LDAP search url attribute returns the users - * that match the search url otherwise returns an empty array. + * @throws ServerNotAvailableException */ - public function getDynamicGroupMembers($dnGroup) { + public function getDynamicGroupMembers(string $dnGroup): array { $dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL); if (empty($dynamicGroupMemberURL)) { @@ -207,26 +204,26 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $dynamicMembers[$value['dn'][0]] = 1; } } else { - \OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' . - 'of group ' . $dnGroup, ILogger::DEBUG); + $this->logger->debug('No search filter found on member url of group {dn}', + [ + 'app' => 'user_ldap', + 'dn' => $dnGroup, + ] + ); } } return $dynamicMembers; } /** - * @param string $dnGroup - * @param array|null &$seen - * @return array|mixed|null - * @throws \OC\ServerNotAvailableException + * @throws ServerNotAvailableException */ - private function _groupMembers($dnGroup, &$seen = null) { + private function _groupMembers(string $dnGroup, ?array &$seen = null): array { if ($seen === null) { $seen = []; } $allMembers = []; if (array_key_exists($dnGroup, $seen)) { - // avoid loops return []; } // used extensively in cron job, caching makes sense for nested groups @@ -251,13 +248,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * @param string $DN - * @param array|null &$seen - * @return array - * @throws \OC\ServerNotAvailableException + * @throws ServerNotAvailableException */ - private function _getGroupDNsFromMemberOf($DN) { - $groups = $this->access->readAttribute($DN, 'memberOf'); + private function _getGroupDNsFromMemberOf(string $dn): array { + $groups = $this->access->readAttribute($dn, 'memberOf'); if (!is_array($groups)) { return []; } @@ -275,17 +269,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return $nestedGroups; }; - $groups = $this->walkNestedGroups($DN, $fetcher, $groups); + $groups = $this->walkNestedGroups($dn, $fetcher, $groups); return $this->filterValidGroups($groups); } - /** - * @param string $dn - * @param \Closure $fetcher args: string $dn, array $seen, returns: string[] of dns - * @param array $list - * @return array - */ - private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): array { + private function walkNestedGroups(string $dn, Closure $fetcher, array $list): array { $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. @@ -322,11 +310,11 @@ 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 + * @throws Exception + * @throws ServerNotAvailableException */ - public function gidNumber2Name($gid, $dn) { + public function gidNumber2Name(string $gid, string $dn) { $cacheKey = 'gidNumberToName' . $gid; $groupName = $this->access->connection->getFromCache($cacheKey); if (!is_null($groupName) && isset($groupName)) { @@ -339,14 +327,22 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD 'objectClass=posixGroup', $this->access->connection->ldapGidNumber . '=' . $gid ]); + return $this->getNameOfGroup($filter, $cacheKey) ?? false; + } + + /** + * @throws ServerNotAvailableException + * @throws Exception + */ + private function getNameOfGroup(string $filter, string $cacheKey) { $result = $this->access->searchGroups($filter, ['dn'], 1); if (empty($result)) { - return false; + return null; } $dn = $result[0]['dn'][0]; //and now the group name - //NOTE once we have separate ownCloud group IDs and group names we can + //NOTE once we have separate Nextcloud group IDs and group names we can //directly read the display name attribute instead of the DN $name = $this->access->dn2groupname($dn); @@ -358,11 +354,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns the entry's gidNumber * - * @param string $dn - * @param string $attribute * @return string|bool + * @throws ServerNotAvailableException */ - private function getEntryGidNumber($dn, $attribute) { + private function getEntryGidNumber(string $dn, string $attribute) { $value = $this->access->readAttribute($dn, $attribute); if (is_array($value) && !empty($value)) { return $value[0]; @@ -371,22 +366,20 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * returns the group's primary ID - * - * @param string $dn * @return string|bool + * @throws ServerNotAvailableException */ - public function getGroupGidNumber($dn) { + public function getGroupGidNumber(string $dn) { return $this->getEntryGidNumber($dn, 'gidNumber'); } /** * returns the user's gidNumber * - * @param string $dn * @return string|bool + * @throws ServerNotAvailableException */ - public function getUserGidNumber($dn) { + public function getUserGidNumber(string $dn) { $gidNumber = false; if ($this->access->connection->hasGidNumber) { $gidNumber = $this->getEntryGidNumber($dn, $this->access->connection->ldapGidNumber); @@ -398,17 +391,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * returns a filter for a "users has specific gid" search or count operation - * - * @param string $groupDN - * @param string $search - * @return string - * @throws \Exception + * @throws ServerNotAvailableException + * @throws Exception */ - private function prepareFilterForUsersHasGidNumber($groupDN, $search = '') { + private function prepareFilterForUsersHasGidNumber(string $groupDN, string $search = ''): string { $groupID = $this->getGroupGidNumber($groupDN); if ($groupID === false) { - throw new \Exception('Not a valid group'); + throw new Exception('Not a valid group'); } $filterParts = []; @@ -424,13 +413,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD /** * returns a list of users that have the given group as gid number * - * @param string $groupDN - * @param string $search - * @param int $limit - * @param int $offset - * @return string[] + * @throws ServerNotAvailableException */ - public function getUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) { + public function getUsersInGidNumber( + string $groupDN, + string $search = '', + ?int $limit = -1, + ?int $offset = 0 + ): array { try { $filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search); $users = $this->access->fetchListOfUsers( @@ -440,37 +430,18 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $offset ); return $this->access->nextcloudUserNames($users); - } catch (\Exception $e) { + } catch (ServerNotAvailableException $e) { + throw $e; + } catch (Exception $e) { return []; } } /** - * returns the number of users that have the given group as gid number - * - * @param string $groupDN - * @param string $search - * @param int $limit - * @param int $offset - * @return int - */ - public function countUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) { - try { - $filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search); - $users = $this->access->countUsers($filter, ['dn'], $limit, $offset); - return (int)$users; - } catch (\Exception $e) { - return 0; - } - } - - /** - * gets the gidNumber of a user - * - * @param string $dn - * @return string + * @throws ServerNotAvailableException + * @return bool */ - public function getUserGroupByGid($dn) { + public function getUserGroupByGid(string $dn) { $groupID = $this->getUserGidNumber($dn); if ($groupID !== false) { $groupName = $this->gidNumber2Name($groupID, $dn); @@ -485,11 +456,11 @@ 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 + * @throws Exception + * @throws ServerNotAvailableException */ - public function primaryGroupID2Name($gid, $dn) { + public function primaryGroupID2Name(string $gid, string $dn) { $cacheKey = 'primaryGroupIDtoName'; $groupNames = $this->access->connection->getFromCache($cacheKey); if (!is_null($groupNames) && isset($groupNames[$gid])) { @@ -506,30 +477,16 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $this->access->connection->ldapGroupFilter, 'objectsid=' . $domainObjectSid . '-' . $gid ]); - $result = $this->access->searchGroups($filter, ['dn'], 1); - if (empty($result)) { - return false; - } - $dn = $result[0]['dn'][0]; - - //and now the group name - //NOTE once we have separate Nextcloud group IDs and group names we can - //directly read the display name attribute instead of the DN - $name = $this->access->dn2groupname($dn); - - $this->access->connection->writeToCache($cacheKey, $name); - - return $name; + return $this->getNameOfGroup($filter, $cacheKey) ?? false; } /** * returns the entry's primary group ID * - * @param string $dn - * @param string $attribute * @return string|bool + * @throws ServerNotAvailableException */ - private function getEntryGroupID($dn, $attribute) { + private function getEntryGroupID(string $dn, string $attribute) { $value = $this->access->readAttribute($dn, $attribute); if (is_array($value) && !empty($value)) { return $value[0]; @@ -538,22 +495,18 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * returns the group's primary ID - * - * @param string $dn * @return string|bool + * @throws ServerNotAvailableException */ - public function getGroupPrimaryGroupID($dn) { + public function getGroupPrimaryGroupID(string $dn) { return $this->getEntryGroupID($dn, 'primaryGroupToken'); } /** - * returns the user's primary group ID - * - * @param string $dn * @return string|bool + * @throws ServerNotAvailableException */ - public function getUserPrimaryGroupIDs($dn) { + public function getUserPrimaryGroupIDs(string $dn) { $primaryGroupID = false; if ($this->access->connection->hasPrimaryGroups) { $primaryGroupID = $this->getEntryGroupID($dn, 'primaryGroupID'); @@ -565,17 +518,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * returns a filter for a "users in primary group" search or count operation - * - * @param string $groupDN - * @param string $search - * @return string - * @throws \Exception + * @throws Exception + * @throws ServerNotAvailableException */ - private function prepareFilterForUsersInPrimaryGroup($groupDN, $search = '') { + private function prepareFilterForUsersInPrimaryGroup(string $groupDN, string $search = ''): string { $groupID = $this->getGroupPrimaryGroupID($groupDN); if ($groupID === false) { - throw new \Exception('Not a valid group'); + throw new Exception('Not a valid group'); } $filterParts = []; @@ -589,15 +538,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * returns a list of users that have the given group as primary group - * - * @param string $groupDN - * @param string $search - * @param int $limit - * @param int $offset - * @return string[] + * @throws ServerNotAvailableException */ - public function getUsersInPrimaryGroup($groupDN, $search = '', $limit = -1, $offset = 0) { + public function getUsersInPrimaryGroup( + string $groupDN, + string $search = '', + ?int $limit = -1, + ?int $offset = 0 + ): array { try { $filter = $this->prepareFilterForUsersInPrimaryGroup($groupDN, $search); $users = $this->access->fetchListOfUsers( @@ -607,37 +555,38 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $offset ); return $this->access->nextcloudUserNames($users); - } catch (\Exception $e) { + } catch (ServerNotAvailableException $e) { + throw $e; + } catch (Exception $e) { return []; } } /** - * returns the number of users that have the given group as primary group - * - * @param string $groupDN - * @param string $search - * @param int $limit - * @param int $offset - * @return int + * @throws ServerNotAvailableException */ - public function countUsersInPrimaryGroup($groupDN, $search = '', $limit = -1, $offset = 0) { + public function countUsersInPrimaryGroup( + string $groupDN, + string $search = '', + int $limit = -1, + int $offset = 0 + ): int { try { $filter = $this->prepareFilterForUsersInPrimaryGroup($groupDN, $search); $users = $this->access->countUsers($filter, ['dn'], $limit, $offset); return (int)$users; - } catch (\Exception $e) { + } catch (ServerNotAvailableException $e) { + throw $e; + } catch (Exception $e) { return 0; } } /** - * gets the primary group of a user - * - * @param string $dn - * @return string + * @return string|bool + * @throws ServerNotAvailableException */ - public function getUserPrimaryGroup($dn) { + public function getUserPrimaryGroup(string $dn) { $groupID = $this->getUserPrimaryGroupIDs($dn); if ($groupID !== false) { $groupName = $this->primaryGroupID2Name($groupID, $dn); @@ -650,15 +599,15 @@ 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 - * * This function fetches all groups a user belongs to. It does not check * if the user exists at all. * * This function includes groups based on dynamic group membership. + * + * @param string $uid Name of the user + * @return array with group names + * @throws Exception + * @throws ServerNotAvailableException */ public function getUserGroups($uid) { if (!$this->enabled) { @@ -709,8 +658,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } } } else { - \OCP\Util::writeLog('user_ldap', 'No search filter found on member url ' . - 'of group ' . print_r($dynamicGroup, true), ILogger::DEBUG); + $this->logger->debug('No search filter found on member url of group {dn}', + [ + 'app' => 'user_ldap', + 'dn' => $dynamicGroup, + ] + ); } } } @@ -752,8 +705,13 @@ 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 ' . - $this->access->connection->ldapHost, ILogger::DEBUG); + $this->logger->debug('No uid attribute found for DN {dn} on {host}', + [ + 'app' => 'user_ldap', + 'dn' => $userDN, + 'host' => $this->access->connection->ldapHost, + ] + ); $uid = false; } else { $uid = $result[0]; @@ -788,11 +746,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * @param string $dn - * @param array|null &$seen - * @return array + * @throws ServerNotAvailableException */ - private function getGroupsByMember($dn, &$seen = null) { + private function getGroupsByMember(string $dn, array &$seen = null): array { if ($seen === null) { $seen = []; } @@ -826,7 +782,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @param int $limit * @param int $offset * @return array with user ids - * @throws \Exception + * @throws Exception + * @throws ServerNotAvailableException */ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { if (!$this->enabled) { @@ -934,6 +891,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @param string $gid the internal group name * @param string $search optional, a search string * @return int|bool + * @throws Exception + * @throws ServerNotAvailableException */ public function countUsersInGroup($gid, $search = '') { if ($this->groupPluginManager->implementsActions(GroupInterface::COUNT_USERS)) { @@ -1003,8 +962,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD continue; } // dn2username will also check if the users belong to the allowed base - if ($ocname = $this->access->dn2username($member)) { - $groupUsers[] = $ocname; + if ($ncGroupId = $this->access->dn2username($member)) { + $groupUsers[] = $ncGroupId; } } } @@ -1027,6 +986,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * Uses a paged search if available to override a * server side search limit. * (active directory has a limit of 1000 by default) + * @throws Exception */ public function getGroups($search = '', $limit = -1, $offset = 0) { if (!$this->enabled) { @@ -1035,7 +995,6 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $cacheKey = 'getGroups-' . $search . '-' . $limit . '-' . $offset; //Check cache before driving unnecessary searches - \OCP\Util::writeLog('user_ldap', 'getGroups ' . $cacheKey, ILogger::DEBUG); $ldap_groups = $this->access->connection->getFromCache($cacheKey); if (!is_null($ldap_groups)) { return $ldap_groups; @@ -1050,7 +1009,6 @@ 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); $ldap_groups = $this->access->fetchListOfGroups($filter, [$this->access->connection->ldapGroupDisplayName, 'dn'], $limit, @@ -1062,18 +1020,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * @param string $group - * @return bool - */ - public function groupMatchesFilter($group) { - return (strripos($group, $this->groupSearch) !== false); - } - - /** * check if a group exists * * @param string $gid * @return bool + * @throws ServerNotAvailableException */ public function groupExists($gid) { $groupExists = $this->access->connection->getFromCache('groupExists' . $gid); @@ -1094,7 +1045,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return false; } - //if group really still exists, we will be able to read its objectclass + //if group really still exists, we will be able to read its objectClass if (!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapGroupFilter))) { $this->access->connection->writeToCache('groupExists' . $gid, false); return false; @@ -1104,6 +1055,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD return true; } + /** + * @throws ServerNotAvailableException + * @throws Exception + */ protected function filterValidGroups(array $listOfGroups): array { $validGroupDNs = []; foreach ($listOfGroups as $key => $item) { @@ -1147,7 +1102,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * * @param string $gid * @return bool - * @throws \Exception + * @throws Exception + * @throws ServerNotAvailableException */ public function createGroup($gid) { if ($this->groupPluginManager->implementsActions(GroupInterface::CREATE_GROUP)) { @@ -1167,7 +1123,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } return $dn != null; } - throw new \Exception('Could not create group in LDAP backend.'); + throw new Exception('Could not create group in LDAP backend.'); } /** @@ -1175,7 +1131,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * * @param string $gid gid of the group to delete * @return bool - * @throws \Exception + * @throws Exception */ public function deleteGroup($gid) { if ($this->groupPluginManager->implementsActions(GroupInterface::DELETE_GROUP)) { @@ -1186,7 +1142,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } return $ret; } - throw new \Exception('Could not delete group in LDAP backend.'); + throw new Exception('Could not delete group in LDAP backend.'); } /** @@ -1195,7 +1151,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @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 - * @throws \Exception + * @throws Exception */ public function addToGroup($uid, $gid) { if ($this->groupPluginManager->implementsActions(GroupInterface::ADD_TO_GROUP)) { @@ -1205,7 +1161,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } return $ret; } - throw new \Exception('Could not add user to group in LDAP backend.'); + throw new Exception('Could not add user to group in LDAP backend.'); } /** @@ -1214,7 +1170,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @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 - * @throws \Exception + * @throws Exception */ public function removeFromGroup($uid, $gid) { if ($this->groupPluginManager->implementsActions(GroupInterface::REMOVE_FROM_GROUP)) { @@ -1224,21 +1180,21 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } return $ret; } - throw new \Exception('Could not remove user from group in LDAP backend.'); + throw new Exception('Could not remove user from group in LDAP backend.'); } /** * Gets group details * * @param string $gid Name of the group - * @return array | false - * @throws \Exception + * @return array|false + * @throws Exception */ public function getGroupDetails($gid) { if ($this->groupPluginManager->implementsActions(GroupInterface::GROUP_DETAILS)) { return $this->groupPluginManager->getGroupDetails($gid); } - throw new \Exception('Could not get group details in LDAP backend.'); + throw new Exception('Could not get group details in LDAP backend.'); } /** @@ -1248,6 +1204,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * * @param string $gid * @return resource of the LDAP connection + * @throws ServerNotAvailableException */ public function getNewLDAPConnection($gid) { $connection = clone $this->access->getConnection(); @@ -1255,7 +1212,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD } /** - * @throws \OC\ServerNotAvailableException + * @throws ServerNotAvailableException */ public function getDisplayName(string $gid): string { if ($this->groupPluginManager instanceof IGetDisplayNameBackend) { diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 60b8b06b3cf..f8dd5af167f 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -36,8 +36,10 @@ use OCA\User_LDAP\Connection; use OCA\User_LDAP\Group_LDAP as GroupLDAP; use OCA\User_LDAP\GroupPluginManager; use OCA\User_LDAP\ILDAPWrapper; +use OCA\User_LDAP\Mapping\GroupMapping; use OCA\User_LDAP\User\Manager; use OCP\GroupInterface; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** @@ -49,18 +51,18 @@ use Test\TestCase; */ class Group_LDAPTest extends TestCase { /** - * @return \PHPUnit_Framework_MockObject_MockObject|Access + * @return MockObject|Access */ private function getAccessMock() { static $conMethods; static $accMethods; if (is_null($conMethods) || is_null($accMethods)) { - $conMethods = get_class_methods('\OCA\User_LDAP\Connection'); - $accMethods = get_class_methods('\OCA\User_LDAP\Access'); + $conMethods = get_class_methods(Connection::class); + $accMethods = get_class_methods(Access::class); } $lw = $this->createMock(ILDAPWrapper::class); - $connector = $this->getMockBuilder('\OCA\User_LDAP\Connection') + $connector = $this->getMockBuilder(Connection::class) ->setMethods($conMethods) ->setConstructorArgs([$lw, null, null]) ->getMock(); @@ -74,14 +76,14 @@ class Group_LDAPTest extends TestCase { return $access; } + /** + * @return MockObject|GroupPluginManager + */ private function getPluginManagerMock() { - return $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')->getMock(); + return $this->createMock(GroupPluginManager::class); } - /** - * @param Access|\PHPUnit_Framework_MockObject_MockObject $access - */ - private function enableGroups($access) { + private function enableGroups(Access $access) { $access->connection = $this->createMock(Connection::class); $access->connection->expects($this->any()) @@ -104,7 +106,6 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('groupname2dn') ->willReturn($groupDN); - $access->expects($this->any()) ->method('readAttribute') ->willReturnCallback(function ($dn) use ($groupDN) { @@ -121,7 +122,6 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('isDNPartOfBase') ->willReturn(true); - // for primary groups $access->expects($this->once()) ->method('countUsers') @@ -166,6 +166,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('isDNPartOfBase') ->willReturn(true); + $access->expects($this->any()) + ->method('escapeFilterPart') + ->willReturnArgument(0); $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('group', '3'); @@ -174,8 +177,8 @@ class Group_LDAPTest extends TestCase { } public function testCountUsersWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'countUsersInGroup']) ->getMock(); @@ -642,11 +645,9 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('username2dn') ->willReturn($dn); - $access->expects($this->exactly(5)) ->method('readAttribute') ->will($this->onConsecutiveCalls(['cn=groupA,dc=foobar', 'cn=groupB,dc=foobar'], [], [], [], [])); - $access->expects($this->any()) ->method('dn2groupname') ->willReturnArgument(0); @@ -776,8 +777,8 @@ class Group_LDAPTest extends TestCase { } public function testCreateGroupWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'createGroup']) ->getMock(); @@ -803,8 +804,8 @@ class Group_LDAPTest extends TestCase { public function testCreateGroupFailing() { $this->expectException(\Exception::class); - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'createGroup']) ->getMock(); @@ -822,8 +823,8 @@ class Group_LDAPTest extends TestCase { } public function testDeleteGroupWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'deleteGroup']) ->getMock(); @@ -837,7 +838,7 @@ class Group_LDAPTest extends TestCase { ->with('gid') ->willReturn('result'); - $mapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\GroupMapping') + $mapper = $this->getMockBuilder(GroupMapping::class) ->setMethods(['unmap']) ->disableOriginalConstructor() ->getMock(); @@ -858,8 +859,8 @@ class Group_LDAPTest extends TestCase { public function testDeleteGroupFailing() { $this->expectException(\Exception::class); - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'deleteGroup']) ->getMock(); @@ -877,8 +878,8 @@ class Group_LDAPTest extends TestCase { } public function testAddToGroupWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'addToGroup']) ->getMock(); @@ -904,8 +905,8 @@ class Group_LDAPTest extends TestCase { public function testAddToGroupFailing() { $this->expectException(\Exception::class); - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'addToGroup']) ->getMock(); @@ -923,8 +924,8 @@ class Group_LDAPTest extends TestCase { } public function testRemoveFromGroupWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'removeFromGroup']) ->getMock(); @@ -950,8 +951,8 @@ class Group_LDAPTest extends TestCase { public function testRemoveFromGroupFailing() { $this->expectException(\Exception::class); - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'removeFromGroup']) ->getMock(); @@ -969,8 +970,8 @@ class Group_LDAPTest extends TestCase { } public function testGetGroupDetailsWithPlugin() { - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'getGroupDetails']) ->getMock(); @@ -996,8 +997,8 @@ class Group_LDAPTest extends TestCase { public function testGetGroupDetailsFailing() { $this->expectException(\Exception::class); - /** @var GroupPluginManager|\PHPUnit_Framework_MockObject_MockObject $pluginManager */ - $pluginManager = $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager') + /** @var GroupPluginManager|MockObject $pluginManager */ + $pluginManager = $this->getMockBuilder(GroupPluginManager::class) ->setMethods(['implementsActions', 'getGroupDetails']) ->getMock(); |