Skip to content

Commit

Permalink
fix: delete all files from object store when user is deleted
Browse files Browse the repository at this point in the history
  • Loading branch information
DeepDiver1975 committed Sep 5, 2023
1 parent 118a4e8 commit 5beff20
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 18 deletions.
24 changes: 20 additions & 4 deletions lib/private/Files/ObjectStore/ObjectStoreStorage.php
Expand Up @@ -27,11 +27,13 @@

use Icewind\Streams\IteratorDirectory;
use OC\Files\Cache\CacheEntry;
use OC\Files\Filesystem;
use OC\Files\Cache\Storage;
use OC\User\User;
use OCP\Files\FileInfo;
use OCP\Files\NotFoundException;
use OCP\Files\ObjectStore\IObjectStore;
use OCP\Files\ObjectStore\IVersionedObjectStorage;
use OCP\IUser;

class ObjectStoreStorage extends \OC\Files\Storage\Common {
/**
Expand All @@ -55,7 +57,7 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common {
*/
protected $availableStorage;
/**
* @var \OC\User\User $user
* @var User $user
*/
protected $user;

Expand Down Expand Up @@ -85,6 +87,20 @@ public function __construct($params) {
}
}

public function removeAllFilesForUser(IUser $user): void {
$storageId = 'object::user:' . $user->getUID();
$numericStorageId = Storage::getNumericStorageId($storageId);
$sql = 'SELECT `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ?';
$result = \OC::$server->getDatabaseConnection()->executeQuery($sql, [$numericStorageId]);
while ($row = $result->fetch()) {
$fileId = $row['fileid'];
$this->objectStore->deleteObject($this->getURN($fileId));
}

# purge db
Storage::remove($storageId);
}

/**
* @return boolean
*/
Expand Down Expand Up @@ -262,9 +278,9 @@ public function stat($path) {
$cacheEntry = $this->getCache()->get($path);
if ($cacheEntry instanceof CacheEntry) {
return $cacheEntry->getData();
} else {
return false;
}

return false;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/private/Files/View.php
Expand Up @@ -1446,6 +1446,9 @@ public function getFileInfo($path, $includeMountPoints = true) {
$path = Filesystem::normalizePath($this->fakeRoot . '/' . $path);

$mount = Filesystem::getMountManager()->find($path);
if ($mount === null) {
return false;
}
$storage = $mount->getStorage();
$internalPath = $mount->getInternalPath($path);
if ($storage) {
Expand Down
52 changes: 38 additions & 14 deletions lib/private/User/User.php
Expand Up @@ -33,6 +33,8 @@
namespace OC\User;

use OC\Files\Cache\Storage;
use OC\Files\ObjectStore\ObjectStoreStorage;
use OC\Files\View;
use OC\Group\Manager;
use OC\Hooks\Emitter;
use OC_Helper;
Expand Down Expand Up @@ -285,19 +287,26 @@ public function getCreationTime() {
*
* @return bool
*/
public function delete() {
public function delete(): bool {
if (\OC::$CLI) {
$this->performDeletion();
return true;
}

# when called from a web request the real deletion is pushed to the command bus
\OC::$server->getCommandBus()->push(new UserDeleteCommand($this->getUID()));
return true;
}

/**
* This operation performs the real deletion. To be called from cron
*/
public function performDeletion(): bool {
if ($this->emitter) {
$this->emitter->emit('\OC\User', 'preDelete', [$this]);
}
// get the home now because it won't return it after user deletion
$homePath = $this->getHome();
$this->mapper->delete($this->account);
$bi = $this->account->getBackendInstance();
if ($bi !== null) {
$bi->deleteUser($this->account->getUserId());
}

// FIXME: Feels like an hack - suggestions?

// We have to delete the user from all groups
foreach (\OC::$server->getGroupManager()->getUserGroups($this) as $group) {
Expand All @@ -311,20 +320,35 @@ public function delete() {
//Delete external storage or remove user from applicableUsers list
\OC::$server->getGlobalStoragesService()->deleteAllForUser($this);

// 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);
$view = new View();
$fileInfo = $view->getFileInfo("/");
if ($fileInfo !== false) {
$honeStorage = $fileInfo->getStorage();
$isPrimaryObjectStore = $honeStorage->instanceOfStorage(ObjectStoreStorage::class);
if ($isPrimaryObjectStore) {
/** @var ObjectStoreStorage $honeStorage */
/** @phan-suppress-next-line PhanUndeclaredMethod */
$honeStorage->removeAllFilesForUser($this);
}
}

// Delete user files in /data/
\OC_Helper::rmdirr($homePath);

// Delete the users entry in the storage table
Storage::remove('home::' . $this->getUID());
Storage::remove('object::user:' . $this->getUID());

\OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->getUID());
\OC::$server->getCommentsManager()->deleteReadMarksFromUser($this);

# finally delete the user and account records
# this way the operation can be re-run in case of errors
$this->mapper->delete($this->account);
$bi = $this->account->getBackendInstance();
if ($bi !== null) {
$bi->deleteUser($this->account->getUserId());
}

if ($this->emitter) {
$this->emitter->emit('\OC\User', 'postDelete', [$this]);
}
Expand Down
20 changes: 20 additions & 0 deletions lib/private/User/UserDeleteCommand.php
@@ -0,0 +1,20 @@
<?php

namespace OC\User;

use OCP\Command\ICommand;

class UserDeleteCommand implements ICommand {
private string $uid;

public function __construct(string $uid) {
$this->uid = $uid;
}

public function handle() {
$user = \OC::$server->getUserManager()->get($this->uid);
if ($user) {
$user->performDeletion();
}
}
}
85 changes: 85 additions & 0 deletions tests/lib/Files/ObjectStore/ObjectStoreDeleteAllTest.php
@@ -0,0 +1,85 @@
<?php
/**
* @author Thomas Müller <thomas.mueller@tmit.eu>
*
* @copyright Copyright (c) 2023, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* 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, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace Test\Files\ObjectStore;

use OC\Files\ObjectStore\ObjectStoreStorage;
use OCP\Files\File;
use Test\TestCase;
use Test\Traits\UserTrait;

/**
* Class ObjectStoreDeleteAllTest
*
* @group DB
*
* @package Test\Files\ObjectStore
*/
class ObjectStoreDeleteAllTest extends TestCase {
use UserTrait;

public function test(): void {
$user = $this->createUser('user1');

$folder = \OC::$server->getUserFolder($user->getUID());
$storage = $folder->getStorage();
if (!$storage->instanceOfStorage(ObjectStoreStorage::class)) {
$this->markTestSkipped();
}

# add some files and folders
$folderFoo = $folder->newFolder('foo');
$file1 = $folder->newFile('foo/lorem.txt');
$file1->putContent('1234567890');
$file2 = $folder->newFile('lorem.txt');
$file2->putContent('1234567890');

# assert objects on storage
$this->assertFileOnObjectStore($storage, $file1);
$this->assertFileOnObjectStore($storage, $file2);

# perform delete
/** @var ObjectStoreStorage $storage */
$storage->removeAllFilesForUser($user);

# assert no objects on storage
$this->assertFileNotOnObjectStore($storage, $file1);
$this->assertFileNotOnObjectStore($storage, $file2);

# assert no records in oc_filecache
self::assertFalse($storage->file_exists($folderFoo->getInternalPath()));
self::assertFalse($storage->file_exists($file2->getInternalPath()));
self::assertFalse($storage->file_exists($file2->getInternalPath()));
}

private function assertFileOnObjectStore($storage, File $file1): void {
$stream = $storage->fopen($file1->getInternalPath(), 'r');
self::assertNotFalse($stream);
$content = stream_get_contents($stream);
fclose($stream);
self::assertEquals('1234567890', $content);
}

private function assertFileNotOnObjectStore($storage, File $file1): void {
$stream = $storage->fopen($file1->getInternalPath(), 'r');
self::assertFalse($stream);
}
}

0 comments on commit 5beff20

Please sign in to comment.