Skip to content

Commit f0c37a1

Browse files
authored
Merge pull request #28400 from nextcloud/backport/26792/stable21
[stable21] better cleanup of user files on user deletion
2 parents 48d97cb + ca3a625 commit f0c37a1

File tree

7 files changed

+83
-14
lines changed

7 files changed

+83
-14
lines changed

core/Application.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use OC\Authentication\Listeners\RemoteWipeActivityListener;
3838
use OC\Authentication\Listeners\RemoteWipeEmailListener;
3939
use OC\Authentication\Listeners\RemoteWipeNotificationsListener;
40+
use OC\Authentication\Listeners\UserDeletedFilesCleanupListener;
4041
use OC\Authentication\Listeners\UserDeletedStoreCleanupListener;
4142
use OC\Authentication\Listeners\UserDeletedTokenCleanupListener;
4243
use OC\Authentication\Listeners\UserDeletedWebAuthnCleanupListener;
@@ -50,6 +51,7 @@
5051
use OCP\AppFramework\App;
5152
use OCP\EventDispatcher\IEventDispatcher;
5253
use OCP\IDBConnection;
54+
use OCP\User\Events\BeforeUserDeletedEvent;
5355
use OCP\User\Events\UserDeletedEvent;
5456
use OCP\Util;
5557
use Symfony\Component\EventDispatcher\GenericEvent;
@@ -272,5 +274,7 @@ function (GenericEvent $event) use ($container) {
272274
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedStoreCleanupListener::class);
273275
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedTokenCleanupListener::class);
274276
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedWebAuthnCleanupListener::class);
277+
$eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, UserDeletedFilesCleanupListener::class);
278+
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedFilesCleanupListener::class);
275279
}
276280
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,7 @@
686686
'OC\\Authentication\\Listeners\\RemoteWipeActivityListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeActivityListener.php',
687687
'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php',
688688
'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php',
689+
'OC\\Authentication\\Listeners\\UserDeletedFilesCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php',
689690
'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php',
690691
'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php',
691692
'OC\\Authentication\\Listeners\\UserDeletedWebAuthnCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedWebAuthnCleanupListener.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
715715
'OC\\Authentication\\Listeners\\RemoteWipeActivityListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeActivityListener.php',
716716
'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php',
717717
'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php',
718+
'OC\\Authentication\\Listeners\\UserDeletedFilesCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php',
718719
'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php',
719720
'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php',
720721
'OC\\Authentication\\Listeners\\UserDeletedWebAuthnCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedWebAuthnCleanupListener.php',
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* @copyright Copyright (c) 2021 Robin Appelman <robin@icewind.nl>
6+
*
7+
* @license GNU AGPL version 3 or any later version
8+
*
9+
* This program is free software: you can redistribute it and/or modify
10+
* it under the terms of the GNU Affero General Public License as
11+
* published by the Free Software Foundation, either version 3 of the
12+
* License, or (at your option) any later version.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU Affero General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU Affero General Public License
20+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
21+
*
22+
*/
23+
24+
namespace OC\Authentication\Listeners;
25+
26+
use OC\Files\Cache\Cache;
27+
use OCP\EventDispatcher\Event;
28+
use OCP\EventDispatcher\IEventListener;
29+
use OCP\Files\Config\IMountProviderCollection;
30+
use OCP\Files\Storage\IStorage;
31+
use OCP\User\Events\BeforeUserDeletedEvent;
32+
use OCP\User\Events\UserDeletedEvent;
33+
34+
class UserDeletedFilesCleanupListener implements IEventListener {
35+
/** @var array<string,IStorage> */
36+
private $homeStorageCache = [];
37+
38+
/** @var IMountProviderCollection */
39+
private $mountProviderCollection;
40+
41+
public function __construct(IMountProviderCollection $mountProviderCollection) {
42+
$this->mountProviderCollection = $mountProviderCollection;
43+
}
44+
45+
public function handle(Event $event): void {
46+
// since we can't reliably get the user home storage after the user is deleted
47+
// but the user deletion might get canceled during the before event
48+
// we only cache the user home storage during the before event and then do the
49+
// action deletion during the after event
50+
51+
if ($event instanceof BeforeUserDeletedEvent) {
52+
$userHome = $this->mountProviderCollection->getHomeMountForUser($event->getUser());
53+
$storage = $userHome->getStorage();
54+
if (!$storage) {
55+
throw new \Exception("User has no home storage");
56+
}
57+
$this->homeStorageCache[$event->getUser()->getUID()] = $storage;
58+
}
59+
if ($event instanceof UserDeletedEvent) {
60+
if (!isset($this->homeStorageCache[$event->getUser()->getUID()])) {
61+
throw new \Exception("UserDeletedEvent fired without matching BeforeUserDeletedEvent");
62+
}
63+
$storage = $this->homeStorageCache[$event->getUser()->getUID()];
64+
$cache = $storage->getCache();
65+
if ($cache instanceof Cache) {
66+
$cache->clear();
67+
} else {
68+
throw new \Exception("Home storage has invalid cache");
69+
}
70+
$storage->rmdir('');
71+
}
72+
}
73+
}

lib/private/Files/Storage/Common.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public function isCreatable($path) {
153153

154154
public function isDeletable($path) {
155155
if ($path === '' || $path === '/') {
156-
return false;
156+
return $this->isUpdatable($path);
157157
}
158158
$parent = dirname($path);
159159
return $this->isUpdatable($parent) && $this->isUpdatable($path);

lib/private/User/User.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838

3939
use OC\Accounts\AccountManager;
4040
use OC\Avatar\AvatarManager;
41-
use OC\Files\Cache\Storage;
4241
use OC\Hooks\Emitter;
4342
use OC_Helper;
4443
use OCP\EventDispatcher\IEventDispatcher;
@@ -221,8 +220,6 @@ public function delete() {
221220
$this->emitter->emit('\OC\User', 'preDelete', [$this]);
222221
}
223222
$this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this));
224-
// get the home now because it won't return it after user deletion
225-
$homePath = $this->getHome();
226223
$result = $this->backend->deleteUser($this->uid);
227224
if ($result) {
228225

@@ -241,16 +238,6 @@ public function delete() {
241238
// Delete the user's keys in preferences
242239
\OC::$server->getConfig()->deleteAllUserValues($this->uid);
243240

244-
// Delete user files in /data/
245-
if ($homePath !== false) {
246-
// FIXME: this operates directly on FS, should use View instead...
247-
// also this is not testable/mockable...
248-
\OC_Helper::rmdirr($homePath);
249-
}
250-
251-
// Delete the users entry in the storage table
252-
Storage::remove('home::' . $this->uid);
253-
254241
\OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->uid);
255242
\OC::$server->getCommentsManager()->deleteReadMarksFromUser($this);
256243

tests/lib/User/UserTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ public function testDeleteHooks($result, $expectedHooks) {
521521
$commentsManager = $this->createMock(ICommentsManager::class);
522522
$notificationManager = $this->createMock(INotificationManager::class);
523523

524+
$config->method('getSystemValue')
525+
->willReturnArgument(1);
526+
524527
if ($result) {
525528
$config->expects($this->once())
526529
->method('deleteAllUserValues')

0 commit comments

Comments
 (0)