fix: Skip users that still exist in backend

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

5 files changed, 30 insertions, 11 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 7ba6808481b..3c107a853e1 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1872,7 +1872,6 @@ return array(
     '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',
@@ -1880,6 +1879,7 @@ return array(
     '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_API' => $baseDir . '/lib/private/legacy/OC_API.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index ff5c6b4b502..0734376c160 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1905,7 +1905,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',
@@ -1913,6 +1912,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_API' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_API.php',
diff --git a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
index 46ca2175c16..3c1b73637ac 100644
--- a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
+++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
@@ -8,10 +8,11 @@ declare(strict_types=1);
  */
 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 @@ class CleanupDeletedUsers extends TimedJob {
 		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 @@ class CleanupDeletedUsers extends TimedJob {
 		}
 
 		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
index 934ceea17da..298ddaff6c6 100644
--- a/lib/private/User/FailedUsersBackend.php
+++ b/lib/private/User/PartiallyDeletedUsersBackend.php
@@ -15,7 +15,7 @@ use OCP\User\Backend\IGetHomeBackend;
  * 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 @@ class FailedUsersBackend extends Backend implements IGetHomeBackend, IUserBacken
 	}
 
 	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 13aed9fe45f..f2e56f1aac3 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -276,7 +276,7 @@ class User implements IUser {
 		// 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);
@@ -322,7 +322,7 @@ class User implements IUser {
 			$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) {
@@ -330,7 +330,7 @@ class User implements IUser {
 			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]);
 		}