Skip to content

Commit c9f26d9

Browse files
committed
fix(LDAP): unblock a cached id from a deleted user or group
As of 155b750 we cache the user id and LDAP DN for a month, because they are highly requested, but rarely change and we have a working repair mechanism. But if an LDAP user is deleted, the information is still cached. So a user that is being regenerated with the same target user ID will get a _xxxx suffix, when the data is still in local cache. Signed-off-by: Arthur Schiwon <[email protected]>
1 parent d14cf6a commit c9f26d9

File tree

3 files changed

+29
-4
lines changed

3 files changed

+29
-4
lines changed

apps/user_ldap/lib/Access.php

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public function __construct(
5757
private Helper $helper,
5858
private IConfig $config,
5959
private IUserManager $ncUserManager,
60+
private IGroupManager $ncGroupManager,
6061
private LoggerInterface $logger,
6162
private IAppConfig $appConfig,
6263
private IEventDispatcher $dispatcher,
@@ -595,18 +596,35 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
595596
$originalTTL = $this->connection->ldapCacheTTL;
596597
$this->connection->setConfiguration(['ldapCacheTTL' => 0]);
597598
if ($intName !== ''
598-
&& (($isUser && !$this->ncUserManager->userExists($intName))
599-
|| (!$isUser && !Server::get(IGroupManager::class)->groupExists($intName))
599+
&& (($isUser
600+
&& (
601+
!$this->ncUserManager->userExists($intName)
602+
// DN might be cached if the user was deleted within last month.
603+
// If it was a valid user we would have returned earlier at the
604+
// $mapper->getNameByDN($fdn) call, which does not run against the cache.
605+
|| $this->ncUserManager->get($intName)?->getBackendClassName() === 'LDAP'
606+
))
607+
|| (!$isUser
608+
&& (
609+
// DN might be cached if the group was deleted within last month.
610+
// If it was a valid group we would have returned earlier at the
611+
// $mapper->getNameByDN($fdn) call, which does not run against the cache.
612+
!$this->ncGroupManager->groupExists($intName)
613+
|| in_array('LDAP', $this->ncGroupManager->get($intName)?->getBackendNames() ?? [], true)
614+
))
600615
)
601616
) {
602617
$this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]);
603618
$newlyMapped = $this->mapAndAnnounceIfApplicable($mapper, $fdn, $intName, $uuid, $isUser);
604619
if ($newlyMapped) {
605-
$this->logger->debug('Mapped {fdn} as {name}', ['fdn' => $fdn,'name' => $intName]);
620+
$this->logger->debug('Mapped {fdn} as {name}', ['fdn' => $fdn, 'name' => $intName]);
606621
return $intName;
607622
}
608623
}
609624

625+
$dude = $this->ncUserManager->get($intName);
626+
$dude->getBackendClassName() === 'LDAP';
627+
610628
$this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]);
611629
$altName = $this->createAltInternalOwnCloudName($intName, $isUser);
612630
if (is_string($altName)) {
@@ -830,7 +848,7 @@ private function _createAltInternalOwnCloudNameForGroups(string $name) {
830848
// Check to be really sure it is unique
831849
// while loop is just a precaution. If a name is not generated within
832850
// 20 attempts, something else is very wrong. Avoids infinite loop.
833-
if (!Server::get(IGroupManager::class)->groupExists($altName)) {
851+
if (!$this->ncGroupManager->groupExists($altName)) {
834852
return $altName;
835853
}
836854
$altName = $name . '_' . ($lastNo + $attempts);

apps/user_ldap/lib/AccessFactory.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCP\EventDispatcher\IEventDispatcher;
1111
use OCP\IAppConfig;
1212
use OCP\IConfig;
13+
use OCP\IGroupManager;
1314
use OCP\IUserManager;
1415
use OCP\Server;
1516
use Psr\Log\LoggerInterface;
@@ -22,6 +23,7 @@ public function __construct(
2223
private IConfig $config,
2324
private IAppConfig $appConfig,
2425
private IUserManager $ncUserManager,
26+
private IGroupManager $ncGroupManager,
2527
private LoggerInterface $logger,
2628
private IEventDispatcher $dispatcher,
2729
) {
@@ -36,6 +38,7 @@ public function get(Connection $connection): Access {
3638
$this->helper,
3739
$this->config,
3840
$this->ncUserManager,
41+
$this->ncGroupManager,
3942
$this->logger,
4043
$this->appConfig,
4144
$this->dispatcher,

apps/user_ldap/tests/AccessTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use OCP\IAppConfig;
2626
use OCP\IAvatarManager;
2727
use OCP\IConfig;
28+
use OCP\IGroupManager;
2829
use OCP\Image;
2930
use OCP\IUserManager;
3031
use OCP\Notification\IManager as INotificationManager;
@@ -51,6 +52,7 @@ class AccessTest extends TestCase {
5152
private Helper&MockObject $helper;
5253
private IConfig&MockObject $config;
5354
private IUserManager&MockObject $ncUserManager;
55+
private IGroupManager&MockObject $ncGroupManager;
5456
private LoggerInterface&MockObject $logger;
5557
private IAppConfig&MockObject $appConfig;
5658
private IEventDispatcher&MockObject $dispatcher;
@@ -67,6 +69,7 @@ protected function setUp(): void {
6769
$this->userMapper = $this->createMock(UserMapping::class);
6870
$this->groupMapper = $this->createMock(GroupMapping::class);
6971
$this->ncUserManager = $this->createMock(IUserManager::class);
72+
$this->ncGroupManager = $this->createMock(IGroupManager::class);
7073
$this->shareManager = $this->createMock(IManager::class);
7174
$this->logger = $this->createMock(LoggerInterface::class);
7275
$this->appConfig = $this->createMock(IAppConfig::class);
@@ -79,6 +82,7 @@ protected function setUp(): void {
7982
$this->helper,
8083
$this->config,
8184
$this->ncUserManager,
85+
$this->ncGroupManager,
8286
$this->logger,
8387
$this->appConfig,
8488
$this->dispatcher,

0 commit comments

Comments
 (0)