Merge pull request #23571 from nextcloud/backport/23566/stable19
[stable19] LDAP: when nesting is not enabled, the group filter can be applied right away
This commit is contained in:
commit
ecadc64abb
|
@ -140,11 +140,7 @@ class Connection extends LDAPUtility {
|
||||||
$this->dontDestruct = true;
|
$this->dontDestruct = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
public function __get(string $name) {
|
||||||
* @param string $name
|
|
||||||
* @return bool|mixed
|
|
||||||
*/
|
|
||||||
public function __get($name) {
|
|
||||||
if (!$this->configured) {
|
if (!$this->configured) {
|
||||||
$this->readConfiguration();
|
$this->readConfiguration();
|
||||||
}
|
}
|
||||||
|
|
|
@ -343,7 +343,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
|
||||||
}
|
}
|
||||||
|
|
||||||
$seen = [];
|
$seen = [];
|
||||||
while ($record = array_pop($list)) {
|
while ($record = array_shift($list)) {
|
||||||
$recordDN = $recordMode ? $record['dn'][0] : $record;
|
$recordDN = $recordMode ? $record['dn'][0] : $record;
|
||||||
if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
|
if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
|
||||||
// Prevent loops
|
// Prevent loops
|
||||||
|
@ -841,6 +841,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
|
||||||
$allGroups = [];
|
$allGroups = [];
|
||||||
$seen[$dn] = true;
|
$seen[$dn] = true;
|
||||||
$filter = $this->access->connection->ldapGroupMemberAssocAttr . '=' . $dn;
|
$filter = $this->access->connection->ldapGroupMemberAssocAttr . '=' . $dn;
|
||||||
|
|
||||||
|
$nesting = (int)$this->access->connection->ldapNestedGroups;
|
||||||
|
if ($nesting === 0) {
|
||||||
|
$filter = $this->access->combineFilterWithAnd([$filter, $this->access->connection->ldapGroupFilter]);
|
||||||
|
}
|
||||||
|
|
||||||
$groups = $this->access->fetchListOfGroups($filter,
|
$groups = $this->access->fetchListOfGroups($filter,
|
||||||
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
|
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
|
||||||
if (is_array($groups)) {
|
if (is_array($groups)) {
|
||||||
|
|
|
@ -727,23 +727,36 @@ class Group_LDAPTest extends TestCase {
|
||||||
$groupBackend->getUserGroups('userX');
|
$groupBackend->getUserGroups('userX');
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetGroupsByMember() {
|
public function nestedGroupsProvider(): array {
|
||||||
|
return [
|
||||||
|
[ true ],
|
||||||
|
[ false ],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider nestedGroupsProvider
|
||||||
|
*/
|
||||||
|
public function testGetGroupsByMember(bool $nestedGroups) {
|
||||||
$access = $this->getAccessMock();
|
$access = $this->getAccessMock();
|
||||||
$pluginManager = $this->getPluginManagerMock();
|
$pluginManager = $this->getPluginManagerMock();
|
||||||
|
|
||||||
|
$groupFilter = '(&(objectclass=nextcloudGroup)(nextcloudEnabled=TRUE))';
|
||||||
$access->connection = $this->createMock(Connection::class);
|
$access->connection = $this->createMock(Connection::class);
|
||||||
$access->connection->expects($this->any())
|
$access->connection->expects($this->any())
|
||||||
->method('__get')
|
->method('__get')
|
||||||
->willReturnCallback(function ($name) {
|
->willReturnCallback(function ($name) use ($nestedGroups, $groupFilter) {
|
||||||
switch ($name) {
|
switch ($name) {
|
||||||
case 'useMemberOfToDetectMembership':
|
case 'useMemberOfToDetectMembership':
|
||||||
return 0;
|
return 0;
|
||||||
case 'ldapDynamicGroupMemberURL':
|
case 'ldapDynamicGroupMemberURL':
|
||||||
return '';
|
return '';
|
||||||
case 'ldapNestedGroups':
|
case 'ldapNestedGroups':
|
||||||
return false;
|
return $nestedGroups;
|
||||||
case 'ldapGroupMemberAssocAttr':
|
case 'ldapGroupMemberAssocAttr':
|
||||||
return 'member';
|
return 'member';
|
||||||
|
case 'ldapGroupFilter':
|
||||||
|
return $groupFilter;
|
||||||
}
|
}
|
||||||
return 1;
|
return 1;
|
||||||
});
|
});
|
||||||
|
@ -756,10 +769,15 @@ class Group_LDAPTest extends TestCase {
|
||||||
$access->expects($this->exactly(2))
|
$access->expects($this->exactly(2))
|
||||||
->method('username2dn')
|
->method('username2dn')
|
||||||
->willReturn($dn);
|
->willReturn($dn);
|
||||||
|
|
||||||
$access->expects($this->any())
|
$access->expects($this->any())
|
||||||
->method('readAttribute')
|
->method('readAttribute')
|
||||||
->willReturn([]);
|
->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 = [
|
$group1 = [
|
||||||
'cn' => 'group1',
|
'cn' => 'group1',
|
||||||
|
@ -774,9 +792,21 @@ class Group_LDAPTest extends TestCase {
|
||||||
->method('nextcloudGroupNames')
|
->method('nextcloudGroupNames')
|
||||||
->with([$group1, $group2])
|
->with([$group1, $group2])
|
||||||
->willReturn(['group1', 'group2']);
|
->willReturn(['group1', 'group2']);
|
||||||
$access->expects($this->once())
|
$access->expects($nestedGroups ? $this->atLeastOnce() : $this->once())
|
||||||
->method('fetchListOfGroups')
|
->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())
|
$access->expects($this->any())
|
||||||
->method('dn2groupname')
|
->method('dn2groupname')
|
||||||
->willReturnCallback(function (string $dn) {
|
->willReturnCallback(function (string $dn) {
|
||||||
|
|
Loading…
Reference in New Issue