better cleanup of user files on user deletion

Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
Robin Appelman 2021-04-27 15:43:34 +02:00
parent 9de6efd14e
commit ed2d02d5f1
No known key found for this signature in database
GPG Key ID: 42B69D8A64526EFB
7 changed files with 83 additions and 14 deletions

View File

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

View File

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

View File

@ -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',

View File

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

View File

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

View File

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

View File

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