From b7fd9af19678a6a0181d354a534e74f7e613786d Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 11 Sep 2024 19:33:53 +0200 Subject: [PATCH] fix: Skip users that still exist in backend Signed-off-by: Ferdinand Thiessen --- lib/composer/composer/autoload_classmap.php | 2 +- lib/composer/composer/autoload_static.php | 2 +- .../User/BackgroundJobs/CleanupDeletedUsers.php | 17 +++++++++++++---- ...end.php => PartiallyDeletedUsersBackend.php} | 14 ++++++++++++-- lib/private/User/User.php | 6 +++--- 5 files changed, 30 insertions(+), 11 deletions(-) rename lib/private/User/{FailedUsersBackend.php => PartiallyDeletedUsersBackend.php} (63%) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index eb5220fbc6787..2a13dbe6b5bb6 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1999,7 +1999,6 @@ 'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => $baseDir . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php', 'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php', 'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php', - 'OC\\User\\FailedUsersBackend' => $baseDir . '/lib/private/User/FailedUsersBackend.php', 'OC\\User\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php', 'OC\\User\\Listeners\\BeforeUserDeletedListener' => $baseDir . '/lib/private/User/Listeners/BeforeUserDeletedListener.php', 'OC\\User\\Listeners\\UserChangedListener' => $baseDir . '/lib/private/User/Listeners/UserChangedListener.php', @@ -2007,6 +2006,7 @@ 'OC\\User\\Manager' => $baseDir . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php', 'OC\\User\\OutOfOfficeData' => $baseDir . '/lib/private/User/OutOfOfficeData.php', + 'OC\\User\\PartiallyDeletedUsersBackend' => $baseDir . '/lib/private/User/PartiallyDeletedUsersBackend.php', 'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.php', 'OC\\User\\User' => $baseDir . '/lib/private/User/User.php', 'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 0d59252bc8f2a..3084c08ee5852 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2032,7 +2032,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => __DIR__ . '/../../..' . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php', 'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php', 'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php', - 'OC\\User\\FailedUsersBackend' => __DIR__ . '/../../..' . '/lib/private/User/FailedUsersBackend.php', 'OC\\User\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php', 'OC\\User\\Listeners\\BeforeUserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/BeforeUserDeletedListener.php', 'OC\\User\\Listeners\\UserChangedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/UserChangedListener.php', @@ -2040,6 +2039,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\Manager' => __DIR__ . '/../../..' . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php', 'OC\\User\\OutOfOfficeData' => __DIR__ . '/../../..' . '/lib/private/User/OutOfOfficeData.php', + 'OC\\User\\PartiallyDeletedUsersBackend' => __DIR__ . '/../../..' . '/lib/private/User/PartiallyDeletedUsersBackend.php', 'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.php', 'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php', 'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php', diff --git a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php index 46ca2175c168d..3c1b73637acdb 100644 --- a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php +++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php @@ -8,10 +8,11 @@ */ namespace OC\User\BackgroundJobs; -use OC\User\FailedUsersBackend; use OC\User\Manager; +use OC\User\PartiallyDeletedUsersBackend; use OC\User\User; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\TimedJob; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; @@ -25,11 +26,12 @@ public function __construct( private LoggerInterface $logger, ) { parent::__construct($time); - $this->setInterval(3600); + $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); + $this->setInterval(24 * 3600); } protected function run($argument): void { - $backend = new FailedUsersBackend($this->config); + $backend = new PartiallyDeletedUsersBackend($this->config); $users = $backend->getUsers(); if (empty($users)) { @@ -38,12 +40,19 @@ protected function run($argument): void { } foreach ($users as $userId) { + if ($this->userManager->userExists($userId)) { + $this->logger->info('Skipping user {userId}, marked as deleted, as they still exists in user backend.', ['userId' => $userId]); + $backend->unmarkUser($userId); + continue; + } + try { $user = new User( $userId, $backend, \OCP\Server::get(IEventDispatcher::class), - config: $this->config, + $this->userManager, + $this->config, ); $user->delete(); $this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]); diff --git a/lib/private/User/FailedUsersBackend.php b/lib/private/User/PartiallyDeletedUsersBackend.php similarity index 63% rename from lib/private/User/FailedUsersBackend.php rename to lib/private/User/PartiallyDeletedUsersBackend.php index 934ceea17dabd..298ddaff6c6bc 100644 --- a/lib/private/User/FailedUsersBackend.php +++ b/lib/private/User/PartiallyDeletedUsersBackend.php @@ -15,7 +15,7 @@ * but not properly removed from Nextcloud (e.g. an exception occurred). * This backend is only needed because some APIs in user-deleted-events require a "real" user with backend. */ -class FailedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend { +class PartiallyDeletedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend { public function __construct( private IConfig $config, @@ -36,11 +36,21 @@ public function userExists($uid) { } public function getHome(string $uid): string|false { - return $this->config->getUserValue($uid, 'core', 'deleted.backup-home') ?: false; + return $this->config->getUserValue($uid, 'core', 'deleted.home-path') ?: false; } public function getUsers($search = '', $limit = null, $offset = null) { return $this->config->getUsersForUserValue('core', 'deleted', 'true'); } + /** + * Unmark a user as deleted. + * This typically the case if the user deletion failed in the backend but before the backend deleted the user, + * meaning the user still exists so we unmark them as it still can be accessed (and deleted) normally. + */ + public function unmarkUser(string $userId): void { + $this->config->deleteUserValue($userId, 'core', 'deleted'); + $this->config->deleteUserValue($userId, 'core', 'deleted.home-path'); + } + } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 21f31fa1a4ff2..92f72fadb2914 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -243,7 +243,7 @@ public function delete() { // because we can not restore the user meaning we could not rollback to any stable state otherwise. $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); // We also need to backup the home path as this can not be reconstructed later if the original backend uses custom home paths - $this->config->setUserValue($this->uid, 'core', 'deleted.backup-home', $this->getHome()); + $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); // Try to delete the user on the backend $result = $this->backend?->deleteUser($this->uid); @@ -289,7 +289,7 @@ public function delete() { $this->config->deleteAllUserValues($this->uid); // But again set flag that this user is about to be deleted $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); - $this->config->setUserValue($this->uid, 'core', 'deleted.backup-home', $this->getHome()); + $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); // Commit the transaction so we are in a defined state: either the preferences are removed or an exception occurred but the delete flag is still present $database->commit(); } catch (\Throwable $e) { @@ -297,7 +297,7 @@ public function delete() { throw $e; } - if ($this->emitter) { + if ($this->emitter !== null) { /** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */ $this->emitter->emit('\OC\User', 'postDelete', [$this]); }