Merge pull request #26792 from nextcloud/user-delete-cleanup-files

better cleanup of user files on user deletion

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\Notifications\Notifier as AuthenticationNotifier;
@@ -49,6 +50,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;
@@ -270,5 +272,7 @@ class Application extends App {
 		$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);
 	}
 }

lib/composer/composer/autoload_classmap.php

@@ -684,6 +684,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\\UserLoggedInListener' => $baseDir . '/lib/private/Authentication/Listeners/UserLoggedInListener.php',

lib/composer/composer/autoload_static.php

@@ -713,6 +713,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',

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('');
+		}
+	}
+}

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);

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);
 

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')