From a96733b08617d76cefc1bdc0001fd7ea5d94d414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 4 Aug 2025 10:42:04 +0200 Subject: [PATCH 1/2] fix(encryption): Ignore shared files in encrypt-all command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copying and renaming a share will not encrypt it anyway. It will get encrypted when the owner’s files get encrypted. Signed-off-by: Côme Chilliet --- apps/encryption/lib/Crypto/EncryptAll.php | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php index d9db616e6f15b..424513757d8f1 100644 --- a/apps/encryption/lib/Crypto/EncryptAll.php +++ b/apps/encryption/lib/Crypto/EncryptAll.php @@ -12,6 +12,7 @@ use OCA\Encryption\KeyManager; use OCA\Encryption\Users\Setup; use OCA\Encryption\Util; +use OCP\Files\FileInfo; use OCP\IConfig; use OCP\IL10N; use OCP\IUser; @@ -203,14 +204,18 @@ protected function encryptUsersFiles($uid, ProgressBar $progress, $userCount) { $content = $this->rootView->getDirectoryContent($root); foreach ($content as $file) { $path = $root . '/' . $file['name']; - if ($this->rootView->is_dir($path)) { + if ($file->isShared()) { + $progress->setMessage("Skip shared file/folder $path"); + $progress->advance(); + continue; + } elseif ($file->getType() === FileInfo::TYPE_FOLDER) { $directories[] = $path; continue; } else { $progress->setMessage("encrypt files for user $userCount: $path"); $progress->advance(); try { - if ($this->encryptFile($path) === false) { + if ($this->encryptFile($file, $path) === false) { $progress->setMessage("encrypt files for user $userCount: $path (already encrypted)"); $progress->advance(); } @@ -231,17 +236,9 @@ protected function encryptUsersFiles($uid, ProgressBar $progress, $userCount) { } } - /** - * encrypt file - * - * @param string $path - * @return bool - */ - protected function encryptFile($path) { - + protected function encryptFile(FileInfo $fileInfo, string $path): bool { // skip already encrypted files - $fileInfo = $this->rootView->getFileInfo($path); - if ($fileInfo !== false && $fileInfo->isEncrypted()) { + if ($fileInfo->isEncrypted()) { return true; } From efd541c64920aa52e26a858f07c70fa5bb9e24e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 4 Aug 2025 11:53:19 +0200 Subject: [PATCH 2/2] chore(encryption): Adapt tests to code changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also pulled tests refactors from master to avoid conflicts Signed-off-by: Côme Chilliet --- apps/encryption/lib/Crypto/EncryptAll.php | 2 +- .../tests/Crypto/EncryptAllTest.php | 102 ++++++++++-------- .../Command/Encryption/EncryptAllTest.php | 56 +++------- 3 files changed, 77 insertions(+), 83 deletions(-) diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php index 424513757d8f1..4ed75b85a939b 100644 --- a/apps/encryption/lib/Crypto/EncryptAll.php +++ b/apps/encryption/lib/Crypto/EncryptAll.php @@ -203,7 +203,7 @@ protected function encryptUsersFiles($uid, ProgressBar $progress, $userCount) { while ($root = array_pop($directories)) { $content = $this->rootView->getDirectoryContent($root); foreach ($content as $file) { - $path = $root . '/' . $file['name']; + $path = $root . '/' . $file->getName(); if ($file->isShared()) { $progress->setMessage("Skip shared file/folder $path"); $progress->advance(); diff --git a/apps/encryption/tests/Crypto/EncryptAllTest.php b/apps/encryption/tests/Crypto/EncryptAllTest.php index 779248a0a748f..90b4a6deca61c 100644 --- a/apps/encryption/tests/Crypto/EncryptAllTest.php +++ b/apps/encryption/tests/Crypto/EncryptAllTest.php @@ -1,5 +1,7 @@ createMock(OutputFormatterInterface::class); $outputFormatter->method('isDecorated')->willReturn(false); @@ -112,6 +114,13 @@ protected function setUp(): void { ); } + protected function createFileInfoMock($type, string $name): FileInfo&MockObject { + $fileInfo = $this->createMock(FileInfo::class); + $fileInfo->method('getType')->willReturn($type); + $fileInfo->method('getName')->willReturn($name); + return $fileInfo; + } + public function testEncryptAll(): void { /** @var EncryptAll&MockObject $encryptAll */ $encryptAll = $this->getMockBuilder(EncryptAll::class) @@ -131,7 +140,7 @@ public function testEncryptAll(): void { $this->logger, ] ) - ->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) + ->onlyMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) ->getMock(); $this->util->expects($this->any())->method('isMasterKeyEnabled')->willReturn(false); @@ -161,7 +170,7 @@ public function testEncryptAllWithMasterKey(): void { $this->logger, ] ) - ->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) + ->onlyMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) ->getMock(); $this->util->expects($this->any())->method('isMasterKeyEnabled')->willReturn(true); @@ -192,7 +201,7 @@ public function testCreateKeyPairs(): void { $this->logger, ] ) - ->setMethods(['setupUserFS', 'generateOneTimePassword']) + ->onlyMethods(['setupUserFS', 'generateOneTimePassword']) ->getMock(); @@ -225,7 +234,7 @@ function ($user) { } public function testEncryptAllUsersFiles(): void { - /** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */ + /** @var EncryptAll&MockObject $encryptAll */ $encryptAll = $this->getMockBuilder(EncryptAll::class) ->setConstructorArgs( [ @@ -243,7 +252,7 @@ public function testEncryptAllUsersFiles(): void { $this->logger, ] ) - ->setMethods(['encryptUsersFiles']) + ->onlyMethods(['encryptUsersFiles']) ->getMock(); $this->util->expects($this->any())->method('isMasterKeyEnabled')->willReturn(false); @@ -252,17 +261,22 @@ public function testEncryptAllUsersFiles(): void { $this->invokePrivate($encryptAll, 'output', [$this->outputInterface]); $this->invokePrivate($encryptAll, 'userPasswords', [['user1' => 'pwd1', 'user2' => 'pwd2']]); - $encryptAll->expects($this->exactly(2))->method('encryptUsersFiles') - ->withConsecutive( - ['user1'], - ['user2'], - ); + $encryptAllCalls = []; + $encryptAll->expects($this->exactly(2)) + ->method('encryptUsersFiles') + ->willReturnCallback(function ($uid) use (&$encryptAllCalls): void { + $encryptAllCalls[] = $uid; + }); $this->invokePrivate($encryptAll, 'encryptAllUsersFiles'); + self::assertEquals([ + 'user1', + 'user2', + ], $encryptAllCalls); } public function testEncryptUsersFiles(): void { - /** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */ + /** @var EncryptAll&MockObject $encryptAll */ $encryptAll = $this->getMockBuilder(EncryptAll::class) ->setConstructorArgs( [ @@ -280,40 +294,39 @@ public function testEncryptUsersFiles(): void { $this->logger, ] ) - ->setMethods(['encryptFile', 'setupUserFS']) + ->onlyMethods(['encryptFile', 'setupUserFS']) ->getMock(); $this->util->expects($this->any())->method('isMasterKeyEnabled')->willReturn(false); $this->view->expects($this->exactly(2))->method('getDirectoryContent') - ->withConsecutive( - ['/user1/files'], - ['/user1/files/foo'], - )->willReturnOnConsecutiveCalls( + ->willReturnMap([ [ - ['name' => 'foo', 'type' => 'dir'], - ['name' => 'bar', 'type' => 'file'], + '/user1/files', + '', + null, + [ + $this->createFileInfoMock(FileInfo::TYPE_FOLDER, 'foo'), + $this->createFileInfoMock(FileInfo::TYPE_FILE, 'bar'), + ], ], [ - ['name' => 'subfile', 'type' => 'file'] - ] - ); - - $this->view->expects($this->any())->method('is_dir') - ->willReturnCallback( - function ($path) { - if ($path === '/user1/files/foo') { - return true; - } - return false; - } - ); + '/user1/files/foo', + '', + null, + [ + $this->createFileInfoMock(FileInfo::TYPE_FILE, 'subfile'), + ], + ], + ]); - $encryptAll->expects($this->exactly(2))->method('encryptFile') - ->withConsecutive( - ['/user1/files/bar'], - ['/user1/files/foo/subfile'], - ); + $encryptAllCalls = []; + $encryptAll->expects($this->exactly(2)) + ->method('encryptFile') + ->willReturnCallback(function (FileInfo $file, string $path) use (&$encryptAllCalls): bool { + $encryptAllCalls[] = $path; + return true; + }); $outputFormatter = $this->createMock(OutputFormatterInterface::class); $outputFormatter->method('isDecorated')->willReturn(false); @@ -323,6 +336,10 @@ function ($path) { $progressBar = new ProgressBar($this->outputInterface); $this->invokePrivate($encryptAll, 'encryptUsersFiles', ['user1', $progressBar, '']); + self::assertEquals([ + '/user1/files/bar', + '/user1/files/foo/subfile', + ], $encryptAllCalls); } public function testGenerateOneTimePassword(): void { @@ -343,8 +360,7 @@ public function testEncryptFile($isEncrypted): void { $fileInfo = $this->createMock(FileInfo::class); $fileInfo->expects($this->any())->method('isEncrypted') ->willReturn($isEncrypted); - $this->view->expects($this->any())->method('getFileInfo') - ->willReturn($fileInfo); + $this->view->expects($this->never())->method('getFileInfo'); if ($isEncrypted) { @@ -356,11 +372,11 @@ public function testEncryptFile($isEncrypted): void { } $this->assertTrue( - $this->invokePrivate($this->encryptAll, 'encryptFile', ['foo.txt']) + $this->invokePrivate($this->encryptAll, 'encryptFile', [$fileInfo, 'foo.txt']) ); } - public function dataTestEncryptFile() { + public static function dataTestEncryptFile(): array { return [ [true], [false], diff --git a/tests/Core/Command/Encryption/EncryptAllTest.php b/tests/Core/Command/Encryption/EncryptAllTest.php index 6e72e87b973fe..b45b6946cf675 100644 --- a/tests/Core/Command/Encryption/EncryptAllTest.php +++ b/tests/Core/Command/Encryption/EncryptAllTest.php @@ -1,4 +1,5 @@ config = $this->getMockBuilder(IConfig::class) - ->disableOriginalConstructor() - ->getMock(); - $this->encryptionManager = $this->getMockBuilder(IManager::class) - ->disableOriginalConstructor() - ->getMock(); - $this->appManager = $this->getMockBuilder(IAppManager::class) - ->disableOriginalConstructor() - ->getMock(); - $this->encryptionModule = $this->getMockBuilder(IEncryptionModule::class) - ->disableOriginalConstructor() - ->getMock(); - $this->questionHelper = $this->getMockBuilder(QuestionHelper::class) - ->disableOriginalConstructor() - ->getMock(); - $this->consoleInput = $this->getMockBuilder(InputInterface::class)->getMock(); + $this->config = $this->createMock(IConfig::class); + $this->encryptionManager = $this->createMock(IManager::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->encryptionModule = $this->createMock(IEncryptionModule::class); + $this->questionHelper = $this->createMock(QuestionHelper::class); + $this->consoleInput = $this->createMock(InputInterface::class); $this->consoleInput->expects($this->any()) ->method('isInteractive') ->willReturn(true); - $this->consoleOutput = $this->getMockBuilder(OutputInterface::class)->getMock(); + $this->consoleOutput = $this->createMock(OutputInterface::class); } public function testEncryptAll(): void {