diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 2b7b5b6e652b1..972bd36e31a74 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1613,6 +1613,7 @@ 'OC\\Repair' => $baseDir . '/lib/private/Repair.php', 'OC\\RepairException' => $baseDir . '/lib/private/RepairException.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', @@ -1804,6 +1805,7 @@ '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', @@ -1813,6 +1815,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_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 2dd883a794b19..806cb8d30a542 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1646,6 +1646,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair' => __DIR__ . '/../../..' . '/lib/private/Repair.php', 'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.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', @@ -1837,6 +1838,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', @@ -1846,6 +1848,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/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php index 5e657be0763b9..bd639a3a37c8b 100644 --- a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php +++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php @@ -33,26 +33,31 @@ use OCP\Files\Storage\IStorage; use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\UserDeletedEvent; +use Psr\Log\LoggerInterface; +/** @template-implements IEventListener */ class UserDeletedFilesCleanupListener implements IEventListener { /** @var array */ 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("User has no home storage"); @@ -67,12 +72,14 @@ public function handle(Event $event): void { $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 fe1925bddfe51..80fc78661359f 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -37,6 +37,7 @@ use OC\DB\Connection; use OC\DB\ConnectionAdapter; use OC\Repair\AddBruteForceCleanupJob; +use OC\Repair\AddCleanupDeletedUsersBackgroundJob; use OC\Repair\AddCleanupUpdaterBackupsJob; use OC\Repair\AddMetadataGenerationJob; use OC\Repair\AddRemoveOldTasksBackgroundJob; @@ -215,6 +216,7 @@ public static function getRepairSteps(): array { \OCP\Server::get(AddRemoveOldTasksBackgroundJob::class), \OCP\Server::get(AddMetadataGenerationJob::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 0000000000000..9713d8595e774 --- /dev/null +++ b/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php @@ -0,0 +1,30 @@ +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 3cd3716c19598..48cf3fa8e07bd 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -56,6 +56,7 @@ use OC\Log\Rotate; use OC\Preview\BackgroundCleanupJob; use OC\TextProcessing\RemoveOldTasksBackgroundJob; +use OC\User\BackgroundJobs\CleanupDeletedUsers; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\IGroup; @@ -463,6 +464,7 @@ public static function installBackgroundJobs() { $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 b68e4c2e541ee..061ad11516145 100644 --- a/lib/private/User/Backend.php +++ b/lib/private/User/Backend.php @@ -124,9 +124,9 @@ public function userExists($uid) { /** * 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 0000000000000..3c1b73637acdb --- /dev/null +++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php @@ -0,0 +1,64 @@ +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 0000000000000..298ddaff6c6bc --- /dev/null +++ b/lib/private/User/PartiallyDeletedUsersBackend.php @@ -0,0 +1,56 @@ +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 580c590e6eb54..3b0172d915164 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -40,15 +40,19 @@ use OC\Hooks\Emitter; use OC_Helper; use OCP\Accounts\IAccountManager; +use OCP\Comments\ICommentsManager; use OCP\EventDispatcher\IEventDispatcher; 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; @@ -61,6 +65,8 @@ use OCP\User\Events\UserDeletedEvent; use OCP\User\GetQuotaEvent; use OCP\UserInterface; +use Psr\Log\LoggerInterface; + use function json_decode; use function json_encode; @@ -69,6 +75,7 @@ class User implements IUser { /** @var IAccountManager */ protected $accountManager; + /** @var string */ private $uid; @@ -84,7 +91,7 @@ class User implements IUser { /** @var bool|null */ private $enabled; - /** @var Emitter|Manager */ + /** @var Emitter|Manager|null */ private $emitter; /** @var string */ @@ -93,28 +100,29 @@ 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) { + /** @var IConfig */ + private $config; + + public function __construct( + string $uid, + ?UserInterface $backend, + IEventDispatcher $dispatcher, + $emitter = null, + ?IConfig $config = null, + $urlGenerator = null, + ) { $this->uid = $uid; $this->backend = $backend; - $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->emitter = $emitter; + $this->config = $config ?? \OCP\Server::get(IConfig::class); + $this->urlGenerator = $urlGenerator ?? \OCP\Server::get(IURLGenerator::class); } /** @@ -240,9 +248,9 @@ private function ensureAccountManager() { */ public function getLastLogin() { if ($this->lastLogin === null) { - $this->lastLogin = (int) $this->config->getUserValue($this->uid, 'login', 'lastLogin', 0); + $this->lastLogin = (int)$this->config->getUserValue($this->uid, 'login', 'lastLogin', 0); } - return (int) $this->lastLogin; + return (int)$this->lastLogin; } /** @@ -268,50 +276,85 @@ public function updateLastLoginTimestamp() { * @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->getCommentsManager()->deleteReferencesOfActor('users', $this->uid); - \OC::$server->getCommentsManager()->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.home-path', $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.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) { - /** @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 !== null) { + /** @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; } /** @@ -354,10 +397,8 @@ public function getHome() { /** @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; @@ -534,7 +575,7 @@ public function setQuota($quota) { if ($quota !== 'none' and $quota !== 'default') { $bytesQuota = OC_Helper::computerFileSize($quota); if ($bytesQuota === false) { - throw new InvalidArgumentException('Failed to set quota to invalid value '.$quota); + throw new InvalidArgumentException('Failed to set quota to invalid value ' . $quota); } $quota = OC_Helper::humanFileSize($bytesQuota); } diff --git a/tests/lib/Traits/UserTrait.php b/tests/lib/Traits/UserTrait.php index 3f7cfe419db45..d2c1444c94377 100644 --- a/tests/lib/Traits/UserTrait.php +++ b/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 { diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 806cb094066cf..965dd65aeabbe 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -514,14 +514,25 @@ public function testDeleteHooks($result, $expectedHooks) { $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 @@ public function testDeleteHooks($result, $expectedHooks) { $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 @@ public function testDeleteHooks($result, $expectedHooks) { $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 @@ public function testDeleteHooks($result, $expectedHooks) { $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'],