aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/composer/composer/autoload_classmap.php3
-rw-r--r--lib/composer/composer/autoload_static.php3
-rw-r--r--lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php22
-rw-r--r--lib/private/Repair.php2
-rw-r--r--lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php30
-rw-r--r--lib/private/Setup.php2
-rw-r--r--lib/private/User/Backend.php4
-rw-r--r--lib/private/User/BackgroundJobs/CleanupDeletedUsers.php64
-rw-r--r--lib/private/User/PartiallyDeletedUsersBackend.php56
-rw-r--r--lib/private/User/User.php146
-rw-r--r--tests/lib/User/UserTest.php81
11 files changed, 328 insertions, 85 deletions
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 603e0a1455c..35416cc1303 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1797,6 +1797,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',
@@ -1996,6 +1997,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',
@@ -2005,6 +2007,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_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 17bbd9738dd..71eb43cddf8 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1830,6 +1830,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',
@@ -2029,6 +2030,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',
@@ -2038,6 +2040,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/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
index 8523fb6abc7..a619021d192 100644
--- a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
+++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
@@ -16,27 +16,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');
@@ -51,12 +55,14 @@ class UserDeletedFilesCleanupListener implements IEventListener {
$this->homeStorageCache[$event->getUser()->getUID()] = $storage;
}
if ($event instanceof UserDeletedEvent) {
- if (!isset($this->homeStorageCache[$event->getUser()->getUID()])) {
+ 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 42d3cfb4910..b1a824ba5e3 100644
--- a/lib/private/Repair.php
+++ b/lib/private/Repair.php
@@ -11,6 +11,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;
@@ -192,6 +193,7 @@ class Repair implements IOutput {
\OCP\Server::get(AddAppConfigLazyMigration::class),
\OCP\Server::get(RepairLogoDimension::class),
\OCP\Server::get(RemoveLegacyDatadirFile::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 e64f6806b87..f286c3698cf 100644
--- a/lib/private/Setup.php
+++ b/lib/private/Setup.php
@@ -17,6 +17,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;
@@ -415,6 +416,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 98b291e5dee..9b6a9a890ef 100644
--- a/lib/private/User/Backend.php
+++ b/lib/private/User/Backend.php
@@ -107,9 +107,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..3c1b73637ac
--- /dev/null
+++ b/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]);
+ }
+ }
+ }
+}
diff --git a/lib/private/User/PartiallyDeletedUsersBackend.php b/lib/private/User/PartiallyDeletedUsersBackend.php
new file mode 100644
index 00000000000..298ddaff6c6
--- /dev/null
+++ b/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');
+ }
+
+}
diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index f2c38825338..6e1085b4fe3 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -19,6 +19,8 @@ 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;
@@ -37,30 +39,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 */
@@ -69,28 +68,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);
}
/**
@@ -244,50 +235,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));
- }
+ 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;
+ }
+
+ // 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));
}
- // 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);
+ $commentsManager = \OCP\Server::get(ICommentsManager::class);
+ $commentsManager->deleteReferencesOfActor('users', $this->uid);
+ $commentsManager->deleteReadMarksFromUser($this);
- /** @var AvatarManager $avatarManager */
- $avatarManager = \OCP\Server::get(AvatarManager::class);
- $avatarManager->deleteUserAvatar($this->uid);
+ $avatarManager = \OCP\Server::get(AvatarManager::class);
+ $avatarManager->deleteUserAvatar($this->uid);
- $notification = \OC::$server->get(INotificationManager::class)->createNotification();
- $notification->setUser($this->uid);
- \OC::$server->get(INotificationManager::class)->markProcessed($notification);
+ $notificationManager = \OCP\Server::get(INotificationManager::class);
+ $notification = $notificationManager->createNotification();
+ $notification->setUser($this->uid);
+ $notificationManager->markProcessed($notification);
- /** @var AccountManager $accountManager */
- $accountManager = \OCP\Server::get(AccountManager::class);
- $accountManager->deleteUser($this);
+ $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));
+ $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;
}
- return !($result === false);
+
+ 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;
}
/**
@@ -344,10 +370,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 55230b83667..e79aaadd593 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -513,14 +513,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
@@ -533,21 +544,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');
@@ -586,7 +587,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());
@@ -597,6 +597,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'],
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-21 05:00:45 +0000