diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 331bddeb65aba..253a72a2a9ad9 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2328,7 +2328,6 @@ 'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php', 'OC\\User\\LastInteractiveLogin' => $baseDir . '/lib/private/User/LastInteractiveLogin.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\\Listeners\\UserLoggedInWithCookieListener' => $baseDir . '/lib/private/User/Listeners/UserLoggedInWithCookieListener.php', 'OC\\User\\LoginException' => $baseDir . '/lib/private/User/LoginException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 04b8c4b2966f8..dae302d8f03aa 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2369,7 +2369,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php', 'OC\\User\\LastInteractiveLogin' => __DIR__ . '/../../..' . '/lib/private/User/LastInteractiveLogin.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\\Listeners\\UserLoggedInWithCookieListener' => __DIR__ . '/../../..' . '/lib/private/User/Listeners/UserLoggedInWithCookieListener.php', 'OC\\User\\LoginException' => __DIR__ . '/../../..' . '/lib/private/User/LoginException.php', diff --git a/lib/private/Server.php b/lib/private/Server.php index 36843d720a854..6e0169faf6e25 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -155,7 +155,6 @@ use OC\Translation\TranslationManager; use OC\User\AvailabilityCoordinator; use OC\User\DisplayNameCache; -use OC\User\Listeners\BeforeUserDeletedListener; use OC\User\Listeners\UserChangedListener; use OC\User\Listeners\UserLoggedInWithCookieListener; use OC\User\Session; @@ -290,7 +289,6 @@ use OCP\Talk\IBroker; use OCP\Teams\ITeamManager; use OCP\Translation\ITranslationManager; -use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\BeforeUserLoggedInEvent; use OCP\User\Events\BeforeUserLoggedInWithCookieEvent; use OCP\User\Events\BeforeUserLoggedOutEvent; @@ -1358,7 +1356,6 @@ private function connectDispatcher(): void { $eventDispatcher->addServiceListener(LoginFailed::class, LoginFailedListener::class); $eventDispatcher->addServiceListener(PostLoginEvent::class, UserLoggedInListener::class); $eventDispatcher->addServiceListener(UserChangedEvent::class, UserChangedListener::class); - $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, BeforeUserDeletedListener::class); $eventDispatcher->addServiceListener(UserLoggedInWithCookieEvent::class, UserLoggedInWithCookieListener::class); FilesMetadataManager::loadListeners($eventDispatcher); diff --git a/lib/private/User/Listeners/BeforeUserDeletedListener.php b/lib/private/User/Listeners/BeforeUserDeletedListener.php deleted file mode 100644 index 322b4e9f94ac2..0000000000000 --- a/lib/private/User/Listeners/BeforeUserDeletedListener.php +++ /dev/null @@ -1,53 +0,0 @@ - - */ -class BeforeUserDeletedListener implements IEventListener { - public function __construct( - private LoggerInterface $logger, - private IAvatarManager $avatarManager, - private ICredentialsManager $credentialsManager, - ) { - } - - #[\Override] - public function handle(Event $event): void { - if (!($event instanceof BeforeUserDeletedEvent)) { - return; - } - - $user = $event->getUser(); - - // Delete avatar on user deletion - try { - $avatar = $this->avatarManager->getAvatar($user->getUID()); - $avatar->remove(true); - } catch (NotFoundException $e) { - // no avatar to remove - } catch (\Exception $e) { - // Ignore exceptions - $this->logger->info('Could not cleanup avatar of ' . $user->getUID(), [ - 'exception' => $e, - ]); - } - // Delete storages credentials on user deletion - $this->credentialsManager->erase($user->getUID()); - } -} diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 636c109c3082e..cfd18862471f7 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -26,6 +26,7 @@ use OCP\IUser; use OCP\IUserBackend; use OCP\Notification\IManager as INotificationManager; +use OCP\Security\ICredentialsManager; use OCP\Server; use OCP\Support\Subscription\IAssertion; use OCP\User\Backend\IGetHomeBackend; @@ -293,6 +294,9 @@ public function delete(): bool { $avatarManager = Server::get(AvatarManager::class); $avatarManager->deleteUserAvatar($this->uid); + $credentialsManager = Server::get(ICredentialsManager::class); + $credentialsManager->erase($this->uid); + $notificationManager = Server::get(INotificationManager::class); $notification = $notificationManager->createNotification(); $notification->setUser($this->uid); diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 76bcac21c01af..39f67f0fe4389 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -9,24 +9,29 @@ namespace Test\User; use OC\AllConfig; +use OC\Avatar\AvatarManager; use OC\Files\Mount\ObjectHomeMountProvider; use OC\Hooks\PublicEmitter; use OC\User\Database; use OC\User\User; use OCP\Comments\ICommentsManager; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\FileInfo; use OCP\Files\Storage\IStorageFactory; use OCP\IConfig; use OCP\IURLGenerator; use OCP\IUser; +use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; use OCP\Notification\INotification; +use OCP\Security\ICredentialsManager; use OCP\Server; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; +use Test\Util\User\Dummy; #[Group('DB')] class UserTest extends TestCase { @@ -429,6 +434,8 @@ public function testDeleteHooks(bool $result, int $expectedHooks): void { $commentsManager = $this->createMock(ICommentsManager::class); $notificationManager = $this->createMock(INotificationManager::class); + $avatarManager = $this->createMock(AvatarManager::class); + $credentialsManager = $this->createMock(ICredentialsManager::class); if ($result) { $config->expects($this->atLeastOnce()) @@ -453,6 +460,13 @@ public function testDeleteHooks(bool $result, int $expectedHooks): void { $notificationManager->expects($this->once()) ->method('markProcessed') ->with($notification); + + $avatarManager->expects($this->once()) + ->method('deleteUserAvatar') + ->with('foo'); + $credentialsManager->expects($this->once()) + ->method('erase') + ->with('foo'); } else { $config->expects($this->never()) ->method('deleteAllUserValues'); @@ -466,9 +480,16 @@ public function testDeleteHooks(bool $result, int $expectedHooks): void { ->method('createNotification'); $notificationManager->expects($this->never()) ->method('markProcessed'); + + $avatarManager->expects($this->never()) + ->method('deleteUserAvatar'); + $credentialsManager->expects($this->never()) + ->method('erase'); } $this->overwriteService(INotificationManager::class, $notificationManager); + $this->overwriteService(AvatarManager::class, $avatarManager); + $this->overwriteService(ICredentialsManager::class, $credentialsManager); $this->overwriteService(ICommentsManager::class, $commentsManager); $this->assertSame($result, $user->delete()); @@ -476,10 +497,44 @@ public function testDeleteHooks(bool $result, int $expectedHooks): void { $this->restoreService(AllConfig::class); $this->restoreService(ICommentsManager::class); $this->restoreService(INotificationManager::class); + $this->restoreService(AvatarManager::class); + $this->restoreService(ICredentialsManager::class); $this->assertEquals($expectedHooks, $hooksCalled); } + public function testDeleteRefusedByBackendKeepsAvatarAndCredentials(): void { + $backend = new class extends Dummy { + #[\Override] + public function deleteUser($uid): bool { + return false; + } + }; + $uid = $this->getUniqueID('user'); + $backend->createUser($uid, 'password'); + + $userManager = Server::get(IUserManager::class); + $userManager->registerBackend($backend); + $avatarManager = Server::get(AvatarManager::class); + $credentialsManager = Server::get(ICredentialsManager::class); + + try { + $user = $userManager->get($uid); + $avatarManager->getAvatar($uid)->set(file_get_contents(\OC::$SERVERROOT . '/tests/data/testavatar.png')); + $credentialsManager->store($uid, 'test', ['password' => 'secret']); + + $this->assertFalse($user->delete()); + + $this->assertTrue($avatarManager->getAvatar($uid)->exists()); + $this->assertSame(['password' => 'secret'], $credentialsManager->retrieve($uid, 'test')); + } finally { + $avatarManager->deleteUserAvatar($uid); + $credentialsManager->erase($uid); + Server::get(IUserConfig::class)->deleteAllUserConfig($uid); + $userManager->removeBackend($backend); + } + } + public function testDeleteRecoverState() { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo');