Skip to content

Commit

Permalink
Only cache base inGroup search
Browse files Browse the repository at this point in the history
And not intermediate search for nested groups, this is causing issues
othewise with nested groups

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
CarlSchwan committed Jul 18, 2022
1 parent 3b11dce commit 07dec64
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions apps/user_ldap/lib/Group_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,23 @@
namespace OCA\User_LDAP;

use Exception;
use OC;
use OCP\Cache\CappedMemoryCache;
use OC\ServerNotAvailableException;
use OCP\Cache\CappedMemoryCache;
use OCP\Group\Backend\IGetDisplayNameBackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\GroupInterface;
use Psr\Log\LoggerInterface;

class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
protected $enabled = false;
protected bool $enabled = false;

/** @var CappedMemoryCache<string[]> $cachedGroupMembers array of users with gid as key */
protected CappedMemoryCache $cachedGroupMembers;
/** @var CappedMemoryCache<string[]> $cachedGroupsByMember array of groups with uid as key */
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;

/**
Expand All @@ -82,7 +81,7 @@ public function __construct(Access $access, GroupPluginManager $groupPluginManag
$this->cachedGroupsByMember = new CappedMemoryCache();
$this->cachedNestedGroups = new CappedMemoryCache();
$this->groupPluginManager = $groupPluginManager;
$this->logger = OC::$server->get(LoggerInterface::class);
$this->logger = \OCP\Server::get(LoggerInterface::class);
$this->ldapGroupMemberAssocAttr = strtolower((string)$gAssoc);
}

Expand All @@ -91,11 +90,10 @@ public function __construct(Access $access, GroupPluginManager $groupPluginManag
*
* @param string $uid uid of the user
* @param string $gid gid of the group
* @return bool
* @throws Exception
* @throws ServerNotAvailableException
*/
public function inGroup($uid, $gid) {
public function inGroup($uid, $gid): bool {
if (!$this->enabled) {
return false;
}
Expand Down Expand Up @@ -248,6 +246,7 @@ private function _groupMembers(string $dnGroup, array &$seen = []): array {
return [];
}
$seen[$dnGroup] = true;
$shouldCacheResult = count($seen) === 0;

// used extensively in cron job, caching makes sense for nested groups
$cacheKey = '_groupMembers' . $dnGroup;
Expand Down Expand Up @@ -317,7 +316,9 @@ private function _groupMembers(string $dnGroup, array &$seen = []): array {
unset($allMembers[$index]);
}

$this->access->connection->writeToCache($cacheKey, $allMembers);
if ($shouldCacheResult) {
$this->access->connection->writeToCache($cacheKey, $allMembers);
}

if (isset($attemptedLdapMatchingRuleInChain)
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN
Expand Down Expand Up @@ -767,6 +768,12 @@ public function getUserGroups($uid) {
}

if ($uid !== false) {
// Clear cache between invocation of getGroupsByMember
// getGroupsByMember is a recursive method and the results stored in
// the cache depends on the already seen groups. This breaks when we
// have circular groups
$this->cachedGroupsByMember = new CappedMemoryCache();

$groupsByMember = array_values($this->getGroupsByMember($uid));
$groupsByMember = $this->access->nextcloudGroupNames($groupsByMember);
$groups = array_merge($groups, $groupsByMember);
Expand Down

0 comments on commit 07dec64

Please sign in to comment.