better cleanup of user files on user deletion

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/core/Application.php b/core/Application.php
index c36db6a328155c05d5e82c972ee0aa876fab732b..a6c64e81b84d64eba98dbcab24a1824729a1d034 100644 (file)
--- a/core/Application.php
+++ b/core/Application.php
@@ -37,6 +37,7 @@ use OC\Authentication\Events\RemoteWipeStarted;
 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\Listeners\UserDeletedWebAuthnCleanupListener;
@@ -50,6 +51,7 @@ use OC\DB\SchemaWrapper;
 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;
@@ -272,5 +274,7 @@ class Application extends App {
                $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedStoreCleanupListener::class);
                $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedTokenCleanupListener::class);
                $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedWebAuthnCleanupListener::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 9f695ba1d4940b2fb7fc5b1a0c8e644f83626fbf..b70bdb4c55d29faf9de3087003e50fb767444b2b 100644 (file)
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -686,6 +686,7 @@ return array(
     '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\\UserDeletedWebAuthnCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedWebAuthnCleanupListener.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 11af2f48ce3b9b4689a6db8eaa3f9b0849e3b32d..5c6dff0353e5c4861ca38836dc6f99d767e7ed92 100644 (file)
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -715,6 +715,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\\UserDeletedWebAuthnCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedWebAuthnCleanupListener.php',

diff --git a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
new file mode 100644 (file)
index 0000000..fba813e
--- /dev/null
+++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php
@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Robin Appelman <robin@icewind.nl>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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<string,IStorage> */
+       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 aa2aeee403b4de57a1b8dbdd6e640a8c2b08f110..21baea1b78f59018e36dbb591e6b448670250133 100644 (file)
--- a/lib/private/Files/Storage/Common.php
+++ b/lib/private/Files/Storage/Common.php
@@ -153,7 +153,7 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
 
        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 24082926b0d94503867b05bdbfa7f34643af9ef8..33296f7733cb822c009a5a249bbd70219e360e6b 100644 (file)
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -38,7 +38,6 @@ namespace OC\User;
 
 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 @@ class User implements IUser {
                        $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 @@ class User implements IUser {
                        // 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 3a9c7766e3c974b8461200c852595e8e2291696e..629a5632d61110b80117b83625727dde4a3c7933 100644 (file)
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -521,6 +521,9 @@ class UserTest extends TestCase {
                $commentsManager = $this->createMock(ICommentsManager::class);
                $notificationManager = $this->createMock(INotificationManager::class);
 
+               $config->method('getSystemValue')
+                       ->willReturnArgument(1);
+
                if ($result) {
                        $config->expects($this->once())
                                ->method('deleteAllUserValues')