Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: delete all files from object store when user is deleted #40959

Merged
merged 1 commit into from Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
41 changes: 26 additions & 15 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 @@ -184,7 +186,8 @@ public function setUserName($userName) {
}

/**
* get the display name for the user, if no specific display name is set it will fallback to the user id
* get the display name for the user, if no specific display name is set
* it will fall back to the user id
*
* @return string
*/
Expand Down Expand Up @@ -285,19 +288,12 @@ public function getCreationTime() {
*
* @return bool
*/
public function delete() {
public function delete(): 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 +307,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) {
$homeStorage = $fileInfo->getStorage();
$isPrimaryObjectStore = $homeStorage->instanceOfStorage(ObjectStoreStorage::class);
if ($isPrimaryObjectStore) {
/** @var ObjectStoreStorage $homeStorage */
/** @phan-suppress-next-line PhanUndeclaredMethod */
$homeStorage->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
88 changes: 88 additions & 0 deletions tests/lib/Files/ObjectStore/ObjectStoreDeleteAllTest.php
@@ -0,0 +1,88 @@
<?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 OC\Files\Storage\Wrapper\Wrapper;
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();
while ($storage instanceof Wrapper) {
$storage = $storage->getWrapperStorage();
}
if (!$storage instanceof ObjectStoreStorage) {
$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
$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);
}
}