Skip to content

Commit 3095a92

Browse files
susnuxsusnux
authored
Merge pull request #47896 from nextcloud/fix/resiliant-user-removal
fix: Make user removal more resilient
2 parents 363ba6b + d57a2dd commit 3095a92

11 files changed

Lines changed: 328 additions & 85 deletions

File tree

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,7 @@
17971797
'OC\\RepairException' => $baseDir . '/lib/private/RepairException.php',
17981798
'OC\\Repair\\AddAppConfigLazyMigration' => $baseDir . '/lib/private/Repair/AddAppConfigLazyMigration.php',
17991799
'OC\\Repair\\AddBruteForceCleanupJob' => $baseDir . '/lib/private/Repair/AddBruteForceCleanupJob.php',
1800+
'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => $baseDir . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php',
18001801
'OC\\Repair\\AddCleanupUpdaterBackupsJob' => $baseDir . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
18011802
'OC\\Repair\\AddMetadataGenerationJob' => $baseDir . '/lib/private/Repair/AddMetadataGenerationJob.php',
18021803
'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => $baseDir . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php',
@@ -1996,6 +1997,7 @@
19961997
'OC\\UserStatus\\Manager' => $baseDir . '/lib/private/UserStatus/Manager.php',
19971998
'OC\\User\\AvailabilityCoordinator' => $baseDir . '/lib/private/User/AvailabilityCoordinator.php',
19981999
'OC\\User\\Backend' => $baseDir . '/lib/private/User/Backend.php',
2000+
'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => $baseDir . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php',
19992001
'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php',
20002002
'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php',
20012003
'OC\\User\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php',
@@ -2005,6 +2007,7 @@
20052007
'OC\\User\\Manager' => $baseDir . '/lib/private/User/Manager.php',
20062008
'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php',
20072009
'OC\\User\\OutOfOfficeData' => $baseDir . '/lib/private/User/OutOfOfficeData.php',
2010+
'OC\\User\\PartiallyDeletedUsersBackend' => $baseDir . '/lib/private/User/PartiallyDeletedUsersBackend.php',
20082011
'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.php',
20092012
'OC\\User\\User' => $baseDir . '/lib/private/User/User.php',
20102013
'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1830,6 +1830,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
18301830
'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.php',
18311831
'OC\\Repair\\AddAppConfigLazyMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/AddAppConfigLazyMigration.php',
18321832
'OC\\Repair\\AddBruteForceCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddBruteForceCleanupJob.php',
1833+
'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php',
18331834
'OC\\Repair\\AddCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
18341835
'OC\\Repair\\AddMetadataGenerationJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddMetadataGenerationJob.php',
18351836
'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php',
@@ -2029,6 +2030,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
20292030
'OC\\UserStatus\\Manager' => __DIR__ . '/../../..' . '/lib/private/UserStatus/Manager.php',
20302031
'OC\\User\\AvailabilityCoordinator' => __DIR__ . '/../../..' . '/lib/private/User/AvailabilityCoordinator.php',
20312032
'OC\\User\\Backend' => __DIR__ . '/../../..' . '/lib/private/User/Backend.php',
2033+
'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => __DIR__ . '/../../..' . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php',
20322034
'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php',
20332035
'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php',
20342036
'OC\\User\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php',
@@ -2038,6 +2040,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
20382040
'OC\\User\\Manager' => __DIR__ . '/../../..' . '/lib/private/User/Manager.php',
20392041
'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php',
20402042
'OC\\User\\OutOfOfficeData' => __DIR__ . '/../../..' . '/lib/private/User/OutOfOfficeData.php',
2043+
'OC\\User\\PartiallyDeletedUsersBackend' => __DIR__ . '/../../..' . '/lib/private/User/PartiallyDeletedUsersBackend.php',
20412044
'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.php',
20422045
'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php',
20432046
'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php',

‎lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,31 @@
1616
use OCP\Files\Storage\IStorage;
1717
use OCP\User\Events\BeforeUserDeletedEvent;
1818
use OCP\User\Events\UserDeletedEvent;
19+
use Psr\Log\LoggerInterface;
1920

2021
/** @template-implements IEventListener<BeforeUserDeletedEvent|UserDeletedEvent> */
2122
class UserDeletedFilesCleanupListener implements IEventListener {
2223
/** @var array<string,IStorage> */
2324
private $homeStorageCache = [];
2425

25-
/** @var IMountProviderCollection */
26-
private $mountProviderCollection;
27-
28-
public function __construct(IMountProviderCollection $mountProviderCollection) {
29-
$this->mountProviderCollection = $mountProviderCollection;
26+
public function __construct(
27+
private IMountProviderCollection $mountProviderCollection,
28+
private LoggerInterface $logger,
29+
) {
3030
}
3131

3232
public function handle(Event $event): void {
33+
$user = $event->getUser();
34+
3335
// since we can't reliably get the user home storage after the user is deleted
3436
// but the user deletion might get canceled during the before event
3537
// we only cache the user home storage during the before event and then do the
3638
// action deletion during the after event
3739

3840
if ($event instanceof BeforeUserDeletedEvent) {
39-
$userHome = $this->mountProviderCollection->getHomeMountForUser($event->getUser());
41+
$this->logger->debug('Prepare deleting storage for user {userId}', ['userId' => $user->getUID()]);
42+
43+
$userHome = $this->mountProviderCollection->getHomeMountForUser($user);
4044
$storage = $userHome->getStorage();
4145
if (!$storage) {
4246
throw new \Exception('Account has no home storage');
@@ -51,12 +55,14 @@ public function handle(Event $event): void {
5155
$this->homeStorageCache[$event->getUser()->getUID()] = $storage;
5256
}
5357
if ($event instanceof UserDeletedEvent) {
54-
if (!isset($this->homeStorageCache[$event->getUser()->getUID()])) {
58+
if (!isset($this->homeStorageCache[$user->getUID()])) {
5559
throw new \Exception('UserDeletedEvent fired without matching BeforeUserDeletedEvent');
5660
}
57-
$storage = $this->homeStorageCache[$event->getUser()->getUID()];
61+
$storage = $this->homeStorageCache[$user->getUID()];
5862
$cache = $storage->getCache();
5963
$storage->rmdir('');
64+
$this->logger->debug('Deleted storage for user {userId}', ['userId' => $user->getUID()]);
65+
6066
if ($cache instanceof Cache) {
6167
$cache->clear();
6268
} else {

‎lib/private/Repair.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OC\DB\ConnectionAdapter;
1212
use OC\Repair\AddAppConfigLazyMigration;
1313
use OC\Repair\AddBruteForceCleanupJob;
14+
use OC\Repair\AddCleanupDeletedUsersBackgroundJob;
1415
use OC\Repair\AddCleanupUpdaterBackupsJob;
1516
use OC\Repair\AddMetadataGenerationJob;
1617
use OC\Repair\AddRemoveOldTasksBackgroundJob;
@@ -192,6 +193,7 @@ public static function getRepairSteps(): array {
192193
\OCP\Server::get(AddAppConfigLazyMigration::class),
193194
\OCP\Server::get(RepairLogoDimension::class),
194195
\OCP\Server::get(RemoveLegacyDatadirFile::class),
196+
\OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class),
195197
];
196198
}
197199

‎lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Repair;
10+
11+
use OC\User\BackgroundJobs\CleanupDeletedUsers;
12+
use OCP\BackgroundJob\IJobList;
13+
use OCP\Migration\IOutput;
14+
use OCP\Migration\IRepairStep;
15+
16+
class AddCleanupDeletedUsersBackgroundJob implements IRepairStep {
17+
private IJobList $jobList;
18+
19+
public function __construct(IJobList $jobList) {
20+
$this->jobList = $jobList;
21+
}
22+
23+
public function getName(): string {
24+
return 'Add cleanup-deleted-users background job';
25+
}
26+
27+
public function run(IOutput $output) {
28+
$this->jobList->add(CleanupDeletedUsers::class);
29+
}
30+
}

‎lib/private/Setup.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OC\Log\Rotate;
1818
use OC\Preview\BackgroundCleanupJob;
1919
use OC\TextProcessing\RemoveOldTasksBackgroundJob;
20+
use OC\User\BackgroundJobs\CleanupDeletedUsers;
2021
use OCP\AppFramework\Utility\ITimeFactory;
2122
use OCP\BackgroundJob\IJobList;
2223
use OCP\Defaults;
@@ -415,6 +416,7 @@ public static function installBackgroundJobs(): void {
415416
$jobList->add(Rotate::class);
416417
$jobList->add(BackgroundCleanupJob::class);
417418
$jobList->add(RemoveOldTasksBackgroundJob::class);
419+
$jobList->add(CleanupDeletedUsers::class);
418420
}
419421

420422
/**

‎lib/private/User/Backend.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ public function userExists($uid) {
107107
/**
108108
* get the user's home directory
109109
* @param string $uid the username
110-
* @return boolean
110+
* @return string|boolean
111111
*/
112-
public function getHome($uid) {
112+
public function getHome(string $uid) {
113113
return false;
114114
}
115115

‎lib/private/User/BackgroundJobs/CleanupDeletedUsers.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\User\BackgroundJobs;
10+
11+
use OC\User\Manager;
12+
use OC\User\PartiallyDeletedUsersBackend;
13+
use OC\User\User;
14+
use OCP\AppFramework\Utility\ITimeFactory;
15+
use OCP\BackgroundJob\IJob;
16+
use OCP\BackgroundJob\TimedJob;
17+
use OCP\EventDispatcher\IEventDispatcher;
18+
use OCP\IConfig;
19+
use Psr\Log\LoggerInterface;
20+
21+
class CleanupDeletedUsers extends TimedJob {
22+
public function __construct(
23+
ITimeFactory $time,
24+
private Manager $userManager,
25+
private IConfig $config,
26+
private LoggerInterface $logger,
27+
) {
28+
parent::__construct($time);
29+
$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
30+
$this->setInterval(24 * 3600);
31+
}
32+
33+
protected function run($argument): void {
34+
$backend = new PartiallyDeletedUsersBackend($this->config);
35+
$users = $backend->getUsers();
36+
37+
if (empty($users)) {
38+
$this->logger->debug('No failed deleted users found.');
39+
return;
40+
}
41+
42+
foreach ($users as $userId) {
43+
if ($this->userManager->userExists($userId)) {
44+
$this->logger->info('Skipping user {userId}, marked as deleted, as they still exists in user backend.', ['userId' => $userId]);
45+
$backend->unmarkUser($userId);
46+
continue;
47+
}
48+
49+
try {
50+
$user = new User(
51+
$userId,
52+
$backend,
53+
\OCP\Server::get(IEventDispatcher::class),
54+
$this->userManager,
55+
$this->config,
56+
);
57+
$user->delete();
58+
$this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]);
59+
} catch (\Throwable $error) {
60+
$this->logger->warning('Could not cleanup deleted user {userId}', ['userId' => $userId, 'exception' => $error]);
61+
}
62+
}
63+
}
64+
}

‎lib/private/User/PartiallyDeletedUsersBackend.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
namespace OC\User;
8+
9+
use OCP\IConfig;
10+
use OCP\IUserBackend;
11+
use OCP\User\Backend\IGetHomeBackend;
12+
13+
/**
14+
* This is a "fake" backend for users that were deleted,
15+
* but not properly removed from Nextcloud (e.g. an exception occurred).
16+
* This backend is only needed because some APIs in user-deleted-events require a "real" user with backend.
17+
*/
18+
class PartiallyDeletedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend {
19+
20+
public function __construct(
21+
private IConfig $config,
22+
) {
23+
}
24+
25+
public function deleteUser($uid): bool {
26+
// fake true, deleting failed users is automatically handled by User::delete()
27+
return true;
28+
}
29+
30+
public function getBackendName(): string {
31+
return 'deleted users';
32+
}
33+
34+
public function userExists($uid) {
35+
return $this->config->getUserValue($uid, 'core', 'deleted') === 'true';
36+
}
37+
38+
public function getHome(string $uid): string|false {
39+
return $this->config->getUserValue($uid, 'core', 'deleted.home-path') ?: false;
40+
}
41+
42+
public function getUsers($search = '', $limit = null, $offset = null) {
43+
return $this->config->getUsersForUserValue('core', 'deleted', 'true');
44+
}
45+
46+
/**
47+
* Unmark a user as deleted.
48+
* This typically the case if the user deletion failed in the backend but before the backend deleted the user,
49+
* meaning the user still exists so we unmark them as it still can be accessed (and deleted) normally.
50+
*/
51+
public function unmarkUser(string $userId): void {
52+
$this->config->deleteUserValue($userId, 'core', 'deleted');
53+
$this->config->deleteUserValue($userId, 'core', 'deleted.home-path');
54+
}
55+
56+
}

0 commit comments

Comments
 (0)