Skip to content

Commit 3d6df9b

Browse files
blizzzbackportbot[bot]
authored andcommitted
fix detecting cyclic group memberships
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
1 parent b9d417f commit 3d6df9b

1 file changed

Lines changed: 19 additions & 8 deletions

File tree

apps/user_ldap/lib/Group_LDAP.php

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,9 @@ public function getDynamicGroupMembers($dnGroup) {
223223
private function _groupMembers($dnGroup, &$seen = null) {
224224
if ($seen === null) {
225225
$seen = [];
226+
// the root entry has to be marked as processed to avoind infinit loops,
227+
// but not included in the results laters on
228+
$excludeFromResult = $dnGroup;
226229
}
227230
$allMembers = [];
228231
if (array_key_exists($dnGroup, $seen)) {
@@ -269,13 +272,19 @@ private function _groupMembers($dnGroup, &$seen = null) {
269272
$seen[$dnGroup] = 1;
270273
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
271274
if (is_array($members)) {
272-
$fetcher = function ($memberDN, &$seen) {
275+
$fetcher = function ($memberDN) use (&$seen) {
273276
return $this->_groupMembers($memberDN, $seen);
274277
};
275-
$allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members);
278+
$allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members, $seen);
276279
}
277280

278281
$allMembers += $this->getDynamicGroupMembers($dnGroup);
282+
if (isset($excludeFromResult)) {
283+
$index = array_search($excludeFromResult, $allMembers, true);
284+
if ($index !== false) {
285+
unset($allMembers[$index]);
286+
}
287+
}
279288

280289
$this->access->connection->writeToCache($cacheKey, $allMembers);
281290
if (isset($attemptedLdapMatchingRuleInChain)
@@ -342,19 +351,21 @@ private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): a
342351
return $list;
343352
}
344353

345-
$seen = [];
346354
while ($record = array_shift($list)) {
347-
$recordDN = $recordMode ? $record['dn'][0] : $record;
355+
$recordDN = $record['dn'][0] ?? $record;
348356
if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
349357
// Prevent loops
350358
continue;
351359
}
352-
$fetched = $fetcher($record, $seen);
360+
$fetched = $fetcher($record);
353361
$list = array_merge($list, $fetched);
354-
$seen[$recordDN] = $record;
362+
if (!isset($seen[$recordDN]) || is_bool($seen[$recordDN]) && is_array($record)) {
363+
$seen[$recordDN] = $record;
364+
}
355365
}
356366

357-
return $recordMode ? $seen : array_keys($seen);
367+
// on record mode, filter out intermediate state
368+
return $recordMode ? array_filter($seen, 'is_array') : array_keys($seen);
358369
}
359370

360371
/**
@@ -850,7 +861,7 @@ private function getGroupsByMember($dn, &$seen = null) {
850861
$groups = $this->access->fetchListOfGroups($filter,
851862
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
852863
if (is_array($groups)) {
853-
$fetcher = function ($dn, &$seen) {
864+
$fetcher = function ($dn) use (&$seen) {
854865
if (is_array($dn) && isset($dn['dn'][0])) {
855866
$dn = $dn['dn'][0];
856867
}

0 commit comments

Comments
 (0)