Skip to content

Commit

Permalink
Merge pull request #48587 from nextcloud/backport/47896/stable28
Browse files Browse the repository at this point in the history
[stable28] fix: Make user removal more resilient
  • Loading branch information
susnux authored Oct 17, 2024
2 parents bc706ac + c42ec8d commit 6c8f6ad
Show file tree
Hide file tree
Showing 12 changed files with 342 additions and 78 deletions.
3 changes: 3 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down
3 changes: 3 additions & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<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("User has no home storage");
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions lib/private/Repair.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
];
}

Expand Down
30 changes: 30 additions & 0 deletions lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
2 changes: 2 additions & 0 deletions lib/private/Setup.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -463,6 +464,7 @@ public static function installBackgroundJobs() {
$jobList->add(Rotate::class);
$jobList->add(BackgroundCleanupJob::class);
$jobList->add(RemoveOldTasksBackgroundJob::class);
$jobList->add(CleanupDeletedUsers::class);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/private/User/Backend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
64 changes: 64 additions & 0 deletions lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
Original file line number Diff line number Diff line change
@@ -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]);
}
}
}
}
56 changes: 56 additions & 0 deletions lib/private/User/PartiallyDeletedUsersBackend.php
Original file line number Diff line number Diff line change
@@ -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');
}

}
Loading

0 comments on commit 6c8f6ad

Please sign in to comment.