Merge pull request #48588 from nextcloud/backport/47896/stable29

[stable29] fix: Make user removal more resilient

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,6 +1869,7 @@ 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\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php',
@@ -1877,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',

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,6 +1902,7 @@ 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\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php',
@@ -1910,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',

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 {

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),
 		];
 	}
 

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);
+	}
+}

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);
 	}
 
 	/**

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;
 	}
 

lib/private/User/BackgroundJobs/CleanupDeletedUsers.php

@@ -0,0 +1,64 @@
+<?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\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;
+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->setTimeSensitivity(IJob::TIME_INSENSITIVE);
+		$this->setInterval(24 * 3600);
+	}
+
+	protected function run($argument): void {
+		$backend = new PartiallyDeletedUsersBackend($this->config);
+		$users = $backend->getUsers();
+
+		if (empty($users)) {
+			$this->logger->debug('No failed deleted users found.');
+			return;
+		}
+
+		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),
+					$this->userManager,
+					$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]);
+			}
+		}
+	}
+}

lib/private/User/PartiallyDeletedUsersBackend.php

@@ -0,0 +1,56 @@
+<?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 PartiallyDeletedUsersBackend 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.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');
+	}
+
+}

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));
+
+		// 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.home-path', $this->getHome());
+
+		// Try to delete the user on the backend
 		$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);
-
-			/** @var AvatarManager $avatarManager */
-			$avatarManager = \OCP\Server::get(AvatarManager::class);
-			$avatarManager->deleteUserAvatar($this->uid);
-
-			$notification = \OC::$server->getNotificationManager()->createNotification();
-			$notification->setUser($this->uid);
-			\OC::$server->getNotificationManager()->markProcessed($notification);
-
-			/** @var AccountManager $accountManager */
-			$accountManager = \OCP\Server::get(AccountManager::class);
-			$accountManager->deleteUser($this);
-
-			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 ($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;
 		}
-		return !($result === false);
+
+		// 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));
+			}
+		}
+
+		$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.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) {
+			$database->rollback();
+			throw $e;
+		}
+
+		if ($this->emitter !== null) {
+			/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
+			$this->emitter->emit('\OC\User', 'postDelete', [$this]);
+		}
+		$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;

tests/lib/Traits/UserTrait.php

@@ -9,13 +9,16 @@
 namespace Test\Traits;
 
 use OC\User\User;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IUser;
+use OCP\Server;
 
 class DummyUser extends User {
 	private string $uid;
 
 	public function __construct(string $uid) {
 		$this->uid = $uid;
+		parent::__construct($uid, null, Server::get(IEventDispatcher::class));
 	}
 
 	public function getUID(): string {

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'],