Skip to content

Commit

Permalink
Fix caching of the result of the fetcher
Browse files Browse the repository at this point in the history 
We have two fetcher methods that return different results, so use a
different cache key for each one of them. This fix instabilities
regarding the group membership

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
@CarlSchwan
CarlSchwan committed Feb 19, 2022
1 parent aca5c5e commit 682a6ac
Showing 1 changed file with 20 additions and 18 deletions.
38 changes: 20 additions & 18 deletions apps/user_ldap/lib/Group_LDAP.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
$filter,
$this->access->userManager->getAttributes(true)
);
$result = array_reduce($memberRecords, function ($carry, $record) {
$result = array_reduce($memberRecords, function ($carry, $record): array {
$carry[] = $record['dn'][0];
return $carry;
}, []);
Expand All @@ -305,7 +305,7 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
$rawMemberReads[$dnGroup] = $members;
}
if (is_array($members)) {
$fetcher = function (string $memberDN) use (&$seen) {
$fetcher = function (string $memberDN) use (&$seen): array {
return $this->_groupMembers($memberDN, $seen);
};
$allMembers = $this->walkNestedGroupsReturnDNs($dnGroup, $fetcher, $members, $seen);
Expand Down Expand Up @@ -343,7 +343,7 @@ private function _getGroupDNsFromMemberOf(string $dn): array {
return [];
}

$fetcher = function (string $groupDN) {
$fetcher = function (string $groupDN): array {
if (isset($this->cachedNestedGroups[$groupDN])) {
$nestedGroups = $this->cachedNestedGroups[$groupDN];
} else {
Expand All @@ -363,22 +363,20 @@ private function _getGroupDNsFromMemberOf(string $dn): array {
/**
* @psalm-param list<array{dn: list<string>}|string> $list
* @psalm-param array<string, int|array|string> $seen List of DN that have already been processed.
* @param Closure(string) $fetcher
* @psalm-param array<string, int|array|string> $groupDns List of group DNs
* @param Closure(string):array $fetcher
*/
private function processListFromWalkingNestedGroups(array &$list, array &$seen, string $dn, Closure $fetcher): void {
while ($record = array_shift($list)) {
/** @var string $recordDN */
$recordDN = $record['dn'][0] ?? $record;
if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
// Prevent loops
continue;
}

$cacheKey = 'walkNestedGroups_' . $recordDN;
$fetched = $this->access->connection->getFromCache($cacheKey);
if ($fetched === null) {
$fetched = $fetcher($recordDN);
$this->access->connection->writeToCache($cacheKey, $fetched);
}
$fetched = $fetcher($recordDN);

$list = array_merge($list, $fetched);
if (!isset($seen[$recordDN]) || is_bool($seen[$recordDN]) && is_array($record)) {
$seen[$recordDN] = $record;
Expand All @@ -389,7 +387,7 @@ private function processListFromWalkingNestedGroups(array &$list, array &$seen,
/**
* @psalm-param list<array{dn: list<string>}|string> $list
* @psalm-param array<string, int|array|string> $seen List of DN that have already been processed.
* @param Closure(string) $fetcher
* @param Closure(string):array $fetcher
*/
private function walkNestedGroupsReturnDNs(string $dn, Closure $fetcher, array $list, array &$seen = []): array {
$nesting = (int)$this->access->connection->ldapNestedGroups;
Expand All @@ -406,14 +404,14 @@ private function walkNestedGroupsReturnDNs(string $dn, Closure $fetcher, array $
* @psalm-param list<array{dn: list<string>}> $list
* @psalm-param array<string, int|array|string> $seen List of DN that have already been processed.
* @return array[] An array of records
* @param Closure(string) $fetcher
* @param Closure(string):array $fetcher
*/
private function walkNestedGroupsReturnRecords(string $dn, Closure $fetcher, array $list, array &$seen = []): array {
$nesting = (int)$this->access->connection->ldapNestedGroups;

if ($nesting !== 1) {
// the keys are numeric, but should hold the DN
return array_reduce($list, function (array $transformed, array $record) use ($dn) {
return array_reduce($list, function (array $transformed, array $record) use ($dn): array {
if ($record['dn'][0] != $dn) {
$transformed[$record['dn'][0]] = $record;
}
Expand Down Expand Up @@ -469,6 +467,10 @@ private function getNameOfGroup(string $filter, string $cacheKey): ?string {

$this->access->connection->writeToCache($cacheKey, $name);

if ($name === false) {
return null;
}

return $name;
}

Expand Down Expand Up @@ -896,14 +898,14 @@ private function getGroupsByMember(string $dn, array &$seen = null): array {
$groups = $this->access->fetchListOfGroups($filter,
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
if (is_array($groups)) {
$fetcher = function (string $dn) use (&$seen) {
return $this->getGroupsByMember($dn, $seen);
};

if (empty($dn)) {
$dn = "";
}

$fetcher = function (string $dn) use (&$seen): array {
return $this->getGroupsByMember($dn, $seen);
};

$allGroups = $this->walkNestedGroupsReturnRecords($dn, $fetcher, $groups, $seen);
}
$visibleGroups = $this->filterValidGroups($allGroups);
Expand All @@ -913,7 +915,7 @@ private function getGroupsByMember(string $dn, array &$seen = null): array {
}

/**
* get a list of all users in a group
* Get a list of all users in a group
*
* @param string $gid
* @param string $search
Expand Down

0 comments on commit 682a6ac

Please sign in to comment.