Skip to content

Commit

Permalink
Merge pull request #30340 from nextcloud/backport/29329/stable22
Browse files Browse the repository at this point in the history
[stable22] fix potential unwarranted memberships in nested groups from LDAP
  • Loading branch information
artonge authored Dec 30, 2021
2 parents 2451f65 + ad0ea2c commit 7f82061
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions apps/user_ldap/lib/Group_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
// but not included in the results laters on
$excludeFromResult = $dnGroup;
}
// cache only base groups, otherwise groups get additional unwarranted members
$shouldCacheResult = count($seen) === 0;

static $rawMemberReads = []; // runtime cache for intermediate ldap read results
$allMembers = [];

if (array_key_exists($dnGroup, $seen)) {
return [];
}
Expand Down Expand Up @@ -290,7 +295,11 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
}

$seen[$dnGroup] = 1;
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
$members = $rawMemberReads[$dnGroup] ?? null;
if ($members === null) {
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
$rawMemberReads[$dnGroup] = $members;
}
if (is_array($members)) {
$fetcher = function ($memberDN) use (&$seen) {
return $this->_groupMembers($memberDN, $seen);
Expand All @@ -306,7 +315,10 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
}
}

$this->access->connection->writeToCache($cacheKey, $allMembers);
if ($shouldCacheResult) {
$this->access->connection->writeToCache($cacheKey, $allMembers);
unset($rawMemberReads[$dnGroup]);
}
if (isset($attemptedLdapMatchingRuleInChain)
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN
&& !empty($allMembers)
Expand Down

0 comments on commit 7f82061

Please sign in to comment.