diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index cb40e3d7e06a1..811efd5951944 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -878,10 +878,18 @@ public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null, 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); - if(is_string($uid)) { + $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); } return ($uid !== false) && ($newlyMapped || $isBackgroundJobModeAjax); @@ -932,9 +940,19 @@ public function batchApplyUserAttributes(array $ldapRecords){ */ 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); } diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index 472cd8d277965..3b49fcd1c6814 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -25,8 +25,11 @@ namespace OCA\User_LDAP\Mapping; +use OC\DB\QueryBuilder\QueryBuilder; + /** * Class AbstractMapping +* * @package OCA\User_LDAP\Mapping */ abstract class AbstractMapping { @@ -39,7 +42,7 @@ 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 @@ -48,6 +51,9 @@ public function __construct(\OCP\IDBConnection $dbc) { $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 @@ -110,7 +116,12 @@ protected function modify($query, $parameters) { * @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; } /** @@ -120,13 +131,21 @@ public function getDNByName($name) { * @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, array($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; } /** @@ -145,6 +164,8 @@ public function setUUIDbyDN($uuid, $fdn) { WHERE `ldap_dn` = ? '); + unset($this->cache[$fdn]); + return $this->modify($query, [$uuid, $fdn]); } @@ -154,7 +175,27 @@ public function setUUIDbyDN($uuid, $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; } /** @@ -190,6 +231,10 @@ public function getNameByUUID($uuid) { 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 @@ -248,6 +293,9 @@ public function map($fdn, $name, $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) { diff --git a/apps/user_ldap/lib/Mapping/GroupMapping.php b/apps/user_ldap/lib/Mapping/GroupMapping.php index 6922a010533a0..1ecc6346fb5ac 100644 --- a/apps/user_ldap/lib/Mapping/GroupMapping.php +++ b/apps/user_ldap/lib/Mapping/GroupMapping.php @@ -33,8 +33,9 @@ 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 9fde960959f68..b87f98affa0eb 100644 --- a/apps/user_ldap/lib/Mapping/UserMapping.php +++ b/apps/user_ldap/lib/Mapping/UserMapping.php @@ -33,8 +33,9 @@ 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/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 7ea4cb463d96f..70a60d73bf55d 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -78,7 +78,7 @@ private function getAccessMock() { private function getPluginManagerMock() { return $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')->getMock(); } - + /** * @param Access|\PHPUnit_Framework_MockObject_MockObject $access */ @@ -119,6 +119,9 @@ public function testCountEmptySearchString() { } return []; }); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); // for primary groups $access->expects($this->once()) @@ -140,11 +143,9 @@ public function testCountWithSearchString() { $access->expects($this->any()) ->method('groupname2dn') ->will($this->returnValue('cn=group,dc=foo,dc=bar')); - $access->expects($this->any()) ->method('fetchListOfUsers') ->will($this->returnValue([])); - $access->expects($this->any()) ->method('readAttribute') ->will($this->returnCallback(function($name) { @@ -158,12 +159,14 @@ public function testCountWithSearchString() { } return ['u11', 'u22', 'u33', 'u34']; })); - $access->expects($this->any()) ->method('dn2username') ->will($this->returnCallback(function() { return 'foobar' . \OC::$server->getSecureRandom()->generate(7); })); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $groupBackend = new GroupLDAP($access,$pluginManager); $users = $groupBackend->countUsersInGroup('group', '3'); @@ -193,7 +196,7 @@ public function testCountUsersWithPlugin() { $ldap = new GroupLDAP($access, $pluginManager); $this->assertEquals($ldap->countUsersInGroup('gid', 'search'),42); - } + } public function testGidNumber2NameSuccess() { $access = $this->getAccessMock(); @@ -533,6 +536,9 @@ public function testUsersInGroupPrimaryMembersOnly() { $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); @@ -568,6 +574,9 @@ public function testUsersInGroupPrimaryAndUnixMembers() { $access->expects($this->once()) ->method('nextcloudUserNames') ->will($this->returnValue(array('lisa', 'bart', 'kira', 'brad'))); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $access->userManager = $this->createMock(Manager::class); $groupBackend = new GroupLDAP($access, $pluginManager); @@ -598,14 +607,15 @@ public function testCountUsersInGroupPrimaryMembersOnly() { } return array(); })); - $access->expects($this->any()) ->method('groupname2dn') ->will($this->returnValue('cn=foobar,dc=foo,dc=bar')); - $access->expects($this->once()) ->method('countUsers') ->will($this->returnValue(4)); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('foobar'); @@ -628,17 +638,19 @@ public function testGetUserGroupsMemberOf() { ->method('username2dn') ->will($this->returnValue($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') ->will($this->returnArgument(0)); - - $access->expects($this->exactly(1)) - ->method('groupsMatchFilter') - ->will($this->returnArgument(0)); + $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'); @@ -676,9 +688,6 @@ public function testGetUserGroupsMemberOfDisabled() { $access->expects($this->once()) ->method('nextcloudGroupNames') ->will($this->returnValue([])); - $access->expects($this->any()) - ->method('groupsMatchFilter') - ->willReturnArgument(0); $groupBackend = new GroupLDAP($access, $pluginManager); $groupBackend->getUserGroups('userX'); @@ -714,9 +723,9 @@ public function testGetGroupsByMember() { ->method('username2dn') ->will($this->returnValue($dn)); - $access->expects($this->never()) + $access->expects($this->any()) ->method('readAttribute') - ->with($dn, 'memberOf'); + ->willReturn([]); $group1 = [ 'cn' => 'group1', @@ -735,8 +744,23 @@ public function testGetGroupsByMember() { ->method('fetchListOfGroups') ->will($this->returnValue([$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'); @@ -770,7 +794,7 @@ public function testCreateGroupWithPlugin() { $this->assertEquals($ldap->createGroup('gid'),true); } - + public function testCreateGroupFailing() { $this->expectException(\Exception::class); @@ -825,7 +849,7 @@ public function testDeleteGroupWithPlugin() { $this->assertEquals($ldap->deleteGroup('gid'),'result'); } - + public function testDeleteGroupFailing() { $this->expectException(\Exception::class); @@ -871,7 +895,7 @@ public function testAddToGroupWithPlugin() { $this->assertEquals($ldap->addToGroup('uid', 'gid'),'result'); } - + public function testAddToGroupFailing() { $this->expectException(\Exception::class); @@ -917,7 +941,7 @@ public function testRemoveFromGroupWithPlugin() { $this->assertEquals($ldap->removeFromGroup('uid', 'gid'),'result'); } - + public function testRemoveFromGroupFailing() { $this->expectException(\Exception::class); @@ -963,7 +987,7 @@ public function testGetGroupDetailsWithPlugin() { $this->assertEquals($ldap->getGroupDetails('gid'),'result'); } - + public function testGetGroupDetailsFailing() { $this->expectException(\Exception::class);