Fix LDAP recursive nested group support

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2022-07-26 09:39:48 +02:00 committed by Carl Schwan
parent be5338e572
commit 746a5fb7e0
2 changed files with 21 additions and 19 deletions

View file

@ -62,7 +62,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
protected CappedMemoryCache $cachedGroupsByMember;
/** @var CappedMemoryCache<string[]> $cachedNestedGroups array of groups with gid (DN) as key */
protected CappedMemoryCache $cachedNestedGroups;
protected GroupInterface $groupPluginManager;
protected GroupPluginManager $groupPluginManager;
protected LoggerInterface $logger;
/**
@ -243,8 +243,9 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
* @psalm-param array<string, bool> $seen List of DN that have already been processed.
* @throws ServerNotAvailableException
*/
private function _groupMembers(string $dnGroup, array &$seen = []): array {
private function _groupMembers(string $dnGroup, array $seen = [], bool &$recursive = false): array {
if (isset($seen[$dnGroup])) {
$recursive = true;
return [];
}
$seen[$dnGroup] = true;
@ -293,7 +294,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
if (is_array($members)) {
if ((int)$this->access->connection->ldapNestedGroups === 1) {
while ($recordDn = array_shift($members)) {
$nestedMembers = $this->_groupMembers($recordDn, $seen);
$nestedMembers = $this->_groupMembers($recordDn, $seen, $recursive);
if (!empty($nestedMembers)) {
// Group, queue its members for processing
$members = array_merge($members, $nestedMembers);
@ -317,7 +318,9 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
unset($allMembers[$index]);
}
$this->access->connection->writeToCache($cacheKey, $allMembers);
if (!$recursive) {
$this->access->connection->writeToCache($cacheKey, $allMembers);
}
if (isset($attemptedLdapMatchingRuleInChain)
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN

View file

@ -1322,35 +1322,32 @@ class Group_LDAPTest extends TestCase {
return [
[ #0 test DNs
'cn=Birds,' . $base,
$birdsDn,
['cn=Birds,' . $base => $birdsDn],
['cn=Birds,' . $base => $birdsDn]
],
[ #1 test uids
'cn=Birds,' . $base,
$birdsUid,
['cn=Birds,' . $base => $birdsUid],
['cn=Birds,' . $base => $birdsUid]
],
[ #2 test simple nested group
'cn=Animals,' . $base,
array_merge($birdsDn, $animalsDn),
['cn=Animals,' . $base => array_merge($birdsDn, $animalsDn)],
[
'cn=Animals,' . $base => array_merge(['cn=Birds,' . $base], $animalsDn),
'cn=Birds,' . $base => $birdsDn,
]
],
[ #3 test recursive nested group
'cn=Animals,' . $base,
// Because of complicated nesting the group gets returned as a member
array_merge(['cn=Birds,' . $base], $birdsDn, $animalsDn),
[
'cn=Animals,' . $base => array_merge($birdsDn, $animalsDn),
'cn=Birds,' . $base => array_merge($birdsDn, $animalsDn),
],
[
'cn=Animals,' . $base => array_merge(['cn=Birds,' . $base,'cn=Birds,' . $base,'cn=Animals,' . $base], $animalsDn),
'cn=Birds,' . $base => array_merge(['cn=Animals,' . $base,'cn=Birds,' . $base], $birdsDn),
]
],
[ #4 Complicated nested group
'cn=Things,' . $base,
array_merge($birdsDn, $animalsDn, $thingsDn, $plantsDn),
['cn=Things,' . $base => array_merge($birdsDn, $animalsDn, $thingsDn, $plantsDn)],
[
'cn=Animals,' . $base => array_merge(['cn=Birds,' . $base], $animalsDn),
'cn=Birds,' . $base => $birdsDn,
@ -1365,11 +1362,11 @@ class Group_LDAPTest extends TestCase {
* @param string[] $expectedMembers
* @dataProvider groupMemberProvider
*/
public function testGroupMembers(string $groupDN, array $expectedMembers, $groupsInfo = null) {
public function testGroupMembers(array $expectedResult, array $groupsInfo = null) {
$access = $this->getAccessMock();
$access->expects($this->any())
->method('readAttribute')
->willReturnCallback(function ($group) use ($groupDN, $expectedMembers, $groupsInfo) {
->willReturnCallback(function ($group) use ($groupsInfo) {
if (isset($groupsInfo[$group])) {
return $groupsInfo[$group];
}
@ -1392,9 +1389,11 @@ class Group_LDAPTest extends TestCase {
$pluginManager = $this->createMock(GroupPluginManager::class);
$ldap = new GroupLDAP($access, $pluginManager);
$resultingMembers = $this->invokePrivate($ldap, '_groupMembers', [$groupDN]);
foreach ($expectedResult as $groupDN => $expectedMembers) {
$resultingMembers = $this->invokePrivate($ldap, '_groupMembers', [$groupDN]);
$this->assertEqualsCanonicalizing($expectedMembers, $resultingMembers);
$this->assertEqualsCanonicalizing($expectedMembers, $resultingMembers);
}
}
public function displayNameProvider() {