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/build/psalm-baseline.xml b/build/psalm-baseline.xml index 07d897963aa6c..2a0d2937d74b5 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2960,6 +2960,16 @@ + + + + + + + + + + diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index f3133fa33e829..614d4ecc40288 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -9,7 +9,6 @@ namespace OC\Core\Command\Encryption; use OCP\App\IAppManager; -use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; @@ -25,7 +24,6 @@ class DecryptAll extends Command { public function __construct( protected IAppManager $appManager, protected IConfig $config, - protected IAppConfig $appConfig, protected \OC\Encryption\DecryptAll $decryptAll, protected QuestionHelper $questionHelper, ) { @@ -89,11 +87,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); + $originallyEnabled = $this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes'; try { if ($originallyEnabled) { $output->write('Disable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', false); + $this->config->setAppValue('core', 'encryption_enabled', 'no'); $output->writeln('done.'); } else { $output->writeln('Server side encryption not enabled. Nothing to do.'); @@ -121,18 +119,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(' aborted.'); if ($originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } } elseif (($uid !== '') && $originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } $this->resetMaintenanceAndTrashbin(); return 0; } if ($originallyEnabled) { $output->write('Enable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->setAppValue('core', 'encryption_enabled', 'yes'); $output->writeln('done.'); } $output->writeln('aborted'); @@ -140,7 +138,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } catch (\Exception $e) { // enable server side encryption again if something went wrong if ($originallyEnabled) { - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->setAppValue('core', 'encryption_enabled', 'yes'); } $this->resetMaintenanceAndTrashbin(); throw $e; diff --git a/lib/private/Encryption/File.php b/lib/private/Encryption/File.php index 26e643d10066a..f197501b9d728 100644 --- a/lib/private/Encryption/File.php +++ b/lib/private/Encryption/File.php @@ -10,7 +10,9 @@ use OCA\Files_External\Service\GlobalStoragesService; use OCP\App\IAppManager; use OCP\Cache\CappedMemoryCache; +use OCP\Files\Folder; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Share\IManager; @@ -73,11 +75,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']); @@ -109,4 +114,23 @@ 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. + * + * @return ?Node null if not even the user folder itself could be resolved + */ + private function getClosestExistingNode(Folder $userFolder, string $path): ?Node { + while (true) { + try { + return $userFolder->get($path); + } catch (NotFoundException) { + $parent = dirname($path); + if ($parent === $path) { + return null; + } + $path = $parent; + } + } + } } diff --git a/tests/Core/Command/Encryption/DecryptAllTest.php b/tests/Core/Command/Encryption/DecryptAllTest.php index 6d0bc045cdd38..956fb46e538a1 100644 --- a/tests/Core/Command/Encryption/DecryptAllTest.php +++ b/tests/Core/Command/Encryption/DecryptAllTest.php @@ -10,7 +10,6 @@ use OC\Core\Command\Encryption\DecryptAll; use OCP\App\IAppManager; -use OCP\IAppConfig; use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Helper\QuestionHelper; @@ -20,7 +19,6 @@ class DecryptAllTest extends TestCase { private MockObject&IConfig $config; - private MockObject&IAppConfig $appConfig; private MockObject&IAppManager $appManager; private MockObject&InputInterface $consoleInput; private MockObject&OutputInterface $consoleOutput; @@ -31,7 +29,6 @@ protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); - $this->appConfig = $this->createMock(IAppConfig::class); $this->appManager = $this->createMock(IAppManager::class); $this->questionHelper = $this->createMock(QuestionHelper::class); $this->decryptAll = $this->createMock(\OC\Encryption\DecryptAll::class); @@ -74,7 +71,6 @@ public function testMaintenanceAndTrashbin(): void { $instance = new DecryptAll( $this->appManager, $this->config, - $this->appConfig, $this->decryptAll, $this->questionHelper ); @@ -95,15 +91,14 @@ public function testExecute($encryptionEnabled, $continue): void { $instance = new DecryptAll( $this->appManager, $this->config, - $this->appConfig, $this->decryptAll, $this->questionHelper ); - $this->appConfig->expects($this->once()) - ->method('getValueBool') - ->with('core', 'encryption_enabled') - ->willReturn($encryptionEnabled); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'encryption_enabled', 'no') + ->willReturn($encryptionEnabled ? 'yes' : 'no'); $this->consoleInput->expects($this->any()) ->method('getArgument') @@ -112,19 +107,18 @@ public function testExecute($encryptionEnabled, $continue): void { if ($encryptionEnabled) { $calls = [ - ['core', 'encryption_enabled', false, false], - ['core', 'encryption_enabled', true, false], + ['core', 'encryption_enabled', 'no'], + ['core', 'encryption_enabled', 'yes'], ]; - $this->appConfig->expects($this->exactly(count($calls))) - ->method('setValueBool') - ->willReturnCallback(function () use (&$calls): bool { + $this->config->expects($this->exactly(count($calls))) + ->method('setAppValue') + ->willReturnCallback(function () use (&$calls): void { $expected = array_shift($calls); $this->assertEquals($expected, func_get_args()); - return true; }); } else { - $this->appConfig->expects($this->never()) - ->method('setValueBool'); + $this->config->expects($this->never()) + ->method('setAppValue'); } $this->questionHelper->expects($this->once()) ->method('ask') @@ -156,27 +150,25 @@ public function testExecuteFailure(): void { $instance = new DecryptAll( $this->appManager, $this->config, - $this->appConfig, $this->decryptAll, $this->questionHelper ); // make sure that we enable encryption again after a exception was thrown $calls = [ - ['core', 'encryption_enabled', false, false], - ['core', 'encryption_enabled', true, false], + ['core', 'encryption_enabled', 'no'], + ['core', 'encryption_enabled', 'yes'], ]; - $this->appConfig->expects($this->exactly(2)) - ->method('setValuebool') - ->willReturnCallback(function () use (&$calls): bool { + $this->config->expects($this->exactly(2)) + ->method('setAppValue') + ->willReturnCallback(function () use (&$calls): void { $expected = array_shift($calls); $this->assertEquals($expected, func_get_args()); - return true; }); - $this->appConfig->expects($this->once()) - ->method('getValueBool') - ->with('core', 'encryption_enabled') - ->willReturn(true); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'encryption_enabled', 'no') + ->willReturn('yes'); $this->consoleInput->expects($this->any()) ->method('getArgument') diff --git a/tests/lib/Encryption/FileTest.php b/tests/lib/Encryption/FileTest.php new file mode 100644 index 0000000000000..de3607743121e --- /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(Folder::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')); + } +}