fix: Make user removal more resilient

Currently there is a problem if an exception is thrown in `User::delete`,
because at that point the user is already removed from the backend,
but not all data is deleted.

There is no way to recover from this state, as the user is gone no information is available anymore.
This means the data is still available on the server but can not removed by any API anymore.

The solution here is to first set a flag and backup the user home,
this can be used to recover failed user deletions in a way the delete can be re-tried.

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

11 files changed, 313 insertions, 88 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 13b04a96531..7ba6808481b 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1673,6 +1673,7 @@ return array(
     'OC\\RepairException' => $baseDir . '/lib/private/RepairException.php',
     'OC\\Repair\\AddAppConfigLazyMigration' => $baseDir . '/lib/private/Repair/AddAppConfigLazyMigration.php',
     'OC\\Repair\\AddBruteForceCleanupJob' => $baseDir . '/lib/private/Repair/AddBruteForceCleanupJob.php',
+    'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => $baseDir . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php',
     'OC\\Repair\\AddCleanupUpdaterBackupsJob' => $baseDir . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
     'OC\\Repair\\AddMetadataGenerationJob' => $baseDir . '/lib/private/Repair/AddMetadataGenerationJob.php',
     'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => $baseDir . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php',
@@ -1868,8 +1869,10 @@ return array(
     'OC\\UserStatus\\Manager' => $baseDir . '/lib/private/UserStatus/Manager.php',
     'OC\\User\\AvailabilityCoordinator' => $baseDir . '/lib/private/User/AvailabilityCoordinator.php',
     'OC\\User\\Backend' => $baseDir . '/lib/private/User/Backend.php',
+    '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',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 8b413e5bc7b..ff5c6b4b502 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1706,6 +1706,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.php',
         'OC\\Repair\\AddAppConfigLazyMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/AddAppConfigLazyMigration.php',
         'OC\\Repair\\AddBruteForceCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddBruteForceCleanupJob.php',
+        'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php',
         'OC\\Repair\\AddCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
         'OC\\Repair\\AddMetadataGenerationJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddMetadataGenerationJob.php',
         'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php',
@@ -1901,8 +1902,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\UserStatus\\Manager' => __DIR__ . '/../../..' . '/lib/private/UserStatus/Manager.php',
         'OC\\User\\AvailabilityCoordinator' => __DIR__ . '/../../..' . '/lib/private/User/AvailabilityCoordinator.php',
         'OC\\User\\Backend' => __DIR__ . '/../../..' . '/lib/private/User/Backend.php',
+        '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',
diff --git a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
index a29a73ad8ad..0e5c324542f 100644
--- a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
+++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
@@ -33,27 +33,31 @@ use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\Storage\IStorage;
 use OCP\User\Events\BeforeUserDeletedEvent;
 use OCP\User\Events\UserDeletedEvent;
+use Psr\Log\LoggerInterface;
 
 /** @template-implements IEventListener<BeforeUserDeletedEvent|UserDeletedEvent> */
 class UserDeletedFilesCleanupListener implements IEventListener {
 	/** @var array<string,IStorage> */
 	private $homeStorageCache = [];
 
-	/** @var IMountProviderCollection */
-	private $mountProviderCollection;
-
-	public function __construct(IMountProviderCollection $mountProviderCollection) {
-		$this->mountProviderCollection = $mountProviderCollection;
+	public function __construct(
+		private IMountProviderCollection $mountProviderCollection,
+		private LoggerInterface $logger,
+	) {
 	}
 
 	public function handle(Event $event): void {
+		$user = $event->getUser();
+
 		// since we can't reliably get the user home storage after the user is deleted
 		// but the user deletion might get canceled during the before event
 		// we only cache the user home storage during the before event and then do the
 		// action deletion during the after event
 
 		if ($event instanceof BeforeUserDeletedEvent) {
-			$userHome = $this->mountProviderCollection->getHomeMountForUser($event->getUser());
+			$this->logger->debug('Prepare deleting storage for user {userId}', ['userId' => $user->getUID()]);
+
+			$userHome = $this->mountProviderCollection->getHomeMountForUser($user);
 			$storage = $userHome->getStorage();
 			if (!$storage) {
 				throw new \Exception("Account has no home storage");
@@ -68,12 +72,14 @@ class UserDeletedFilesCleanupListener implements IEventListener {
 			$this->homeStorageCache[$event->getUser()->getUID()] = $storage;
 		}
 		if ($event instanceof UserDeletedEvent) {
-			if (!isset($this->homeStorageCache[$event->getUser()->getUID()])) {
-				throw new \Exception("UserDeletedEvent fired without matching BeforeUserDeletedEvent");
+			if (!isset($this->homeStorageCache[$user->getUID()])) {
+				throw new \Exception('UserDeletedEvent fired without matching BeforeUserDeletedEvent');
 			}
-			$storage = $this->homeStorageCache[$event->getUser()->getUID()];
+			$storage = $this->homeStorageCache[$user->getUID()];
 			$cache = $storage->getCache();
 			$storage->rmdir('');
+			$this->logger->debug('Deleted storage for user {userId}', ['userId' => $user->getUID()]);
+
 			if ($cache instanceof Cache) {
 				$cache->clear();
 			} else {
diff --git a/lib/private/Repair.php b/lib/private/Repair.php
index 96a3feee2c2..0a0b257a8ff 100644
--- a/lib/private/Repair.php
+++ b/lib/private/Repair.php
@@ -38,6 +38,7 @@ use OC\DB\Connection;
 use OC\DB\ConnectionAdapter;
 use OC\Repair\AddAppConfigLazyMigration;
 use OC\Repair\AddBruteForceCleanupJob;
+use OC\Repair\AddCleanupDeletedUsersBackgroundJob;
 use OC\Repair\AddCleanupUpdaterBackupsJob;
 use OC\Repair\AddMetadataGenerationJob;
 use OC\Repair\AddRemoveOldTasksBackgroundJob;
@@ -213,6 +214,7 @@ class Repair implements IOutput {
 			\OCP\Server::get(AddMetadataGenerationJob::class),
 			\OCP\Server::get(AddAppConfigLazyMigration::class),
 			\OCP\Server::get(RepairLogoDimension::class),
+			\OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class),
 		];
 	}
 
diff --git a/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php b/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php
new file mode 100644
index 00000000000..9713d8595e7
--- /dev/null
+++ b/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair;
+
+use OC\User\BackgroundJobs\CleanupDeletedUsers;
+use OCP\BackgroundJob\IJobList;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class AddCleanupDeletedUsersBackgroundJob implements IRepairStep {
+	private IJobList $jobList;
+
+	public function __construct(IJobList $jobList) {
+		$this->jobList = $jobList;
+	}
+
+	public function getName(): string {
+		return 'Add cleanup-deleted-users background job';
+	}
+
+	public function run(IOutput $output) {
+		$this->jobList->add(CleanupDeletedUsers::class);
+	}
+}
diff --git a/lib/private/Setup.php b/lib/private/Setup.php
index 6c0419ed6d6..9ea954c6e9a 100644
--- a/lib/private/Setup.php
+++ b/lib/private/Setup.php
@@ -59,6 +59,7 @@ use OC\Authentication\Token\TokenCleanupJob;
 use OC\Log\Rotate;
 use OC\Preview\BackgroundCleanupJob;
 use OC\TextProcessing\RemoveOldTasksBackgroundJob;
+use OC\User\BackgroundJobs\CleanupDeletedUsers;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Defaults;
@@ -453,6 +454,7 @@ class Setup {
 		$jobList->add(Rotate::class);
 		$jobList->add(BackgroundCleanupJob::class);
 		$jobList->add(RemoveOldTasksBackgroundJob::class);
+		$jobList->add(CleanupDeletedUsers::class);
 	}
 
 	/**
diff --git a/lib/private/User/Backend.php b/lib/private/User/Backend.php
index b68e4c2e541..061ad115161 100644
--- a/lib/private/User/Backend.php
+++ b/lib/private/User/Backend.php
@@ -124,9 +124,9 @@ abstract class Backend implements UserInterface {
 	/**
 	 * get the user's home directory
 	 * @param string $uid the username
-	 * @return boolean
+	 * @return string|boolean
 	 */
-	public function getHome($uid) {
+	public function getHome(string $uid) {
 		return false;
 	}
 
diff --git a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
new file mode 100644
index 00000000000..46ca2175c16
--- /dev/null
+++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\User\BackgroundJobs;
+
+use OC\User\FailedUsersBackend;
+use OC\User\Manager;
+use OC\User\User;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IConfig;
+use Psr\Log\LoggerInterface;
+
+class CleanupDeletedUsers extends TimedJob {
+	public function __construct(
+		ITimeFactory $time,
+		private Manager $userManager,
+		private IConfig $config,
+		private LoggerInterface $logger,
+	) {
+		parent::__construct($time);
+		$this->setInterval(3600);
+	}
+
+	protected function run($argument): void {
+		$backend = new FailedUsersBackend($this->config);
+		$users = $backend->getUsers();
+
+		if (empty($users)) {
+			$this->logger->debug('No failed deleted users found.');
+			return;
+		}
+
+		foreach ($users as $userId) {
+			try {
+				$user = new User(
+					$userId,
+					$backend,
+					\OCP\Server::get(IEventDispatcher::class),
+					config: $this->config,
+				);
+				$user->delete();
+				$this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]);
+			} catch (\Throwable $error) {
+				$this->logger->warning('Could not cleanup deleted user {userId}', ['userId' => $userId, 'exception' => $error]);
+			}
+		}
+	}
+}
diff --git a/lib/private/User/FailedUsersBackend.php b/lib/private/User/FailedUsersBackend.php
new file mode 100644
index 00000000000..934ceea17da
--- /dev/null
+++ b/lib/private/User/FailedUsersBackend.php
@@ -0,0 +1,46 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\User;
+
+use OCP\IConfig;
+use OCP\IUserBackend;
+use OCP\User\Backend\IGetHomeBackend;
+
+/**
+ * This is a "fake" backend for users that were deleted,
+ * 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 {
+
+	public function __construct(
+		private IConfig $config,
+	) {
+	}
+
+	public function deleteUser($uid): bool {
+		// fake true, deleting failed users is automatically handled by User::delete()
+		return true;
+	}
+
+	public function getBackendName(): string {
+		return 'deleted users';
+	}
+
+	public function userExists($uid) {
+		return $this->config->getUserValue($uid, 'core', 'deleted') === 'true';
+	}
+
+	public function getHome(string $uid): string|false {
+		return $this->config->getUserValue($uid, 'core', 'deleted.backup-home') ?: false;
+	}
+
+	public function getUsers($search = '', $limit = null, $offset = null) {
+		return $this->config->getUsersForUserValue('core', 'deleted', 'true');
+	}
+
+}
diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index daa78011007..13aed9fe45f 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -46,10 +46,13 @@ use OCP\Group\Events\BeforeUserRemovedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\IAvatarManager;
 use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IImage;
 use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserBackend;
+use OCP\Notification\IManager as INotificationManager;
 use OCP\User\Backend\IGetHomeBackend;
 use OCP\User\Backend\IProvideAvatarBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
@@ -62,30 +65,27 @@ use OCP\User\Events\UserChangedEvent;
 use OCP\User\Events\UserDeletedEvent;
 use OCP\User\GetQuotaEvent;
 use OCP\UserInterface;
+use Psr\Log\LoggerInterface;
+
 use function json_decode;
 use function json_encode;
 
 class User implements IUser {
 	private const CONFIG_KEY_MANAGERS = 'manager';
 
+	private IConfig $config;
+	private IURLGenerator $urlGenerator;
+
 	/** @var IAccountManager */
 	protected $accountManager;
-	/** @var string */
-	private $uid;
 
 	/** @var string|null */
 	private $displayName;
 
-	/** @var UserInterface|null */
-	private $backend;
-
-	/** @var IEventDispatcher */
-	private $dispatcher;
-
 	/** @var bool|null */
 	private $enabled;
 
-	/** @var Emitter|Manager */
+	/** @var Emitter|Manager|null */
 	private $emitter;
 
 	/** @var string */
@@ -94,28 +94,20 @@ class User implements IUser {
 	/** @var int|null */
 	private $lastLogin;
 
-	/** @var \OCP\IConfig */
-	private $config;
-
 	/** @var IAvatarManager */
 	private $avatarManager;
 
-	/** @var IURLGenerator */
-	private $urlGenerator;
-
-	public function __construct(string $uid, ?UserInterface $backend, IEventDispatcher $dispatcher, $emitter = null, ?IConfig $config = null, $urlGenerator = null) {
-		$this->uid = $uid;
-		$this->backend = $backend;
+	public function __construct(
+		private string $uid,
+		private ?UserInterface $backend,
+		private IEventDispatcher $dispatcher,
+		$emitter = null,
+		?IConfig $config = null,
+		$urlGenerator = null,
+	) {
 		$this->emitter = $emitter;
-		if (is_null($config)) {
-			$config = \OC::$server->getConfig();
-		}
-		$this->config = $config;
-		$this->urlGenerator = $urlGenerator;
-		if (is_null($this->urlGenerator)) {
-			$this->urlGenerator = \OC::$server->getURLGenerator();
-		}
-		$this->dispatcher = $dispatcher;
+		$this->config = $config ?? \OCP\Server::get(IConfig::class);
+		$this->urlGenerator = $urlGenerator ?? \OCP\Server::get(IURLGenerator::class);
 	}
 
 	/**
@@ -269,50 +261,85 @@ class User implements IUser {
 	 * @return bool
 	 */
 	public function delete() {
+		if ($this->backend === null) {
+			\OCP\Server::get(LoggerInterface::class)->error('Cannot delete user: No backend set');
+			return false;
+		}
+
 		if ($this->emitter) {
 			/** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */
 			$this->emitter->emit('\OC\User', 'preDelete', [$this]);
 		}
 		$this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this));
-		$result = $this->backend->deleteUser($this->uid);
-		if ($result) {
-			// FIXME: Feels like an hack - suggestions?
-
-			$groupManager = \OC::$server->getGroupManager();
-			// We have to delete the user from all groups
-			foreach ($groupManager->getUserGroupIds($this) as $groupId) {
-				$group = $groupManager->get($groupId);
-				if ($group) {
-					$this->dispatcher->dispatchTyped(new BeforeUserRemovedEvent($group, $this));
-					$group->removeUser($this);
-					$this->dispatcher->dispatchTyped(new UserRemovedEvent($group, $this));
-				}
-			}
-			// Delete the user's keys in preferences
-			\OC::$server->getConfig()->deleteAllUserValues($this->uid);
 
-			\OC::$server->get(ICommentsManager::class)->deleteReferencesOfActor('users', $this->uid);
-			\OC::$server->get(ICommentsManager::class)->deleteReadMarksFromUser($this);
+		// Set delete flag on the user - this is needed to ensure that the user data is removed if there happen any exception in the backend
+		// 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());
 
-			/** @var AvatarManager $avatarManager */
-			$avatarManager = \OCP\Server::get(AvatarManager::class);
-			$avatarManager->deleteUserAvatar($this->uid);
+		// Try to delete the user on the backend
+		$result = $this->backend->deleteUser($this->uid);
+		if ($result === false) {
+			// The deletion was aborted or something else happened, we are in a defined state, so remove the delete flag
+			$this->config->deleteUserValue($this->uid, 'core', 'deleted');
+			return false;
+		}
 
-			$notification = \OC::$server->getNotificationManager()->createNotification();
-			$notification->setUser($this->uid);
-			\OC::$server->getNotificationManager()->markProcessed($notification);
+		// We have to delete the user from all groups
+		$groupManager = \OCP\Server::get(IGroupManager::class);
+		foreach ($groupManager->getUserGroupIds($this) as $groupId) {
+			$group = $groupManager->get($groupId);
+			if ($group) {
+				$this->dispatcher->dispatchTyped(new BeforeUserRemovedEvent($group, $this));
+				$group->removeUser($this);
+				$this->dispatcher->dispatchTyped(new UserRemovedEvent($group, $this));
+			}
+		}
 
-			/** @var AccountManager $accountManager */
-			$accountManager = \OCP\Server::get(AccountManager::class);
-			$accountManager->deleteUser($this);
+		$commentsManager = \OCP\Server::get(ICommentsManager::class);
+		$commentsManager->deleteReferencesOfActor('users', $this->uid);
+		$commentsManager->deleteReadMarksFromUser($this);
+
+		$avatarManager = \OCP\Server::get(AvatarManager::class);
+		$avatarManager->deleteUserAvatar($this->uid);
+
+		$notificationManager = \OCP\Server::get(INotificationManager::class);
+		$notification = $notificationManager->createNotification();
+		$notification->setUser($this->uid);
+		$notificationManager->markProcessed($notification);
+
+		$accountManager = \OCP\Server::get(AccountManager::class);
+		$accountManager->deleteUser($this);
+
+		$database = \OCP\Server::get(IDBConnection::class);
+		try {
+			// We need to create a transaction to make sure we are in a defined state
+			// because if all user values are removed also the flag is gone, but if an exception happens (e.g. database lost connection on the set operation)
+			// exactly here we are in an undefined state as the data is still present but the user does not exist on the system anymore.
+			$database->beginTransaction();
+			// Remove all user settings
+			$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());
+			// 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) {
+			$database->rollback();
+			throw $e;
+		}
 
-			if ($this->emitter) {
-				/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
-				$this->emitter->emit('\OC\User', 'postDelete', [$this]);
-			}
-			$this->dispatcher->dispatchTyped(new UserDeletedEvent($this));
+		if ($this->emitter) {
+			/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
+			$this->emitter->emit('\OC\User', 'postDelete', [$this]);
 		}
-		return !($result === false);
+		$this->dispatcher->dispatchTyped(new UserDeletedEvent($this));
+
+		// Finally we can unset the delete flag and all other states
+		$this->config->deleteAllUserValues($this->uid);
+
+		return true;
 	}
 
 	/**
@@ -355,10 +382,8 @@ class User implements IUser {
 			/** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */
 			if (($this->backend instanceof IGetHomeBackend || $this->backend->implementsActions(Backend::GET_HOME)) && $home = $this->backend->getHome($this->uid)) {
 				$this->home = $home;
-			} elseif ($this->config) {
-				$this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid;
 			} else {
-				$this->home = \OC::$SERVERROOT . '/data/' . $this->uid;
+				$this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid;
 			}
 		}
 		return $this->home;
diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
index 806cb094066..965dd65aeab 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -514,14 +514,25 @@ class UserTest extends TestCase {
 		$test = $this;
 
 		/**
-		 * @var Backend | MockObject $backend
+		 * @var UserInterface&MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
 			->method('deleteUser')
 			->willReturn($result);
+
+		$config = $this->createMock(IConfig::class);
+		$config->method('getSystemValue')
+			->willReturnArgument(1);
+		$config->method('getSystemValueString')
+			->willReturnArgument(1);
+		$config->method('getSystemValueBool')
+			->willReturnArgument(1);
+		$config->method('getSystemValueInt')
+			->willReturnArgument(1);
+
 		$emitter = new PublicEmitter();
-		$user = new User('foo', $backend, $this->dispatcher, $emitter);
+		$user = new User('foo', $backend, $this->dispatcher, $emitter, $config);
 
 		/**
 		 * @param User $user
@@ -534,21 +545,11 @@ class UserTest extends TestCase {
 		$emitter->listen('\OC\User', 'preDelete', $hook);
 		$emitter->listen('\OC\User', 'postDelete', $hook);
 
-		$config = $this->createMock(IConfig::class);
 		$commentsManager = $this->createMock(ICommentsManager::class);
 		$notificationManager = $this->createMock(INotificationManager::class);
 
-		$config->method('getSystemValue')
-			->willReturnArgument(1);
-		$config->method('getSystemValueString')
-			->willReturnArgument(1);
-		$config->method('getSystemValueBool')
-			->willReturnArgument(1);
-		$config->method('getSystemValueInt')
-			->willReturnArgument(1);
-
 		if ($result) {
-			$config->expects($this->once())
+			$config->expects($this->atLeastOnce())
 				->method('deleteAllUserValues')
 				->with('foo');
 
@@ -587,7 +588,6 @@ class UserTest extends TestCase {
 
 		$this->overwriteService(\OCP\Notification\IManager::class, $notificationManager);
 		$this->overwriteService(\OCP\Comments\ICommentsManager::class, $commentsManager);
-		$this->overwriteService(AllConfig::class, $config);
 
 		$this->assertSame($result, $user->delete());
 
@@ -598,6 +598,59 @@ class UserTest extends TestCase {
 		$this->assertEquals($expectedHooks, $hooksCalled);
 	}
 
+	public function testDeleteRecoverState() {
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+		$backend->expects($this->once())
+			->method('deleteUser')
+			->willReturn(true);
+
+		$config = $this->createMock(IConfig::class);
+		$config->method('getSystemValue')
+			->willReturnArgument(1);
+		$config->method('getSystemValueString')
+			->willReturnArgument(1);
+		$config->method('getSystemValueBool')
+			->willReturnArgument(1);
+		$config->method('getSystemValueInt')
+			->willReturnArgument(1);
+
+		$userConfig = [];
+		$config->expects(self::atLeast(2))
+			->method('setUserValue')
+			->willReturnCallback(function () {
+				$userConfig[] = func_get_args();
+			});
+
+		$commentsManager = $this->createMock(ICommentsManager::class);
+		$commentsManager->expects($this->once())
+			->method('deleteReferencesOfActor')
+			->willThrowException(new \Error('Test exception'));
+
+		$this->overwriteService(\OCP\Comments\ICommentsManager::class, $commentsManager);
+		$this->expectException(\Error::class);
+
+		$user = $this->getMockBuilder(User::class)
+			->onlyMethods(['getHome'])
+			->setConstructorArgs(['foo', $backend, $this->dispatcher, null, $config])
+			->getMock();
+		
+		$user->expects(self::atLeastOnce())
+			->method('getHome')
+			->willReturn('/home/path');
+
+		$user->delete();
+
+		$this->assertEqualsCanonicalizing(
+			[
+				['foo', 'core', 'deleted', 'true', null],
+				['foo', 'core', 'deleted.backup-home', '/home/path', null],
+			],
+			$userConfig,
+		);
+
+		$this->restoreService(\OCP\Comments\ICommentsManager::class);
+	}
+
 	public function dataGetCloudId(): array {
 		return [
 			['https://localhost:8888/nextcloud', 'foo@localhost:8888/nextcloud'],