From ed2d02d5f1000c76776c6e8dbe24fa787ffe6d0d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 27 Apr 2021 15:43:34 +0200 Subject: [PATCH] better cleanup of user files on user deletion Signed-off-by: Robin Appelman --- core/Application.php | 4 + lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../UserDeletedFilesCleanupListener.php | 73 +++++++++++++++++++ lib/private/Files/Storage/Common.php | 2 +- lib/private/User/User.php | 13 ---- tests/lib/User/UserTest.php | 3 + 7 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php diff --git a/core/Application.php b/core/Application.php index c3cb6f02ed5f8..fe65faeb9dfc7 100644 --- a/core/Application.php +++ b/core/Application.php @@ -37,6 +37,7 @@ use OC\Authentication\Listeners\RemoteWipeActivityListener; use OC\Authentication\Listeners\RemoteWipeEmailListener; use OC\Authentication\Listeners\RemoteWipeNotificationsListener; +use OC\Authentication\Listeners\UserDeletedFilesCleanupListener; use OC\Authentication\Listeners\UserDeletedStoreCleanupListener; use OC\Authentication\Listeners\UserDeletedTokenCleanupListener; use OC\Authentication\Notifications\Notifier as AuthenticationNotifier; @@ -49,6 +50,7 @@ use OCP\AppFramework\App; use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; +use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Symfony\Component\EventDispatcher\GenericEvent; @@ -270,5 +272,7 @@ function (GenericEvent $event) use ($container) { $eventDispatcher->addServiceListener(RemoteWipeFinished::class, RemoteWipeEmailListener::class); $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedStoreCleanupListener::class); $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedTokenCleanupListener::class); + $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, UserDeletedFilesCleanupListener::class); + $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedFilesCleanupListener::class); } } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3033c0f69e5ab..88d7a165dcc7a 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -685,6 +685,7 @@ 'OC\\Authentication\\Listeners\\RemoteWipeActivityListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeActivityListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php', + 'OC\\Authentication\\Listeners\\UserDeletedFilesCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php', 'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php', 'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php', 'OC\\Authentication\\Listeners\\UserLoggedInListener' => $baseDir . '/lib/private/Authentication/Listeners/UserLoggedInListener.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index dafae4aa46e3b..2abb4e8f33361 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -714,6 +714,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Authentication\\Listeners\\RemoteWipeActivityListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeActivityListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php', + 'OC\\Authentication\\Listeners\\UserDeletedFilesCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php', 'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php', 'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php', 'OC\\Authentication\\Listeners\\UserLoggedInListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserLoggedInListener.php', diff --git a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php new file mode 100644 index 0000000000000..fba813e0a2470 --- /dev/null +++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php @@ -0,0 +1,73 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Authentication\Listeners; + +use OC\Files\Cache\Cache; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\Files\Config\IMountProviderCollection; +use OCP\Files\Storage\IStorage; +use OCP\User\Events\BeforeUserDeletedEvent; +use OCP\User\Events\UserDeletedEvent; + +class UserDeletedFilesCleanupListener implements IEventListener { + /** @var array */ + private $homeStorageCache = []; + + /** @var IMountProviderCollection */ + private $mountProviderCollection; + + public function __construct(IMountProviderCollection $mountProviderCollection) { + $this->mountProviderCollection = $mountProviderCollection; + } + + public function handle(Event $event): void { + // 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()); + $storage = $userHome->getStorage(); + if (!$storage) { + throw new \Exception("User has no home storage"); + } + $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"); + } + $storage = $this->homeStorageCache[$event->getUser()->getUID()]; + $cache = $storage->getCache(); + if ($cache instanceof Cache) { + $cache->clear(); + } else { + throw new \Exception("Home storage has invalid cache"); + } + $storage->rmdir(''); + } + } +} diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index aa2aeee403b4d..21baea1b78f59 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -153,7 +153,7 @@ public function isCreatable($path) { public function isDeletable($path) { if ($path === '' || $path === '/') { - return false; + return $this->isUpdatable($path); } $parent = dirname($path); return $this->isUpdatable($parent) && $this->isUpdatable($path); diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 5bc42a469d7ee..771cb431cbda3 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -38,7 +38,6 @@ use OC\Accounts\AccountManager; use OC\Avatar\AvatarManager; -use OC\Files\Cache\Storage; use OC\Hooks\Emitter; use OC_Helper; use OCP\EventDispatcher\IEventDispatcher; @@ -221,8 +220,6 @@ public function delete() { $this->emitter->emit('\OC\User', 'preDelete', [$this]); } $this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this)); - // get the home now because it won't return it after user deletion - $homePath = $this->getHome(); $result = $this->backend->deleteUser($this->uid); if ($result) { @@ -241,16 +238,6 @@ public function delete() { // Delete the user's keys in preferences \OC::$server->getConfig()->deleteAllUserValues($this->uid); - // Delete user files in /data/ - if ($homePath !== false) { - // FIXME: this operates directly on FS, should use View instead... - // also this is not testable/mockable... - \OC_Helper::rmdirr($homePath); - } - - // Delete the users entry in the storage table - Storage::remove('home::' . $this->uid); - \OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->uid); \OC::$server->getCommentsManager()->deleteReadMarksFromUser($this); diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 3a9c7766e3c97..629a5632d6111 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -521,6 +521,9 @@ public function testDeleteHooks($result, $expectedHooks) { $commentsManager = $this->createMock(ICommentsManager::class); $notificationManager = $this->createMock(INotificationManager::class); + $config->method('getSystemValue') + ->willReturnArgument(1); + if ($result) { $config->expects($this->once()) ->method('deleteAllUserValues')