when nesting is not enabled, the group filter can be applied right away

- helps performance, but skipping unnecessary entries
- reduces reoccuring info-level log output against groups that do not
  qualify ("no or empty name")

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2020-10-19 13:17:06 +02:00 committed by backportbot[bot]
parent 97ad590aea
commit fbfac673c9
2 changed files with 42 additions and 7 deletions

View File

@ -344,7 +344,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
}
$seen = [];
while ($record = array_pop($list)) {
while ($record = array_shift($list)) {
$recordDN = $recordMode ? $record['dn'][0] : $record;
if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
// Prevent loops
@ -822,6 +822,11 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
$filter .= '@*';
}
$nesting = (int)$this->access->connection->ldapNestedGroups;
if ($nesting === 0) {
$filter = $this->access->combineFilterWithAnd([$filter, $this->access->connection->ldapGroupFilter]);
}
$groups = $this->access->fetchListOfGroups($filter,
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
if (is_array($groups)) {

View File

@ -719,23 +719,36 @@ class Group_LDAPTest extends TestCase {
$groupBackend->getUserGroups('userX');
}
public function testGetGroupsByMember() {
public function nestedGroupsProvider(): array {
return [
[ true ],
[ false ],
];
}
/**
* @dataProvider nestedGroupsProvider
*/
public function testGetGroupsByMember(bool $nestedGroups) {
$access = $this->getAccessMock();
$pluginManager = $this->getPluginManagerMock();
$groupFilter = '(&(objectclass=nextcloudGroup)(nextcloudEnabled=TRUE))';
$access->connection = $this->createMock(Connection::class);
$access->connection->expects($this->any())
->method('__get')
->willReturnCallback(function ($name) {
->willReturnCallback(function ($name) use ($nestedGroups, $groupFilter) {
switch ($name) {
case 'useMemberOfToDetectMembership':
return 0;
case 'ldapDynamicGroupMemberURL':
return '';
case 'ldapNestedGroups':
return false;
return $nestedGroups;
case 'ldapGroupMemberAssocAttr':
return 'member';
case 'ldapGroupFilter':
return $groupFilter;
}
return 1;
});
@ -748,10 +761,15 @@ class Group_LDAPTest extends TestCase {
$access->expects($this->exactly(2))
->method('username2dn')
->willReturn($dn);
$access->expects($this->any())
->method('readAttribute')
->willReturn([]);
$access->expects($this->any())
->method('combineFilterWithAnd')
->willReturnCallback(function (array $filterParts) {
// ⚠ returns a pseudo-filter only, not real LDAP Filter syntax
return implode('&', $filterParts);
});
$group1 = [
'cn' => 'group1',
@ -766,9 +784,21 @@ class Group_LDAPTest extends TestCase {
->method('nextcloudGroupNames')
->with([$group1, $group2])
->willReturn(['group1', 'group2']);
$access->expects($this->once())
$access->expects($nestedGroups ? $this->atLeastOnce() : $this->once())
->method('fetchListOfGroups')
->willReturn([$group1, $group2]);
->willReturnCallback(function ($filter, $attr, $limit, $offset) use ($nestedGroups, $groupFilter, $group1, $group2) {
static $firstRun = true;
if (!$nestedGroups) {
// When nested groups are enabled, groups cannot be filtered early as it would
// exclude intermediate groups. But we can, and should, when working with flat groups.
$this->assertTrue(strpos($filter, $groupFilter) !== false);
}
if ($firstRun) {
$firstRun = false;
return [$group1, $group2];
}
return [];
});
$access->expects($this->any())
->method('dn2groupname')
->willReturnCallback(function (string $dn) {