Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Skip users that still exist in backend
Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Oct 15, 2024
commit 1b76925aed75d94481c1230c6c32b7f7ff57e18e
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1808,14 +1808,14 @@
'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => $baseDir . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php',
'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php',
'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php',
'OC\\User\\FailedUsersBackend' => $baseDir . '/lib/private/User/FailedUsersBackend.php',
'OC\\User\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php',
'OC\\User\\Listeners\\BeforeUserDeletedListener' => $baseDir . '/lib/private/User/Listeners/BeforeUserDeletedListener.php',
'OC\\User\\Listeners\\UserChangedListener' => $baseDir . '/lib/private/User/Listeners/UserChangedListener.php',
'OC\\User\\LoginException' => $baseDir . '/lib/private/User/LoginException.php',
'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
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1841,14 +1841,14 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => __DIR__ . '/../../..' . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php',
'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php',
'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php',
'OC\\User\\FailedUsersBackend' => __DIR__ . '/../../..' . '/lib/private/User/FailedUsersBackend.php',
'OC\\User\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php',
'OC\\User\\Listeners\\BeforeUserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/BeforeUserDeletedListener.php',
'OC\\User\\Listeners\\UserChangedListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/UserChangedListener.php',
'OC\\User\\LoginException' => __DIR__ . '/../../..' . '/lib/private/User/LoginException.php',
'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
17 changes: 13 additions & 4 deletions lib/private/User/BackgroundJobs/CleanupDeletedUsers.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
*/
namespace OC\User\BackgroundJobs;

use OC\User\FailedUsersBackend;
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;
Expand All @@ -25,11 +26,12 @@ public function __construct(
private LoggerInterface $logger,
) {
parent::__construct($time);
$this->setInterval(3600);
$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
$this->setInterval(24 * 3600);
}

protected function run($argument): void {
$backend = new FailedUsersBackend($this->config);
$backend = new PartiallyDeletedUsersBackend($this->config);
$users = $backend->getUsers();

if (empty($users)) {
Expand All @@ -38,12 +40,19 @@ protected function run($argument): void {
}

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),
config: $this->config,
$this->userManager,
$this->config,
);
$user->delete();
$this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* but not properly removed from Nextcloud (e.g. an exception occurred).
* This backend is only needed because some APIs in user-deleted-events require a "real" user with backend.
*/
class FailedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend {
class PartiallyDeletedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend {

public function __construct(
private IConfig $config,
Expand All @@ -36,11 +36,21 @@ public function userExists($uid) {
}

public function getHome(string $uid): string|false {
return $this->config->getUserValue($uid, 'core', 'deleted.backup-home') ?: 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');
}

}
6 changes: 3 additions & 3 deletions lib/private/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ public function delete() {
// because we can not restore the user meaning we could not rollback to any stable state otherwise.
$this->config->setUserValue($this->uid, 'core', 'deleted', 'true');
// We also need to backup the home path as this can not be reconstructed later if the original backend uses custom home paths
$this->config->setUserValue($this->uid, 'core', 'deleted.backup-home', $this->getHome());
$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);
Expand Down Expand Up @@ -337,15 +337,15 @@ public function delete() {
$this->config->deleteAllUserValues($this->uid);
// But again set flag that this user is about to be deleted
$this->config->setUserValue($this->uid, 'core', 'deleted', 'true');
$this->config->setUserValue($this->uid, 'core', 'deleted.backup-home', $this->getHome());
$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) {
if ($this->emitter !== null) {
/** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */
$this->emitter->emit('\OC\User', 'postDelete', [$this]);
}
Expand Down