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 ]