From 9883838ef8f444d28f513c3a0bf781b5ab904db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 3 Oct 2024 16:07:02 +0200 Subject: [PATCH 01/18] feat(encryption): Migrate from hooks to events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Listener/FileEventsListener.php | 6 +- lib/base.php | 14 +-- lib/composer/composer/autoload_classmap.php | 2 +- lib/composer/composer/autoload_static.php | 2 +- .../Encryption/EncryptionEventListener.php | 92 +++++++++++++++++++ lib/private/Encryption/EncryptionWrapper.php | 1 - lib/private/Encryption/HookManager.php | 75 --------------- lib/private/Encryption/Update.php | 45 +++------ tests/lib/Encryption/UpdateTest.php | 49 +++------- 9 files changed, 133 insertions(+), 153 deletions(-) create mode 100644 lib/private/Encryption/EncryptionEventListener.php delete mode 100644 lib/private/Encryption/HookManager.php diff --git a/apps/files_versions/lib/Listener/FileEventsListener.php b/apps/files_versions/lib/Listener/FileEventsListener.php index c078f4bc34755..cd4db886b7898 100644 --- a/apps/files_versions/lib/Listener/FileEventsListener.php +++ b/apps/files_versions/lib/Listener/FileEventsListener.php @@ -241,7 +241,7 @@ public function post_write_hook(Node $node): void { /** * Erase versions of deleted file * - * This function is connected to the delete signal of OC_Filesystem + * This function is connected to the NodeDeletedEvent event * cleanup the versions directory if the actual file gets deleted */ public function remove_hook(Node $node): void { @@ -273,7 +273,7 @@ public function pre_remove_hook(Node $node): void { /** * rename/move versions of renamed/moved files * - * This function is connected to the rename signal of OC_Filesystem and adjust the name and location + * This function is connected to the NodeRenamedEvent event and adjust the name and location * of the stored versions along the actual file */ public function rename_hook(Node $source, Node $target): void { @@ -292,7 +292,7 @@ public function rename_hook(Node $source, Node $target): void { /** * copy versions of copied files * - * This function is connected to the copy signal of OC_Filesystem and copies the + * This function is connected to the NodeCopiedEvent event and copies the * the stored versions to the new location */ public function copy_hook(Node $source, Node $target): void { diff --git a/lib/base.php b/lib/base.php index 25f978df836c3..879ba92104d39 100644 --- a/lib/base.php +++ b/lib/base.php @@ -6,9 +6,9 @@ * SPDX-FileCopyrightText: 2013-2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ -use OC\Encryption\HookManager; use OC\Share20\Hooks; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Events\BeforeFileSystemSetupEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\ILogger; use OCP\IRequest; @@ -16,7 +16,6 @@ use OCP\IUserSession; use OCP\Security\Bruteforce\IThrottler; use OCP\Server; -use OCP\Share; use OCP\User\Events\UserChangedEvent; use Psr\Log\LoggerInterface; use Symfony\Component\Routing\Exception\MethodNotAllowedException; @@ -868,15 +867,16 @@ public static function registerCleanupHooks(\OC\SystemConfig $systemConfig): voi } private static function registerEncryptionWrapperAndHooks(): void { + /** @var \OC\Encryption\Manager */ $manager = Server::get(\OCP\Encryption\IManager::class); - \OCP\Util::connectHook('OC_Filesystem', 'preSetup', $manager, 'setupStorage'); + Server::get(IEventDispatcher::class)->addListener( + BeforeFileSystemSetupEvent::class, + $manager->setupStorage(...), + ); $enabled = $manager->isEnabled(); if ($enabled) { - \OCP\Util::connectHook(Share::class, 'post_shared', HookManager::class, 'postShared'); - \OCP\Util::connectHook(Share::class, 'post_unshare', HookManager::class, 'postUnshared'); - \OCP\Util::connectHook('OC_Filesystem', 'post_rename', HookManager::class, 'postRename'); - \OCP\Util::connectHook('\OCA\Files_Trashbin\Trashbin', 'post_restore', HookManager::class, 'postRestore'); + \OC\Encryption\EncryptionEventListener::register(Server::get(IEventDispatcher::class)); } } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index cbadc2deb15b4..00ff9165b3b73 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1515,6 +1515,7 @@ 'OC\\DirectEditing\\Token' => $baseDir . '/lib/private/DirectEditing/Token.php', 'OC\\EmojiHelper' => $baseDir . '/lib/private/EmojiHelper.php', 'OC\\Encryption\\DecryptAll' => $baseDir . '/lib/private/Encryption/DecryptAll.php', + 'OC\\Encryption\\EncryptionEventListener' => $baseDir . '/lib/private/Encryption/EncryptionEventListener.php', 'OC\\Encryption\\EncryptionWrapper' => $baseDir . '/lib/private/Encryption/EncryptionWrapper.php', 'OC\\Encryption\\Exceptions\\DecryptionFailedException' => $baseDir . '/lib/private/Encryption/Exceptions/DecryptionFailedException.php', 'OC\\Encryption\\Exceptions\\EmptyEncryptionDataException' => $baseDir . '/lib/private/Encryption/Exceptions/EmptyEncryptionDataException.php', @@ -1525,7 +1526,6 @@ 'OC\\Encryption\\Exceptions\\ModuleDoesNotExistsException' => $baseDir . '/lib/private/Encryption/Exceptions/ModuleDoesNotExistsException.php', 'OC\\Encryption\\Exceptions\\UnknownCipherException' => $baseDir . '/lib/private/Encryption/Exceptions/UnknownCipherException.php', 'OC\\Encryption\\File' => $baseDir . '/lib/private/Encryption/File.php', - 'OC\\Encryption\\HookManager' => $baseDir . '/lib/private/Encryption/HookManager.php', 'OC\\Encryption\\Keys\\Storage' => $baseDir . '/lib/private/Encryption/Keys/Storage.php', 'OC\\Encryption\\Manager' => $baseDir . '/lib/private/Encryption/Manager.php', 'OC\\Encryption\\Update' => $baseDir . '/lib/private/Encryption/Update.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 10086cabce066..e07c252f75ed1 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1564,6 +1564,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\DirectEditing\\Token' => __DIR__ . '/../../..' . '/lib/private/DirectEditing/Token.php', 'OC\\EmojiHelper' => __DIR__ . '/../../..' . '/lib/private/EmojiHelper.php', 'OC\\Encryption\\DecryptAll' => __DIR__ . '/../../..' . '/lib/private/Encryption/DecryptAll.php', + 'OC\\Encryption\\EncryptionEventListener' => __DIR__ . '/../../..' . '/lib/private/Encryption/EncryptionEventListener.php', 'OC\\Encryption\\EncryptionWrapper' => __DIR__ . '/../../..' . '/lib/private/Encryption/EncryptionWrapper.php', 'OC\\Encryption\\Exceptions\\DecryptionFailedException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/DecryptionFailedException.php', 'OC\\Encryption\\Exceptions\\EmptyEncryptionDataException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/EmptyEncryptionDataException.php', @@ -1574,7 +1575,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Encryption\\Exceptions\\ModuleDoesNotExistsException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/ModuleDoesNotExistsException.php', 'OC\\Encryption\\Exceptions\\UnknownCipherException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/UnknownCipherException.php', 'OC\\Encryption\\File' => __DIR__ . '/../../..' . '/lib/private/Encryption/File.php', - 'OC\\Encryption\\HookManager' => __DIR__ . '/../../..' . '/lib/private/Encryption/HookManager.php', 'OC\\Encryption\\Keys\\Storage' => __DIR__ . '/../../..' . '/lib/private/Encryption/Keys/Storage.php', 'OC\\Encryption\\Manager' => __DIR__ . '/../../..' . '/lib/private/Encryption/Manager.php', 'OC\\Encryption\\Update' => __DIR__ . '/../../..' . '/lib/private/Encryption/Update.php', diff --git a/lib/private/Encryption/EncryptionEventListener.php b/lib/private/Encryption/EncryptionEventListener.php new file mode 100644 index 0000000000000..a22661e9f7503 --- /dev/null +++ b/lib/private/Encryption/EncryptionEventListener.php @@ -0,0 +1,92 @@ + */ +class EncryptionEventListener implements IEventListener { + private ?Update $updater = null; + + public function __construct( + private IUserSession $userSession, + private SetupManager $setupManager, + ) { + } + + public static function register(IEventDispatcher $dispatcher): void { + $dispatcher->addServiceListener(NodeRenamedEvent::class, static::class); + $dispatcher->addServiceListener(ShareCreatedEvent::class, static::class); + $dispatcher->addServiceListener(ShareDeletedEvent::class, static::class); + $dispatcher->addServiceListener(NodeRestoredEvent::class, static::class); + } + + public function handle(Event $event): void { + if ($event instanceof NodeRenamedEvent) { + $this->getUpdate()->postRename($event->getSource() instanceof Folder, $event->getSource()->getPath(), $event->getTarget()->getPath()); + } elseif ($event instanceof ShareCreatedEvent) { + $this->getUpdate()->postShared($event->getShare()->getNodeType(), $event->getShare()->getNodeId()); + } elseif ($event instanceof ShareDeletedEvent) { + // In case the unsharing happens in a background job, we don't have + // a session and we load instead the user from the UserManager + $owner = $event->getShare()->getNode()->getOwner(); + $this->getUpdate($owner)->postUnshared($event->getShare()->getNodeType(), $event->getShare()->getNodeId()); + } elseif ($event instanceof NodeRestoredEvent) { + $this->getUpdate()->postRestore($event->getTarget() instanceof Folder, $event->getTarget()->getPath()); + } + } + + private function getUpdate(?IUser $owner = null): Update { + if (is_null($this->updater)) { + $user = $this->userSession->getUser(); + if (!$user && ($owner !== null)) { + $user = $owner; + } + if (!$user) { + throw new \Exception('Inconsistent data, File unshared, but owner not found. Should not happen'); + } + + $uid = $user->getUID(); + + if (!$this->setupManager->isSetupComplete($user)) { + $this->setupManager->setupForUser($user); + } + + $this->updater = new Update( + new Util( + new View(), + \OC::$server->getUserManager(), + \OC::$server->getGroupManager(), + \OC::$server->getConfig()), + Filesystem::getMountManager(), + \OC::$server->getEncryptionManager(), + \OC::$server->get(IFile::class), + \OC::$server->get(LoggerInterface::class), + $uid + ); + } + + return $this->updater; + } +} diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index 7f355b603d620..6a19c1cccaa96 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -76,7 +76,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $ \OC::$server->getConfig() ); $update = new Update( - new View(), $util, Filesystem::getMountManager(), $this->manager, diff --git a/lib/private/Encryption/HookManager.php b/lib/private/Encryption/HookManager.php deleted file mode 100644 index 39e7edabb9561..0000000000000 --- a/lib/private/Encryption/HookManager.php +++ /dev/null @@ -1,75 +0,0 @@ -postShared($params); - } - public static function postUnshared($params): void { - // In case the unsharing happens in a background job, we don't have - // a session and we load instead the user from the UserManager - $path = Filesystem::getPath($params['fileSource']); - $owner = Filesystem::getOwner($path); - self::getUpdate($owner)->postUnshared($params); - } - - public static function postRename($params): void { - self::getUpdate()->postRename($params); - } - - public static function postRestore($params): void { - self::getUpdate()->postRestore($params); - } - - private static function getUpdate(?string $owner = null): Update { - if (is_null(self::$updater)) { - $user = \OC::$server->getUserSession()->getUser(); - if (!$user && $owner) { - $user = \OC::$server->getUserManager()->get($owner); - } - if (!$user) { - throw new \Exception('Inconsistent data, File unshared, but owner not found. Should not happen'); - } - - $uid = ''; - if ($user) { - $uid = $user->getUID(); - } - - $setupManager = \OC::$server->get(SetupManager::class); - if (!$setupManager->isSetupComplete($user)) { - $setupManager->setupForUser($user); - } - - self::$updater = new Update( - new View(), - new Util( - new View(), - \OC::$server->getUserManager(), - \OC::$server->getGroupManager(), - \OC::$server->getConfig()), - Filesystem::getMountManager(), - \OC::$server->getEncryptionManager(), - \OC::$server->get(IFile::class), - \OC::$server->get(LoggerInterface::class), - $uid - ); - } - - return self::$updater; - } -} diff --git a/lib/private/Encryption/Update.php b/lib/private/Encryption/Update.php index 0b27d63c19a2a..d3f9c0ebef7cd 100644 --- a/lib/private/Encryption/Update.php +++ b/lib/private/Encryption/Update.php @@ -18,9 +18,6 @@ * update encrypted files, e.g. because a file was shared */ class Update { - /** @var View */ - protected $view; - /** @var Util */ protected $util; @@ -43,7 +40,6 @@ class Update { * @param string $uid */ public function __construct( - View $view, Util $util, Mount\Manager $mountManager, Manager $encryptionManager, @@ -51,7 +47,6 @@ public function __construct( LoggerInterface $logger, $uid, ) { - $this->view = $view; $this->util = $util; $this->mountManager = $mountManager; $this->encryptionManager = $encryptionManager; @@ -62,32 +57,28 @@ public function __construct( /** * hook after file was shared - * - * @param array $params */ - public function postShared($params) { + public function postShared(string $nodeType, int $nodeId): void { if ($this->encryptionManager->isEnabled()) { - if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') { - $path = Filesystem::getPath($params['fileSource']); + if ($nodeType === 'file' || $nodeType === 'folder') { + $path = Filesystem::getPath($nodeId); [$owner, $ownerPath] = $this->getOwnerPath($path); $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($absPath); + $this->update($nodeType === 'folder', $absPath); } } } /** * hook after file was unshared - * - * @param array $params */ - public function postUnshared($params) { + public function postUnshared(string $nodeType, int $nodeId): void { if ($this->encryptionManager->isEnabled()) { - if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') { - $path = Filesystem::getPath($params['fileSource']); + if ($nodeType === 'file' || $nodeType === 'folder') { + $path = Filesystem::getPath($nodeId); [$owner, $ownerPath] = $this->getOwnerPath($path); $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($absPath); + $this->update($nodeType === 'folder', $absPath); } } } @@ -95,32 +86,26 @@ public function postUnshared($params) { /** * inform encryption module that a file was restored from the trash bin, * e.g. to update the encryption keys - * - * @param array $params */ - public function postRestore($params) { + public function postRestore(bool $directory, string $filePath): void { if ($this->encryptionManager->isEnabled()) { - $path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $params['filePath']); - $this->update($path); + $path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $filePath); + $this->update($directory, $path); } } /** * inform encryption module that a file was renamed, * e.g. to update the encryption keys - * - * @param array $params */ - public function postRename($params) { - $source = $params['oldpath']; - $target = $params['newpath']; + public function postRename(bool $directory, string $source, string $target): void { if ( $this->encryptionManager->isEnabled() && dirname($source) !== dirname($target) ) { [$owner, $ownerPath] = $this->getOwnerPath($target); $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($absPath); + $this->update($directory, $absPath); } } @@ -149,7 +134,7 @@ protected function getOwnerPath($path) { * @param string $path relative to data/ * @throws Exceptions\ModuleDoesNotExistsException */ - public function update($path) { + public function update(bool $directory, string $path): void { $encryptionModule = $this->encryptionManager->getEncryptionModule(); // if the encryption module doesn't encrypt the files on a per-user basis @@ -159,7 +144,7 @@ public function update($path) { } // if a folder was shared, get a list of all (sub-)folders - if ($this->view->is_dir($path)) { + if ($directory) { $allFiles = $this->util->getAllFiles($path); } else { $allFiles = [$path]; diff --git a/tests/lib/Encryption/UpdateTest.php b/tests/lib/Encryption/UpdateTest.php index 2627e18601d26..d54aeab35ddf3 100644 --- a/tests/lib/Encryption/UpdateTest.php +++ b/tests/lib/Encryption/UpdateTest.php @@ -13,36 +13,21 @@ use OC\Files\Mount\Manager; use OC\Files\View; use OCP\Encryption\IEncryptionModule; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; class UpdateTest extends TestCase { - /** @var \OC\Encryption\Update */ - private $update; + private Update $update; - /** @var string */ - private $uid; - - /** @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject */ - private $view; - - /** @var Util | \PHPUnit\Framework\MockObject\MockObject */ - private $util; - - /** @var \OC\Files\Mount\Manager | \PHPUnit\Framework\MockObject\MockObject */ - private $mountManager; - - /** @var \OC\Encryption\Manager | \PHPUnit\Framework\MockObject\MockObject */ - private $encryptionManager; - - /** @var \OCP\Encryption\IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject */ - private $encryptionModule; - - /** @var \OC\Encryption\File | \PHPUnit\Framework\MockObject\MockObject */ - private $fileHelper; - - /** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */ - private $logger; + private string $uid; + private View&MockObject $view; + private Util&MockObject $util; + private Manager&MockObject $mountManager; + private \OC\Encryption\Manager&MockObject $encryptionManager; + private IEncryptionModule&MockObject $encryptionModule; + private File&MockObject $fileHelper; + private LoggerInterface&MockObject $logger; protected function setUp(): void { parent::setUp(); @@ -58,7 +43,6 @@ protected function setUp(): void { $this->uid = 'testUser1'; $this->update = new Update( - $this->view, $this->util, $this->mountManager, $this->encryptionManager, @@ -80,10 +64,6 @@ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { ->method('getEncryptionModule') ->willReturn($this->encryptionModule); - $this->view->expects($this->once()) - ->method('is_dir') - ->willReturn($isDir); - if ($isDir) { $this->util->expects($this->once()) ->method('getAllFiles') @@ -98,7 +78,7 @@ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { ->method('update') ->willReturn(true); - $this->update->update($path); + $this->update->update($isDir, $path); } /** @@ -143,7 +123,7 @@ public function testPostRename($source, $target, $encryptionEnabled): void { $updateMock->expects($this->once())->method('update'); } - $updateMock->postRename(['oldpath' => $source, 'newpath' => $target]); + $updateMock->postRename(false, $source, $target); } /** @@ -181,7 +161,7 @@ public function testPostRestore($encryptionEnabled): void { $updateMock->expects($this->never())->method('update'); } - $updateMock->postRestore(['filePath' => '/folder/test.txt']); + $updateMock->postRestore(false, '/folder/test.txt'); } /** @@ -200,13 +180,12 @@ public function dataTestPostRestore() { * create mock of the update method * * @param array $methods methods which should be set - * @return \OC\Encryption\Update | \PHPUnit\Framework\MockObject\MockObject + * @return \OC\Encryption\Update | MockObject */ protected function getUpdateMock($methods) { return $this->getMockBuilder('\OC\Encryption\Update') ->setConstructorArgs( [ - $this->view, $this->util, $this->mountManager, $this->encryptionManager, From b611bcfba0b1348b187e02d8d7bfead2a5c29dc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 5 Nov 2024 17:30:42 +0100 Subject: [PATCH 02/18] fix(tests): Unregister encryption modules in ViewTest to avoid side effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was clearing the hooks with the same results before Signed-off-by: Côme Chilliet --- tests/lib/Files/ViewTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index a6a7722f363bb..1e2e48aa2625b 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -99,6 +99,12 @@ class ViewTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); \OC_Hook::clear(); + /* Disable encryption, this is not what we want to test */ + $encryptionManager = Server::get(\OCP\Encryption\IManager::class); + $encryptionModules = $encryptionManager->getEncryptionModules(); + foreach (array_keys($encryptionModules) as $encryptionModuleId) { + $encryptionManager->unregisterEncryptionModule($encryptionModuleId); + } \OC_User::clearBackends(); \OC_User::useBackend(new \Test\Util\User\Dummy()); From 98a23c6f4f8c6d463f613974bd860d0cb6d5461a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 11:07:40 +0100 Subject: [PATCH 03/18] fix(tests): Remove Encryption disabling in ViewTest to avoid side effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapt tests a bit to make them pass with Encryption wrapper registered Signed-off-by: Côme Chilliet --- tests/lib/Files/ViewTest.php | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 1e2e48aa2625b..bbe9c2b3ee8e2 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -99,12 +99,6 @@ class ViewTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); \OC_Hook::clear(); - /* Disable encryption, this is not what we want to test */ - $encryptionManager = Server::get(\OCP\Encryption\IManager::class); - $encryptionModules = $encryptionManager->getEncryptionModules(); - foreach (array_keys($encryptionModules) as $encryptionModuleId) { - $encryptionManager->unregisterEncryptionModule($encryptionModuleId); - } \OC_User::clearBackends(); \OC_User::useBackend(new \Test\Util\User\Dummy()); @@ -512,10 +506,10 @@ public function testMoveBetweenStorageCrossNonLocal(): void { } public function moveBetweenStorages($storage1, $storage2) { - Filesystem::mount($storage1, [], '/'); - Filesystem::mount($storage2, [], '/substorage'); + Filesystem::mount($storage1, [], '/' . $this->user . '/'); + Filesystem::mount($storage2, [], '/' . $this->user . '/substorage'); - $rootView = new View(''); + $rootView = new View('/' . $this->user); $rootView->rename('foo.txt', 'substorage/folder/foo.txt'); $this->assertFalse($rootView->file_exists('foo.txt')); $this->assertTrue($rootView->file_exists('substorage/folder/foo.txt')); @@ -935,14 +929,16 @@ public function testPartFileInfo(): void { $storage = new Temporary([]); $scanner = $storage->getScanner(); Filesystem::mount($storage, [], '/test/'); - $storage->file_put_contents('test.part', 'foobar'); + $sizeWritten = $storage->file_put_contents('test.part', 'foobar'); $scanner->scan(''); $view = new View('/test'); $info = $view->getFileInfo('test.part'); $this->assertInstanceOf('\OCP\Files\FileInfo', $info); $this->assertNull($info->getId()); + $this->assertEquals(6, $sizeWritten); $this->assertEquals(6, $info->getSize()); + $this->assertEquals('foobar', $view->file_get_contents('test.part')); } public function absolutePathProvider() { From d60355db611a5c9eb13456698fdc9189e32d4434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 11:10:11 +0100 Subject: [PATCH 04/18] fix(encryption): Fix filesize for part files in Encryption wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Files/Storage/Wrapper/Encryption.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index ba23f3c43ecd3..f033c66ae428d 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -64,7 +64,8 @@ public function filesize(string $path): int|float|false { $info = $this->getCache()->get($path); if ($info === false) { - return false; + /* Pass call to wrapped storage, it may be a special file like a part file */ + return $this->storage->filesize($path); } if (isset($this->unencryptedSize[$fullPath])) { $size = $this->unencryptedSize[$fullPath]; @@ -318,7 +319,7 @@ public function fopen(string $path, string $mode) { if (!empty($encryptionModuleId)) { $encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId); $shouldEncrypt = true; - } elseif (empty($encryptionModuleId) && $info['encrypted'] === true) { + } elseif ($info !== false && $info['encrypted'] === true) { // we come from a old installation. No header and/or no module defined // but the file is encrypted. In this case we need to use the // OC_DEFAULT_MODULE to read the file From ab6cd32534032b4fe7dc32543a5f4351d13eca39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 11:41:31 +0100 Subject: [PATCH 05/18] fix: Fix mtime preservation when moving a directory across storages with encryption registered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Files/Storage/Wrapper/Encryption.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index f033c66ae428d..0577215b10197 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -655,7 +655,7 @@ private function copyBetweenStorage( if (is_resource($dh)) { while ($result && ($file = readdir($dh)) !== false) { if (!Filesystem::isIgnoredDir($file)) { - $result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file, false, $isRename); + $result = $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file, $preserveMtime, $isRename); } } } From 31fd585b06fbb2fde9e21e79824b5e80337651d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 13:57:55 +0100 Subject: [PATCH 06/18] fix(tests): Disable encryption wrapper when it makes sense MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/Files/ViewTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index bbe9c2b3ee8e2..350858f630a78 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -25,6 +25,7 @@ use OCP\Files\GenericFileException; use OCP\Files\Mount\IMountManager; use OCP\Files\Storage\IStorage; +use OCP\Files\Storage\IStorageFactory; use OCP\IDBConnection; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; @@ -1937,6 +1938,8 @@ public function testLockBasicOperation( /* Pause trash to avoid the trashbin intercepting rmdir and unlink calls */ Server::get(ITrashManager::class)->pauseTrash(); + /* Same thing with encryption wrapper */ + Server::get(IStorageFactory::class)->removeStorageWrapper('oc_encryption'); Filesystem::mount($storage, [], $this->user . '/'); @@ -2087,6 +2090,8 @@ public function testLockBasicOperationUnlocksAfterException( /* Pause trash to avoid the trashbin intercepting rmdir and unlink calls */ Server::get(ITrashManager::class)->pauseTrash(); + /* Same thing with encryption wrapper */ + Server::get(IStorageFactory::class)->removeStorageWrapper('oc_encryption'); Filesystem::mount($storage, [], $this->user . '/'); @@ -2228,6 +2233,9 @@ public function testLockFileRename($operation, $expectedLockTypeSourceDuring): v $sourcePath = 'original.txt'; $targetPath = 'target.txt'; + /* Disable encryption wrapper to avoid it intercepting mocked call */ + Server::get(IStorageFactory::class)->removeStorageWrapper('oc_encryption'); + Filesystem::mount($storage, [], $this->user . '/'); $storage->mkdir('files'); $view->file_put_contents($sourcePath, 'meh'); @@ -2281,6 +2289,9 @@ public function testLockFileCopyException(): void { $sourcePath = 'original.txt'; $targetPath = 'target.txt'; + /* Disable encryption wrapper to avoid it intercepting mocked call */ + Server::get(IStorageFactory::class)->removeStorageWrapper('oc_encryption'); + Filesystem::mount($storage, [], $this->user . '/'); $storage->mkdir('files'); $view->file_put_contents($sourcePath, 'meh'); @@ -2417,6 +2428,9 @@ public function testLockFileRenameCrossStorage($viewOperation, $storageOperation $sourcePath = 'original.txt'; $targetPath = 'substorage/target.txt'; + /* Disable encryption wrapper to avoid it intercepting mocked call */ + Server::get(IStorageFactory::class)->removeStorageWrapper('oc_encryption'); + Filesystem::mount($storage, [], $this->user . '/'); Filesystem::mount($storage2, [], $this->user . '/files/substorage'); $storage->mkdir('files'); From cf60adf0498c0cb2244b980be3c4a34418469f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 14:54:42 +0100 Subject: [PATCH 07/18] chore(files_versions): Only mock getSystemValue method to avoid problems in files_versions tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_versions/tests/VersioningTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index 20a0a783ecc87..cc09c89068b2f 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -19,7 +19,6 @@ use OCA\Files_Versions\Versions\IVersionManager; use OCP\Constants; use OCP\Files\IMimeTypeLoader; -use OCP\IConfig; use OCP\IUser; use OCP\Server; use OCP\Share\IShare; @@ -79,7 +78,10 @@ protected function setUp(): void { parent::setUp(); $config = \OC::$server->getConfig(); - $mockConfig = $this->createMock(IConfig::class); + $mockConfig = $this->getMockBuilder(AllConfig::class) + ->onlyMethods(['getSystemValue']) + ->setConstructorArgs([Server::get(\OC\SystemConfig::class)]) + ->getMock(); $mockConfig->expects($this->any()) ->method('getSystemValue') ->willReturnCallback(function ($key, $default) use ($config) { From e209d6524c0a8acf3171f408c875bc92f104bb45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 17:37:09 +0100 Subject: [PATCH 08/18] chore(trashbin): Fix configuration mocking in trashbin tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_trashbin/tests/TrashbinTest.php | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php index ea9abedd14489..b30e9d458ad4b 100644 --- a/apps/files_trashbin/tests/TrashbinTest.php +++ b/apps/files_trashbin/tests/TrashbinTest.php @@ -19,7 +19,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Constants; use OCP\Files\FileInfo; -use OCP\IConfig; +use OCP\Server; use OCP\Share\IShare; /** @@ -110,8 +110,11 @@ protected function setUp(): void { \OC::$server->getAppManager()->enableApp('files_trashbin'); $config = \OC::$server->getConfig(); - $mockConfig = $this->createMock(IConfig::class); - $mockConfig + $mockConfig = $this->getMockBuilder(AllConfig::class) + ->onlyMethods(['getSystemValue']) + ->setConstructorArgs([Server::get(\OC\SystemConfig::class)]) + ->getMock(); + $mockConfig->expects($this->any()) ->method('getSystemValue') ->willReturnCallback(static function ($key, $default) use ($config) { if ($key === 'filesystem_check_changes') { @@ -120,16 +123,6 @@ protected function setUp(): void { return $config->getSystemValue($key, $default); } }); - $mockConfig - ->method('getUserValue') - ->willReturnCallback(static function ($userId, $appName, $key, $default = '') use ($config) { - return $config->getUserValue($userId, $appName, $key, $default); - }); - $mockConfig - ->method('getAppValue') - ->willReturnCallback(static function ($appName, $key, $default = '') use ($config) { - return $config->getAppValue($appName, $key, $default); - }); $this->overwriteService(AllConfig::class, $mockConfig); $this->trashRoot1 = '/' . self::TEST_TRASHBIN_USER1 . '/files_trashbin'; From eb56ecda4c03b5eb3acfce22276be774bde3ce3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Nov 2024 17:38:33 +0100 Subject: [PATCH 09/18] chore: Update psalm baseline to remove fixed issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline.xml | 3 --- 1 file changed, 3 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 2824f99f79d1c..1b646654ebd6a 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2031,9 +2031,6 @@ - - copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file, false, $isRename)]]> - From 9f8707b7b99d0221248e45a6fc91a25af12051b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 28 Nov 2024 12:08:57 +0100 Subject: [PATCH 10/18] fix(tests): Avoid user login before a private key is setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_sharing/tests/EncryptedSizePropagationTest.php | 7 ++++++- apps/files_sharing/tests/TestCase.php | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/tests/EncryptedSizePropagationTest.php b/apps/files_sharing/tests/EncryptedSizePropagationTest.php index af2cf379358e3..53da510f9f14e 100644 --- a/apps/files_sharing/tests/EncryptedSizePropagationTest.php +++ b/apps/files_sharing/tests/EncryptedSizePropagationTest.php @@ -16,12 +16,17 @@ class EncryptedSizePropagationTest extends SizePropagationTest { use EncryptionTrait; protected function setupUser($name, $password = '') { + $this->config->setAppValue('encryption', 'useMasterKey', '0'); $this->createUser($name, $password); $tmpFolder = \OC::$server->getTempManager()->getTemporaryFolder(); $this->registerMount($name, '\OC\Files\Storage\Local', '/' . $name, ['datadir' => $tmpFolder]); - $this->config->setAppValue('encryption', 'useMasterKey', '0'); $this->setupForUser($name, $password); $this->loginWithEncryption($name); return new View('/' . $name . '/files'); } + + protected function loginHelper($user, $create = false, $password = false) { + $this->setupForUser($user, $password); + parent::loginHelper($user, $create, $password); + } } diff --git a/apps/files_sharing/tests/TestCase.php b/apps/files_sharing/tests/TestCase.php index d12a6bf0ec2c3..b05c597b6bea6 100644 --- a/apps/files_sharing/tests/TestCase.php +++ b/apps/files_sharing/tests/TestCase.php @@ -102,7 +102,7 @@ protected function setUp(): void { \OC::$server->get(DisplayNameCache::class)->clear(); //login as user1 - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); $this->data = 'foobar'; $this->view = new View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files'); @@ -167,7 +167,7 @@ public static function tearDownAfterClass(): void { * @param bool $create * @param bool $password */ - protected static function loginHelper($user, $create = false, $password = false) { + protected function loginHelper($user, $create = false, $password = false) { if ($password === false) { $password = $user; } From 68bbe63ed30a418c61181306abdefcfbefb9ca36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 28 Nov 2024 16:57:45 +0100 Subject: [PATCH 11/18] fix(admin_audit): Survive if file change id after rename (it should not) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/admin_audit/lib/Actions/Files.php | 23 ++------------------ apps/admin_audit/lib/AppInfo/Application.php | 8 ------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/apps/admin_audit/lib/Actions/Files.php b/apps/admin_audit/lib/Actions/Files.php index 3993f1a76ee11..8e833812f552a 100644 --- a/apps/admin_audit/lib/Actions/Files.php +++ b/apps/admin_audit/lib/Actions/Files.php @@ -10,7 +10,6 @@ use OC\Files\Node\NonExistingFile; use OCP\Files\Events\Node\BeforeNodeDeletedEvent; use OCP\Files\Events\Node\BeforeNodeReadEvent; -use OCP\Files\Events\Node\BeforeNodeRenamedEvent; use OCP\Files\Events\Node\NodeCopiedEvent; use OCP\Files\Events\Node\NodeCreatedEvent; use OCP\Files\Events\Node\NodeRenamedEvent; @@ -26,9 +25,6 @@ * @package OCA\AdminAudit\Actions */ class Files extends Action { - - private array $renamedNodes = []; - /** * Logs file read actions */ @@ -52,31 +48,16 @@ public function read(BeforeNodeReadEvent $event): void { ); } - /** - * Logs rename actions of files - */ - public function beforeRename(BeforeNodeRenamedEvent $event): void { - try { - $source = $event->getSource(); - $this->renamedNodes[$source->getId()] = $source; - } catch (InvalidPathException|NotFoundException $e) { - Server::get(LoggerInterface::class)->error( - 'Exception thrown in file rename: ' . $e->getMessage(), ['app' => 'admin_audit', 'exception' => $e] - ); - return; - } - } - /** * Logs rename actions of files */ public function afterRename(NodeRenamedEvent $event): void { try { $target = $event->getTarget(); - $originalSource = $this->renamedNodes[$target->getId()]; + $source = $event->getSource(); $params = [ 'newid' => $target->getId(), - 'oldpath' => mb_substr($originalSource->getInternalPath(), 5), + 'oldpath' => mb_substr($source->getInternalPath(), 5), 'newpath' => mb_substr($target->getInternalPath(), 5), ]; } catch (InvalidPathException|NotFoundException $e) { diff --git a/apps/admin_audit/lib/AppInfo/Application.php b/apps/admin_audit/lib/AppInfo/Application.php index 201a8fe255a93..08c13c66d595b 100644 --- a/apps/admin_audit/lib/AppInfo/Application.php +++ b/apps/admin_audit/lib/AppInfo/Application.php @@ -41,7 +41,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\BeforeNodeDeletedEvent; use OCP\Files\Events\Node\BeforeNodeReadEvent; -use OCP\Files\Events\Node\BeforeNodeRenamedEvent; use OCP\Files\Events\Node\NodeCopiedEvent; use OCP\Files\Events\Node\NodeCreatedEvent; use OCP\Files\Events\Node\NodeRenamedEvent; @@ -168,13 +167,6 @@ private function tagHooks(IAuditLogger $logger, private function fileHooks(IAuditLogger $logger, IEventDispatcher $eventDispatcher): void { $fileActions = new Files($logger); - $eventDispatcher->addListener( - BeforeNodeRenamedEvent::class, - function (BeforeNodeRenamedEvent $event) use ($fileActions): void { - $fileActions->beforeRename($event); - } - ); - $eventDispatcher->addListener( NodeRenamedEvent::class, function (NodeRenamedEvent $event) use ($fileActions): void { From 60ce725dd25efe91b63182498a19cfbab7040c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 09:56:49 +0100 Subject: [PATCH 12/18] fix(encryption): Fix a PHP error in Encryption Util in specific situations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Encryption/Util.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Encryption/Util.php b/lib/private/Encryption/Util.php index 1fb08b15696ec..0566ab9a760cb 100644 --- a/lib/private/Encryption/Util.php +++ b/lib/private/Encryption/Util.php @@ -304,7 +304,7 @@ public function isExcluded($path) { // detect user specific folders if ($this->userManager->userExists($root[1]) - && in_array($root[2], $this->excludedPaths)) { + && in_array($root[2] ?? '', $this->excludedPaths)) { return true; } } From b1260a468e5fd8448c472112696147ccabb3b5c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 13 Jan 2025 11:11:54 +0100 Subject: [PATCH 13/18] fix: Preserve file id when moving from object store even if encryption wrapper is present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Files/Storage/Wrapper/Encryption.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 0577215b10197..a416e61bd2fbb 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -527,10 +527,21 @@ public function moveFromStorage( $result = $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, true); if ($result) { - if ($sourceStorage->is_dir($sourceInternalPath)) { - $result = $sourceStorage->rmdir($sourceInternalPath); - } else { - $result = $sourceStorage->unlink($sourceInternalPath); + if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) { + /** @var ObjectStoreStorage $sourceStorage */ + $sourceStorage->setPreserveCacheOnDelete(true); + } + try { + if ($sourceStorage->is_dir($sourceInternalPath)) { + $result = $sourceStorage->rmdir($sourceInternalPath); + } else { + $result = $sourceStorage->unlink($sourceInternalPath); + } + } finally { + if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) { + /** @var ObjectStoreStorage $sourceStorage */ + $sourceStorage->setPreserveCacheOnDelete(false); + } } } return $result; From 07c3c99c46286f638d1c1021c3a225abcf15209f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 16 Jan 2025 18:04:54 +0100 Subject: [PATCH 14/18] chore: Assert rename success in versionning tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_versions/tests/VersioningTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index cc09c89068b2f..707c4a8c6b16b 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -425,8 +425,9 @@ public function testMoveFileIntoSharedFolderAsRecipient(): void { $this->rootView->file_put_contents($v2, 'version2'); // move file into the shared folder as recipient - Filesystem::rename('/test.txt', '/folder1/test.txt'); + $success = Filesystem::rename('/test.txt', '/folder1/test.txt'); + $this->assertTrue($success); $this->assertFalse($this->rootView->file_exists($v1)); $this->assertFalse($this->rootView->file_exists($v2)); From d5fcd9b5e0d6ac47c00e46b781739f020535756a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 20 Jan 2025 10:15:32 +0100 Subject: [PATCH 15/18] chore: WIP - debug for CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_versions/tests/VersioningTest.php | 15 ++++++++++++++- .../Files/ObjectStore/ObjectStoreStorage.php | 4 ++++ lib/private/Files/Storage/Wrapper/Encryption.php | 11 +++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index 707c4a8c6b16b..ec1233b07c27a 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -427,7 +427,20 @@ public function testMoveFileIntoSharedFolderAsRecipient(): void { // move file into the shared folder as recipient $success = Filesystem::rename('/test.txt', '/folder1/test.txt'); - $this->assertTrue($success); + // TODO: Voir is $v2 existe pour voir si c’est la copie ou la suppression qui foire + // Mettre du debug dans Wrapper/Encrytion::copyBetweenStorage c’est le suspect n-1 + $versionsFolder1 = '/' . self::TEST_VERSIONS_USER . '/files_versions'; + + $v1Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t1; + $v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2; + var_dump([ + 'success' => $success, + 'v1 exists' => $this->rootView->file_exists($v1), + 'v2 exists' => $this->rootView->file_exists($v2), + 'v1Renamed exists' => $this->rootView->file_exists($v1Renamed), + 'v2Renamed exists' => $this->rootView->file_exists($v2Renamed), + ]); + $this->assertTrue($success); // renvoi true :-O $this->assertFalse($this->rootView->file_exists($v1)); $this->assertFalse($this->rootView->file_exists($v2)); diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index e05de7cd91d04..8069215370fa7 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -176,13 +176,17 @@ private function rmObjects(ICacheEntry $entry): bool { } public function unlink(string $path): bool { + echo "[DEBUG] unlink($path) " . __FILE__ . ':' . __LINE__ . "\n"; $path = $this->normalizePath($path); + echo "[DEBUG] unlink($path) " . __FILE__ . ':' . __LINE__ . "\n"; $entry = $this->getCache()->get($path); if ($entry instanceof ICacheEntry) { if ($entry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) { + echo "[DEBUG] unlink directory $path " . __FILE__ . ':' . __LINE__ . "\n"; return $this->rmObjects($entry); } else { + echo "[DEBUG] unlink file $path " . __FILE__ . ':' . __LINE__ . "\n"; return $this->rmObject($entry); } } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index a416e61bd2fbb..980f590d169f9 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -169,8 +169,11 @@ public function file_put_contents(string $path, mixed $data): int|float|false { } public function unlink(string $path): bool { + echo "[DEBUG] unlink($path) " . __FILE__ . ':' . __LINE__ . "\n"; $fullPath = $this->getFullPath($path); + echo "[DEBUG] unlink($fullPath) " . __FILE__ . ':' . __LINE__ . "\n"; if ($this->util->isExcluded($fullPath)) { + echo "[DEBUG] excluded $fullPath " . __FILE__ . ':' . __LINE__ . "\n"; return $this->storage->unlink($path); } @@ -522,11 +525,14 @@ public function moveFromStorage( // - remove $this->copyBetweenStorage if (!$sourceStorage->isDeletable($sourceInternalPath)) { + echo "[DEBUG] $sourceInternalPath $targetInternalPath not deletable " . __FILE__ . ':' . __LINE__ . "\n"; return false; } $result = $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, true); + echo "[DEBUG] $sourceInternalPath $targetInternalPath copy:$result " . __FILE__ . ':' . __LINE__ . "\n"; if ($result) { + echo "[DEBUG] $sourceInternalPath $targetInternalPath copy success " . __FILE__ . ':' . __LINE__ . "\n"; if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) { /** @var ObjectStoreStorage $sourceStorage */ $sourceStorage->setPreserveCacheOnDelete(true); @@ -534,8 +540,10 @@ public function moveFromStorage( try { if ($sourceStorage->is_dir($sourceInternalPath)) { $result = $sourceStorage->rmdir($sourceInternalPath); + echo "[DEBUG] $sourceInternalPath rmdir:$result " . __FILE__ . ':' . __LINE__ . "\n"; } else { $result = $sourceStorage->unlink($sourceInternalPath); + echo "[DEBUG] $sourceInternalPath unlink:$result " . __FILE__ . ':' . __LINE__ . "\n"; } } finally { if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) { @@ -621,10 +629,12 @@ private function copyBetweenStorage( bool $preserveMtime, bool $isRename, ): bool { + echo "[DEBUG] $sourceInternalPath $targetInternalPath $isRename " . __FILE__ . ':' . __LINE__ . "\n"; // for versions we have nothing to do, because versions should always use the // key from the original file. Just create a 1:1 copy and done if ($this->isVersion($targetInternalPath) || $this->isVersion($sourceInternalPath)) { + echo "[DEBUG] $sourceInternalPath $targetInternalPath $isRename " . __FILE__ . ':' . __LINE__ . "\n"; // remember that we try to create a version so that we can detect it during // fopen($sourceInternalPath) and by-pass the encryption in order to // create a 1:1 copy of the file @@ -642,6 +652,7 @@ private function copyBetweenStorage( } $this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename, true); } + echo "[DEBUG] $result " . __FILE__ . ':' . __LINE__ . "\n"; return $result; } From e95bf85a0a52d08eb35642e8413554f13a76d9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 11 Feb 2025 16:06:33 +0100 Subject: [PATCH 16/18] fix(encryption): Improve Update class and event listenening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to avoid back&forth between path and Node object Signed-off-by: Côme Chilliet --- .../Encryption/EncryptionEventListener.php | 13 +- lib/private/Encryption/EncryptionWrapper.php | 9 -- lib/private/Encryption/Update.php | 115 +++++--------- .../Files/Storage/Wrapper/Encryption.php | 2 - tests/lib/Encryption/UpdateTest.php | 140 +++++++++--------- .../Files/Storage/Wrapper/EncryptionTest.php | 15 -- 6 files changed, 112 insertions(+), 182 deletions(-) diff --git a/lib/private/Encryption/EncryptionEventListener.php b/lib/private/Encryption/EncryptionEventListener.php index a22661e9f7503..1801a34be5664 100644 --- a/lib/private/Encryption/EncryptionEventListener.php +++ b/lib/private/Encryption/EncryptionEventListener.php @@ -18,7 +18,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; use OCP\Files\Events\Node\NodeRenamedEvent; -use OCP\Files\Folder; use OCP\IUser; use OCP\IUserSession; use OCP\Share\Events\ShareCreatedEvent; @@ -32,6 +31,7 @@ class EncryptionEventListener implements IEventListener { public function __construct( private IUserSession $userSession, private SetupManager $setupManager, + private Manager $encryptionManager, ) { } @@ -43,17 +43,20 @@ public static function register(IEventDispatcher $dispatcher): void { } public function handle(Event $event): void { + if (!$this->encryptionManager->isEnabled()) { + return; + } if ($event instanceof NodeRenamedEvent) { - $this->getUpdate()->postRename($event->getSource() instanceof Folder, $event->getSource()->getPath(), $event->getTarget()->getPath()); + $this->getUpdate()->postRename($event->getSource(), $event->getTarget()); } elseif ($event instanceof ShareCreatedEvent) { - $this->getUpdate()->postShared($event->getShare()->getNodeType(), $event->getShare()->getNodeId()); + $this->getUpdate()->postShared($event->getShare()->getNode()); } elseif ($event instanceof ShareDeletedEvent) { // In case the unsharing happens in a background job, we don't have // a session and we load instead the user from the UserManager $owner = $event->getShare()->getNode()->getOwner(); - $this->getUpdate($owner)->postUnshared($event->getShare()->getNodeType(), $event->getShare()->getNodeId()); + $this->getUpdate($owner)->postUnshared($event->getShare()->getNode()); } elseif ($event instanceof NodeRestoredEvent) { - $this->getUpdate()->postRestore($event->getTarget() instanceof Folder, $event->getTarget()->getPath()); + $this->getUpdate()->postRestore($event->getTarget()); } } diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index 6a19c1cccaa96..b9db9616538a6 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -75,14 +75,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $ \OC::$server->getGroupManager(), \OC::$server->getConfig() ); - $update = new Update( - $util, - Filesystem::getMountManager(), - $this->manager, - $fileHelper, - $this->logger, - $uid - ); return new Encryption( $parameters, $this->manager, @@ -91,7 +83,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $ $fileHelper, $uid, $keyStorage, - $update, $mountManager, $this->arrayCache ); diff --git a/lib/private/Encryption/Update.php b/lib/private/Encryption/Update.php index d3f9c0ebef7cd..293a1ce653ca4 100644 --- a/lib/private/Encryption/Update.php +++ b/lib/private/Encryption/Update.php @@ -1,131 +1,85 @@ util = $util; - $this->mountManager = $mountManager; - $this->encryptionManager = $encryptionManager; - $this->file = $file; - $this->logger = $logger; - $this->uid = $uid; } /** * hook after file was shared */ - public function postShared(string $nodeType, int $nodeId): void { - if ($this->encryptionManager->isEnabled()) { - if ($nodeType === 'file' || $nodeType === 'folder') { - $path = Filesystem::getPath($nodeId); - [$owner, $ownerPath] = $this->getOwnerPath($path); - $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($nodeType === 'folder', $absPath); - } - } + public function postShared(OCPFile|Folder $node): void { + $this->update($node); } /** * hook after file was unshared */ - public function postUnshared(string $nodeType, int $nodeId): void { - if ($this->encryptionManager->isEnabled()) { - if ($nodeType === 'file' || $nodeType === 'folder') { - $path = Filesystem::getPath($nodeId); - [$owner, $ownerPath] = $this->getOwnerPath($path); - $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($nodeType === 'folder', $absPath); - } - } + public function postUnshared(OCPFile|Folder $node): void { + $this->update($node); } /** * inform encryption module that a file was restored from the trash bin, * e.g. to update the encryption keys */ - public function postRestore(bool $directory, string $filePath): void { - if ($this->encryptionManager->isEnabled()) { - $path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $filePath); - $this->update($directory, $path); - } + public function postRestore(OCPFile|Folder $node): void { + $this->update($node); } /** * inform encryption module that a file was renamed, * e.g. to update the encryption keys */ - public function postRename(bool $directory, string $source, string $target): void { - if ( - $this->encryptionManager->isEnabled() && - dirname($source) !== dirname($target) - ) { - [$owner, $ownerPath] = $this->getOwnerPath($target); - $absPath = '/' . $owner . '/files/' . $ownerPath; - $this->update($directory, $absPath); + public function postRename(OCPFile|Folder $source, OCPFile|Folder $target): void { + if (dirname($source->getPath()) !== dirname($target->getPath())) { + $this->update($target); } } /** - * get owner and path relative to data//files + * get owner and path relative to data/ * - * @param string $path path to file for current user - * @return array ['owner' => $owner, 'path' => $path] * @throws \InvalidArgumentException */ - protected function getOwnerPath($path) { - $info = Filesystem::getFileInfo($path); - $owner = Filesystem::getOwner($path); + protected function getOwnerPath(OCPFile|Folder $node): string { + $owner = $node->getOwner()?->getUID(); + if ($owner === null) { + throw new InvalidArgumentException('No owner found for ' . $node->getId()); + } $view = new View('/' . $owner . '/files'); - $path = $view->getPath($info->getId()); - if ($path === null) { - throw new InvalidArgumentException('No file found for ' . $info->getId()); + try { + $path = $view->getPath($node->getId()); + } catch (NotFoundException $e) { + throw new InvalidArgumentException('No file found for ' . $node->getId(), previous:$e); } - - return [$owner, $path]; + return '/' . $owner . '/files/' . $path; } /** @@ -134,7 +88,7 @@ protected function getOwnerPath($path) { * @param string $path relative to data/ * @throws Exceptions\ModuleDoesNotExistsException */ - public function update(bool $directory, string $path): void { + public function update(OCPFile|Folder $node): void { $encryptionModule = $this->encryptionManager->getEncryptionModule(); // if the encryption module doesn't encrypt the files on a per-user basis @@ -143,15 +97,14 @@ public function update(bool $directory, string $path): void { return; } + $path = $this->getOwnerPath($node); // if a folder was shared, get a list of all (sub-)folders - if ($directory) { + if ($node instanceof Folder) { $allFiles = $this->util->getAllFiles($path); } else { $allFiles = [$path]; } - - foreach ($allFiles as $file) { $usersSharing = $this->file->getAccessList($file); try { diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 980f590d169f9..5a84ae3b27635 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -8,7 +8,6 @@ namespace OC\Files\Storage\Wrapper; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; -use OC\Encryption\Update; use OC\Encryption\Util; use OC\Files\Cache\CacheEntry; use OC\Files\Filesystem; @@ -49,7 +48,6 @@ public function __construct( private IFile $fileHelper, private ?string $uid, private IStorage $keyStorage, - private Update $update, private Manager $mountManager, private ArrayCache $arrayCache, ) { diff --git a/tests/lib/Encryption/UpdateTest.php b/tests/lib/Encryption/UpdateTest.php index d54aeab35ddf3..938578ff2d3bf 100644 --- a/tests/lib/Encryption/UpdateTest.php +++ b/tests/lib/Encryption/UpdateTest.php @@ -1,4 +1,7 @@ view = $this->createMock(View::class); $this->util = $this->createMock(Util::class); - $this->mountManager = $this->createMock(Manager::class); $this->encryptionManager = $this->createMock(\OC\Encryption\Manager::class); $this->fileHelper = $this->createMock(File::class); $this->encryptionModule = $this->createMock(IEncryptionModule::class); $this->logger = $this->createMock(LoggerInterface::class); $this->uid = 'testUser1'; + } - $this->update = new Update( - $this->util, - $this->mountManager, - $this->encryptionManager, - $this->fileHelper, - $this->logger, - $this->uid); + private function getUserMock(string $uid): IUser&MockObject { + $user = $this->createMock(IUser::class); + $user->expects(self::any()) + ->method('getUID') + ->willReturn($uid); + return $user; + } + + private function getFileMock(string $path, string $owner): OCPFile&MockObject { + $node = $this->createMock(OCPFile::class); + $node->expects(self::atLeastOnce()) + ->method('getPath') + ->willReturn($path); + $node->expects(self::any()) + ->method('getOwner') + ->willReturn($this->getUserMock($owner)); + + return $node; + } + + private function getFolderMock(string $path, string $owner): Folder&MockObject { + $node = $this->createMock(Folder::class); + $node->expects(self::atLeastOnce()) + ->method('getPath') + ->willReturn($path); + $node->expects(self::any()) + ->method('getOwner') + ->willReturn($this->getUserMock($owner)); + + return $node; } /** @@ -60,6 +85,10 @@ protected function setUp(): void { * @param integer $numberOfFiles */ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { + $updateMock = $this->getUpdateMock(['getOwnerPath']); + $updateMock->expects($this->once())->method('getOwnerPath') + ->willReturnCallback(fn (OCPFile|Folder $node) => '/user/'.$node->getPath()); + $this->encryptionManager->expects($this->once()) ->method('getEncryptionModule') ->willReturn($this->encryptionModule); @@ -68,6 +97,9 @@ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { $this->util->expects($this->once()) ->method('getAllFiles') ->willReturn($allFiles); + $node = $this->getFolderMock($path, 'user'); + } else { + $node = $this->getFileMock($path, 'user'); } $this->fileHelper->expects($this->exactly($numberOfFiles)) @@ -78,7 +110,7 @@ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { ->method('update') ->willReturn(true); - $this->update->update($isDir, $path); + $updateMock->update($node); } /** @@ -98,32 +130,26 @@ public function dataTestUpdate() { * * @param string $source * @param string $target - * @param boolean $encryptionEnabled */ - public function testPostRename($source, $target, $encryptionEnabled): void { - $updateMock = $this->getUpdateMock(['update', 'getOwnerPath']); + public function testPostRename($source, $target): void { + $updateMock = $this->getUpdateMock(['update','getOwnerPath']); - $this->encryptionManager->expects($this->once()) - ->method('isEnabled') - ->willReturn($encryptionEnabled); + $sourceNode = $this->getFileMock($source, 'user'); + $targetNode = $this->getFileMock($target, 'user'); - if (dirname($source) === dirname($target) || $encryptionEnabled === false) { + if (dirname($source) === dirname($target)) { $updateMock->expects($this->never())->method('getOwnerPath'); $updateMock->expects($this->never())->method('update'); } else { - $updateMock->expects($this->once()) - ->method('getOwnerPath') - ->willReturnCallback(function ($path) use ($target) { - $this->assertSame( - $target, - $path, - 'update needs to be executed for the target destination'); - return ['owner', $path]; - }); - $updateMock->expects($this->once())->method('update'); + $updateMock->expects($this->once())->method('update') + ->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame( + $target, + $node->getPath(), + 'update needs to be executed for the target destination' + )); } - $updateMock->postRename(false, $source, $target); + $updateMock->postRename($sourceNode, $targetNode); } /** @@ -133,61 +159,35 @@ public function testPostRename($source, $target, $encryptionEnabled): void { */ public function dataTestPostRename() { return [ - ['/test.txt', '/testNew.txt', true], - ['/test.txt', '/testNew.txt', false], - ['/folder/test.txt', '/testNew.txt', true], - ['/folder/test.txt', '/testNew.txt', false], - ['/folder/test.txt', '/testNew.txt', true], - ['/test.txt', '/folder/testNew.txt', false], + ['/test.txt', '/testNew.txt'], + ['/folder/test.txt', '/testNew.txt'], + ['/test.txt', '/folder/testNew.txt'], ]; } - - /** - * @dataProvider dataTestPostRestore - * - * @param boolean $encryptionEnabled - */ - public function testPostRestore($encryptionEnabled): void { + public function testPostRestore(): void { $updateMock = $this->getUpdateMock(['update']); - $this->encryptionManager->expects($this->once()) - ->method('isEnabled') - ->willReturn($encryptionEnabled); - - if ($encryptionEnabled) { - $updateMock->expects($this->once())->method('update'); - } else { - $updateMock->expects($this->never())->method('update'); - } + $updateMock->expects($this->once())->method('update') + ->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame( + '/folder/test.txt', + $node->getPath(), + 'update needs to be executed for the target destination' + )); - $updateMock->postRestore(false, '/folder/test.txt'); - } - - /** - * test data for testPostRestore() - * - * @return array - */ - public function dataTestPostRestore() { - return [ - [true], - [false], - ]; + $updateMock->postRestore($this->getFileMock('/folder/test.txt', 'user')); } /** * create mock of the update method * * @param array $methods methods which should be set - * @return \OC\Encryption\Update | MockObject */ - protected function getUpdateMock($methods) { - return $this->getMockBuilder('\OC\Encryption\Update') + protected function getUpdateMock(array $methods): Update&MockObject { + return $this->getMockBuilder(Update::class) ->setConstructorArgs( [ $this->util, - $this->mountManager, $this->encryptionManager, $this->fileHelper, $this->logger, diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index bb3df36dec2bf..e8ad557dae550 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -11,7 +11,6 @@ use OC; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\Encryption\File; -use OC\Encryption\Update; use OC\Encryption\Util; use OC\Files\Cache\Cache; use OC\Files\Cache\CacheEntry; @@ -46,7 +45,6 @@ class EncryptionTest extends Storage { private Util&MockObject $util; private \OC\Encryption\Manager&MockObject $encryptionManager; private IEncryptionModule&MockObject $encryptionModule; - private Update&MockObject $update; private Cache&MockObject $cache; private LoggerInterface&MockObject $logger; private File&MockObject $file; @@ -111,9 +109,6 @@ protected function setUp(): void { $this->keyStore = $this->getMockBuilder('\OC\Encryption\Keys\Storage') ->disableOriginalConstructor()->getMock(); - $this->update = $this->getMockBuilder('\OC\Encryption\Update') - ->disableOriginalConstructor()->getMock(); - $this->mount = $this->getMockBuilder('\OC\Files\Mount\MountPoint') ->disableOriginalConstructor() ->setMethods(['getOption']) @@ -155,7 +150,6 @@ protected function setUp(): void { $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache ] @@ -237,7 +231,6 @@ function ($path) use ($encrypted) { $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ] @@ -316,7 +309,6 @@ public function testFilesize(): void { $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ] @@ -361,7 +353,6 @@ public function testVerifyUnencryptedSize($encryptedSize, $unencryptedSize, $fai $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ] @@ -491,7 +482,6 @@ public function testRmdir($path, $rmdirResult, $isExcluded, $encryptionEnabled): $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ); @@ -598,7 +588,6 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath): void { $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ] @@ -692,7 +681,6 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $strippedPat $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache, ] @@ -867,7 +855,6 @@ public function testCopyBetweenStorageVersions($sourceInternalPath, $targetInter $this->file, null, $this->keyStore, - $this->update, $this->mountManager, $this->arrayCache ] @@ -968,7 +955,6 @@ public function testShouldEncrypt( $util = $this->createMock(Util::class); $fileHelper = $this->createMock(IFile::class); $keyStorage = $this->createMock(IStorage::class); - $update = $this->createMock(Update::class); $mountManager = $this->createMock(\OC\Files\Mount\Manager::class); $mount = $this->createMock(IMountPoint::class); $arrayCache = $this->createMock(ArrayCache::class); @@ -986,7 +972,6 @@ public function testShouldEncrypt( $fileHelper, null, $keyStorage, - $update, $mountManager, $arrayCache ] From c60c67946dc983c6e7896422b32356cfe3557a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 11 Feb 2025 17:21:50 +0100 Subject: [PATCH 17/18] fixup! fix(encryption): Improve Update class and event listenening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/Encryption/UpdateTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/lib/Encryption/UpdateTest.php b/tests/lib/Encryption/UpdateTest.php index 938578ff2d3bf..9940549981f32 100644 --- a/tests/lib/Encryption/UpdateTest.php +++ b/tests/lib/Encryption/UpdateTest.php @@ -87,7 +87,7 @@ private function getFolderMock(string $path, string $owner): Folder&MockObject { public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void { $updateMock = $this->getUpdateMock(['getOwnerPath']); $updateMock->expects($this->once())->method('getOwnerPath') - ->willReturnCallback(fn (OCPFile|Folder $node) => '/user/'.$node->getPath()); + ->willReturnCallback(fn (OCPFile|Folder $node) => '/user/' . $node->getPath()); $this->encryptionManager->expects($this->once()) ->method('getEncryptionModule') @@ -169,11 +169,11 @@ public function testPostRestore(): void { $updateMock = $this->getUpdateMock(['update']); $updateMock->expects($this->once())->method('update') - ->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame( - '/folder/test.txt', - $node->getPath(), - 'update needs to be executed for the target destination' - )); + ->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame( + '/folder/test.txt', + $node->getPath(), + 'update needs to be executed for the target destination' + )); $updateMock->postRestore($this->getFileMock('/folder/test.txt', 'user')); } From 840489b6284feca977305722b589bc7a3ab490cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Feb 2025 17:30:19 +0100 Subject: [PATCH 18/18] WIP debug 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_versions/tests/VersioningTest.php | 2 ++ lib/private/Files/View.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index ec1233b07c27a..abad0293f1a8f 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -435,6 +435,8 @@ public function testMoveFileIntoSharedFolderAsRecipient(): void { $v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2; var_dump([ 'success' => $success, + 'source exists' => Filesystem::file_exists('/test.txt'), + 'target exists' => Filesystem::file_exists('/folder1/test.txt'), 'v1 exists' => $this->rootView->file_exists($v1), 'v2 exists' => $this->rootView->file_exists($v2), 'v1Renamed exists' => $this->rootView->file_exists($v1Renamed), diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 5e09ce69fb22c..5a5c0ea14dfa9 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -273,7 +273,9 @@ protected function removeUpdate(Storage $storage, string $internalPath): void { } protected function renameUpdate(Storage $sourceStorage, Storage $targetStorage, string $sourceInternalPath, string $targetInternalPath): void { + echo "[DEBUG] renameUpdate($sourceInternalPath, $targetInternalPath) " . __FILE__ . ':' . __LINE__ . "\n"; if ($this->updaterEnabled) { + echo "[DEBUG] renameUpdate($sourceInternalPath, $targetInternalPath)! " . __FILE__ . ':' . __LINE__ . "\n"; $targetStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } }