Skip to content

Commit 10f81ad

Browse files
authored
Merge pull request #41107 from nextcloud/backport/41106/stable26
[stable26] fix(ldap): store last known user groups
2 parents 58ee21f + 873222f commit 10f81ad

File tree

5 files changed

+444
-416
lines changed

5 files changed

+444
-416
lines changed

apps/user_ldap/lib/Connection.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,10 @@ public function getFromCache($key) {
288288
return json_decode(base64_decode($this->cache->get($key) ?? ''), true);
289289
}
290290

291+
public function getConfigPrefix(): string {
292+
return $this->configPrefix;
293+
}
294+
291295
/**
292296
* @param string $key
293297
* @param mixed $value

apps/user_ldap/lib/Group_LDAP.php

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,17 @@
4545
namespace OCA\User_LDAP;
4646

4747
use Exception;
48+
use OCA\User_LDAP\User\OfflineUser;
4849
use OCP\Cache\CappedMemoryCache;
4950
use OCP\GroupInterface;
5051
use OCP\Group\Backend\IDeleteGroupBackend;
5152
use OCP\Group\Backend\IGetDisplayNameBackend;
5253
use OC\ServerNotAvailableException;
54+
use OCP\IConfig;
55+
use OCP\IUserManager;
56+
use OCP\Server;
5357
use Psr\Log\LoggerInterface;
58+
use function json_decode;
5459

5560
class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
5661
protected bool $enabled = false;
@@ -68,8 +73,15 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
6873
* @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name
6974
*/
7075
protected string $ldapGroupMemberAssocAttr;
71-
72-
public function __construct(Access $access, GroupPluginManager $groupPluginManager) {
76+
private IConfig $config;
77+
private IUserManager $ncUserManager;
78+
79+
public function __construct(
80+
Access $access,
81+
GroupPluginManager $groupPluginManager,
82+
IConfig $config,
83+
IUserManager $ncUserManager
84+
) {
7385
parent::__construct($access);
7486
$filter = $this->access->connection->ldapGroupFilter;
7587
$gAssoc = $this->access->connection->ldapGroupMemberAssocAttr;
@@ -81,8 +93,10 @@ public function __construct(Access $access, GroupPluginManager $groupPluginManag
8193
$this->cachedGroupsByMember = new CappedMemoryCache();
8294
$this->cachedNestedGroups = new CappedMemoryCache();
8395
$this->groupPluginManager = $groupPluginManager;
84-
$this->logger = \OCP\Server::get(LoggerInterface::class);
96+
$this->logger = Server::get(LoggerInterface::class);
8597
$this->ldapGroupMemberAssocAttr = strtolower((string)$gAssoc);
98+
$this->config = $config;
99+
$this->ncUserManager = $ncUserManager;
86100
}
87101

88102
/**
@@ -437,6 +451,7 @@ public function getGroupGidNumber(string $dn) {
437451
public function getUserGidNumber(string $dn) {
438452
$gidNumber = false;
439453
if ($this->access->connection->hasGidNumber) {
454+
// FIXME: when $dn does not exist on LDAP anymore, this will be set wrongly to false :/
440455
$gidNumber = $this->getEntryGidNumber($dn, $this->access->connection->ldapGidNumber);
441456
if ($gidNumber === false) {
442457
$this->access->connection->hasGidNumber = false;
@@ -650,6 +665,25 @@ public function getUserPrimaryGroup(string $dn) {
650665
return false;
651666
}
652667

668+
private function isUserOnLDAP(string $uid): bool {
669+
// forces a user exists check - but does not help if a positive result is cached, while group info is not
670+
$ncUser = $this->ncUserManager->get($uid);
671+
if ($ncUser === null) {
672+
return false;
673+
}
674+
$backend = $ncUser->getBackend();
675+
if ($backend instanceof User_Proxy) {
676+
// ignoring cache as safeguard (and we are behind the group cache check anyway)
677+
return $backend->userExistsOnLDAP($uid, true);
678+
}
679+
return false;
680+
}
681+
682+
protected function getCachedGroupsForUserId(string $uid): array {
683+
$groupStr = $this->config->getUserValue($uid, 'user_ldap', 'cached-group-memberships-' . $this->access->connection->getConfigPrefix(), '[]');
684+
return json_decode($groupStr) ?? [];
685+
}
686+
653687
/**
654688
* This function fetches all groups a user belongs to. It does not check
655689
* if the user exists at all.
@@ -661,15 +695,25 @@ public function getUserPrimaryGroup(string $dn) {
661695
* @throws Exception
662696
* @throws ServerNotAvailableException
663697
*/
664-
public function getUserGroups($uid) {
698+
public function getUserGroups($uid): array {
665699
if (!$this->enabled) {
666700
return [];
667701
}
702+
$ncUid = $uid;
703+
668704
$cacheKey = 'getUserGroups' . $uid;
669705
$userGroups = $this->access->connection->getFromCache($cacheKey);
670706
if (!is_null($userGroups)) {
671707
return $userGroups;
672708
}
709+
710+
$user = $this->access->userManager->get($uid);
711+
if ($user instanceof OfflineUser) {
712+
// We load known group memberships from configuration for remnants,
713+
// because LDAP server does not contain them anymore
714+
return $this->getCachedGroupsForUserId($uid);
715+
}
716+
673717
$userDN = $this->access->username2dn($uid);
674718
if (!$userDN) {
675719
$this->access->connection->writeToCache($cacheKey, []);
@@ -781,9 +825,21 @@ public function getUserGroups($uid) {
781825
$groups[] = $gidGroupName;
782826
}
783827

828+
if (empty($groups) && !$this->isUserOnLDAP($ncUid)) {
829+
// Groups are enabled, but you user has none? Potentially suspicious:
830+
// it could be that the user was deleted from LDAP, but we are not
831+
// aware of it yet.
832+
$groups = $this->getCachedGroupsForUserId($ncUid);
833+
$this->access->connection->writeToCache($cacheKey, $groups);
834+
return $groups;
835+
}
836+
784837
$groups = array_unique($groups, SORT_LOCALE_STRING);
785838
$this->access->connection->writeToCache($cacheKey, $groups);
786839

840+
$groupStr = \json_encode($groups);
841+
$this->config->setUserValue($ncUid, 'user_ldap', 'cached-group-memberships-' . $this->access->connection->getConfigPrefix(), $groupStr);
842+
787843
return $groups;
788844
}
789845

apps/user_ldap/lib/Group_Proxy.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,32 @@
3131
use OCP\Group\Backend\IDeleteGroupBackend;
3232
use OCP\Group\Backend\IGetDisplayNameBackend;
3333
use OCP\Group\Backend\INamedBackend;
34+
use OCP\GroupInterface;
35+
use OCP\IConfig;
36+
use OCP\IUserManager;
3437

3538
class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend {
3639
private $backends = [];
3740
private ?Group_LDAP $refBackend = null;
3841
private Helper $helper;
3942
private GroupPluginManager $groupPluginManager;
4043
private bool $isSetUp = false;
44+
private IConfig $config;
45+
private IUserManager $ncUserManager;
4146

4247
public function __construct(
4348
Helper $helper,
4449
ILDAPWrapper $ldap,
4550
AccessFactory $accessFactory,
46-
GroupPluginManager $groupPluginManager
51+
GroupPluginManager $groupPluginManager,
52+
IConfig $config,
53+
IUserManager $ncUserManager,
4754
) {
4855
parent::__construct($ldap, $accessFactory);
4956
$this->helper = $helper;
5057
$this->groupPluginManager = $groupPluginManager;
58+
$this->config = $config;
59+
$this->ncUserManager = $ncUserManager;
5160
}
5261

5362
protected function setup(): void {
@@ -58,7 +67,7 @@ protected function setup(): void {
5867
$serverConfigPrefixes = $this->helper->getServerConfigurationPrefixes(true);
5968
foreach ($serverConfigPrefixes as $configPrefix) {
6069
$this->backends[$configPrefix] =
61-
new Group_LDAP($this->getAccess($configPrefix), $this->groupPluginManager);
70+
new Group_LDAP($this->getAccess($configPrefix), $this->groupPluginManager, $this->config, $this->ncUserManager);
6271
if (is_null($this->refBackend)) {
6372
$this->refBackend = &$this->backends[$configPrefix];
6473
}

0 commit comments

Comments
 (0)