diff --git a/build/integration/files_features/encryption.feature b/build/integration/files_features/encryption.feature index d961f15267151..65093dd9f4b72 100644 --- a/build/integration/files_features/encryption.feature +++ b/build/integration/files_features/encryption.feature @@ -40,3 +40,62 @@ Feature: encryption Then the command output does not contain the text "server-side encrypted: yes" And Downloading file "/non-encrypted.txt" with range "bytes=0-8" And Downloaded content should be "BLABLABLA" + + Scenario: copy a folder with per-user keys + # Setup encryption with per-user keys + Given using new dav path + And invoking occ with "app:enable encryption" + And the command was successful + And invoking occ with "encryption:disable-master-key" with input "y" + And the command was successful + And invoking occ with "encryption:enable" + And the command was successful + And user "user1" exists + And User "user1" created a folder "/source" + And User "user1" created a folder "/source/sub" + And User "user1" uploads file with content "BLABLABLA" to "/source/sub/encrypted.txt" + # The target folders only exist on the storage, not yet in the cache, while the files inside are written + When User "user1" copies file "/source" to "/copy" + Then the HTTP status code should be "201" + And As an "user1" + And Downloading file "/copy/sub/encrypted.txt" + And Downloaded content should be "BLABLABLA" + # Restore the initial encryption state + And invoking occ with "encryption:disable" + And the command was successful + And invoking occ with "encryption:enable-master-key" with input "y" + And the command was successful + + Scenario: copy a folder into a shared folder with per-user keys + # Setup encryption with per-user keys + Given using new dav path + And invoking occ with "app:enable encryption" + And the command was successful + And invoking occ with "encryption:disable-master-key" with input "y" + And the command was successful + And invoking occ with "encryption:enable" + And the command was successful + And user "user1" exists + And user "user2" exists + # Log in once so that the key pair of the share recipient exists + And User "user2" uploads file with content "BLABLABLA" to "/init.txt" + And User "user1" created a folder "/shared" + And User "user1" created a folder "/source" + And User "user1" uploads file with content "BLABLABLA" to "/source/encrypted.txt" + And as "user1" creating a share with + | path | /shared | + | shareType | 0 | + | shareWith | user2 | + | permissions | 31 | + And the HTTP status code should be "200" + # The share key of the recipient has to be created from the closest known parent + When User "user1" copies file "/source" to "/shared/copy" + Then the HTTP status code should be "201" + And As an "user2" + And Downloading file "/shared/copy/encrypted.txt" + And Downloaded content should be "BLABLABLA" + # Restore the initial encryption state + And invoking occ with "encryption:disable" + And the command was successful + And invoking occ with "encryption:enable-master-key" with input "y" + And the command was successful diff --git a/lib/private/Encryption/File.php b/lib/private/Encryption/File.php index 2c08468dffe1e..487bce719e469 100644 --- a/lib/private/Encryption/File.php +++ b/lib/private/Encryption/File.php @@ -13,6 +13,8 @@ use OCP\Cache\CappedMemoryCache; use OCP\Encryption\IFile; use OCP\Files\IRootFolder; +use OCP\Files\IUserFolder; +use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Server; use OCP\Share\IManager; @@ -72,11 +74,14 @@ public function getAccessList($path) { // first get the shares for the parent and cache the result so that we don't // need to check all parents for every file $parent = dirname($ownerPath); - $parentNode = $userFolder->get($parent); if (isset($this->cache[$parent])) { $resultForParents = $this->cache[$parent]; } else { - $resultForParents = $this->shareManager->getAccessList($parentNode); + $resultForParents = ['users' => [], 'public' => false, 'remote' => false]; + $parentNode = $this->getClosestExistingNode($userFolder, $parent); + if ($parentNode !== null) { + $resultForParents = $this->shareManager->getAccessList($parentNode) + $resultForParents; + } $this->cache[$parent] = $resultForParents; } $userIds = array_merge($userIds, $resultForParents['users']); @@ -107,4 +112,28 @@ public function getAccessList($path) { return ['users' => $uniqueUserIds, 'public' => $public]; } + + /** + * Get the node for $path, or for its closest ancestor that is known to the cache. + * + * Copying a folder creates the target directories on the storage before their + * cache entries exist, so while the files inside are written the parent path + * can not be resolved yet. As shares only exist on nodes that are in the cache, + * the access list of the closest known ancestor also applies to $path. + * + * @return ?Node null if not even the user folder itself could be resolved + */ + private function getClosestExistingNode(IUserFolder $userFolder, string $path): ?Node { + while (true) { + try { + return $userFolder->get($path); + } catch (NotFoundException) { + $parent = dirname($path); + if ($parent === $path || $parent === '.') { + return null; + } + $path = $parent; + } + } + } } diff --git a/tests/lib/Encryption/FileTest.php b/tests/lib/Encryption/FileTest.php new file mode 100644 index 0000000000000..97598ebc36dfe --- /dev/null +++ b/tests/lib/Encryption/FileTest.php @@ -0,0 +1,156 @@ +util = $this->createMock(Util::class); + $this->shareManager = $this->createMock(IManager::class); + $this->userFolder = $this->createMock(IUserFolder::class); + + $rootFolder = $this->createMock(IRootFolder::class); + $rootFolder->method('getUserFolder') + ->with('user1') + ->willReturn($this->userFolder); + + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isEnabledForUser') + ->willReturn(false); + + $this->file = $this->getMockBuilder(File::class) + ->setConstructorArgs([$this->util, $rootFolder, $this->shareManager]) + ->onlyMethods(['getAppManager']) + ->getMock(); + $this->file->method('getAppManager') + ->willReturn($appManager); + } + + /** + * Let the util mock resolve every path to $owner and $ownerPath + */ + private function mockUtil(string $owner, string $ownerPath): void { + $this->util->method('getUidAndFilename') + ->willReturn([$owner, $ownerPath]); + $this->util->method('isFile') + ->willReturn(true); + $this->util->method('stripPartialFileExtension') + ->willReturnArgument(0); + } + + public function testGetAccessList(): void { + $this->mockUtil('user1', '/files/folder/file.txt'); + + $node = $this->createMock(Node::class); + $parentNode = $this->createMock(Node::class); + $this->userFolder->method('get') + ->willReturnMap([ + ['/folder/file.txt', $node], + ['/folder', $parentNode], + ]); + + $this->shareManager->method('getAccessList') + ->willReturnCallback(fn (Node $requested) => match ($requested) { + $parentNode => ['users' => ['user2'], 'public' => false, 'remote' => false], + $node => ['users' => ['user3'], 'public' => true, 'remote' => false], + }); + + $this->assertSame( + ['users' => ['user1', 'user2', 'user3'], 'public' => true], + $this->file->getAccessList('/user1/files/folder/file.txt') + ); + } + + /** + * Copying a folder creates the target directories on the storage before their + * cache entries exist, so the parent of a file written into it can not be resolved. + */ + public function testGetAccessListWithUncachedParent(): void { + $this->mockUtil('user1', '/files/target/sub'); + + $rootNode = $this->createMock(Node::class); + $this->userFolder->method('get') + ->willReturnCallback(function (string $path) use ($rootNode) { + if ($path !== '/') { + throw new NotFoundException($path); + } + return $rootNode; + }); + + $this->shareManager->expects($this->once()) + ->method('getAccessList') + ->with($rootNode) + ->willReturn(['users' => ['user2'], 'public' => false, 'remote' => false]); + + $this->assertSame( + ['users' => ['user1', 'user2'], 'public' => false], + $this->file->getAccessList('/user1/files/target/sub') + ); + } + + public function testGetAccessListWithoutAnyResolvablePath(): void { + $this->mockUtil('user1', '/files/target/sub'); + + $this->userFolder->method('get') + ->willThrowException(new NotFoundException()); + + $this->shareManager->expects($this->never()) + ->method('getAccessList'); + + $this->assertSame( + ['users' => ['user1'], 'public' => false], + $this->file->getAccessList('/user1/files/target/sub') + ); + } + + public function testGetAccessListCachesTheParentResult(): void { + $this->util->method('getUidAndFilename') + ->willReturnCallback(fn (string $path) => ['user1', substr($path, strlen('/user1'))]); + $this->util->method('isFile') + ->willReturn(true); + $this->util->method('stripPartialFileExtension') + ->willReturnArgument(0); + + $parentNode = $this->createMock(Node::class); + $this->userFolder->method('get') + ->willReturnCallback(function (string $path) use ($parentNode) { + if ($path !== '/folder') { + throw new NotFoundException($path); + } + return $parentNode; + }); + + $this->shareManager->expects($this->once()) + ->method('getAccessList') + ->with($parentNode) + ->willReturn(['users' => ['user2'], 'public' => false, 'remote' => false]); + + $expected = ['users' => ['user1', 'user2'], 'public' => false]; + $this->assertSame($expected, $this->file->getAccessList('/user1/files/folder/first.txt')); + $this->assertSame($expected, $this->file->getAccessList('/user1/files/folder/second.txt')); + } +}