diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 8bb5d593e88b9..11aa1d11b3f20 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2450,7 +2450,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 b7412813556c6..f03c2ab10e01f 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2491,7 +2491,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 e2f0bccd27800..5bcab4ee937c9 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -154,7 +154,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; @@ -1195,7 +1193,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 163ec2bb2cdc6..0000000000000 --- a/lib/private/User/Listeners/BeforeUserDeletedListener.php +++ /dev/null @@ -1,54 +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 e3424211e0ced..2cd9b42bca095 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -28,6 +28,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; @@ -297,6 +298,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 895dd8e39e333..1b81089b36052 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -9,6 +9,7 @@ namespace Test\User; use OC\AllConfig; +use OC\Avatar\AvatarManager; use OC\Files\Mount\ObjectHomeMountProvider; use OC\Hooks\PublicEmitter; use OC\User\Database; @@ -21,13 +22,16 @@ 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 { @@ -434,6 +438,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) { $userConfig->expects($this->atLeastOnce()) @@ -458,6 +464,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 { $userConfig->expects($this->never()) ->method('deleteAllUserConfig'); @@ -471,9 +484,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()); @@ -481,10 +501,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');