diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index 60decd4c8461e..53cab1df50830 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\DAV\Tests\unit\Connector\Sabre\RequestTest; use OCP\AppFramework\Http; diff --git a/apps/encryption/lib/Command/DropLegacyFileKey.php b/apps/encryption/lib/Command/DropLegacyFileKey.php index a9add1ad93b45..e487a0aa013d5 100644 --- a/apps/encryption/lib/Command/DropLegacyFileKey.php +++ b/apps/encryption/lib/Command/DropLegacyFileKey.php @@ -85,7 +85,7 @@ private function scanFolder(OutputInterface $output, string $folder): bool { $output->writeln('' . $path . ' does not have a proper header'); } else { try { - $legacyFileKey = $this->keyManager->getFileKey($path, null, true); + $legacyFileKey = $this->keyManager->getFileKey($path, true); if ($legacyFileKey === '') { $output->writeln('Got an empty legacy filekey for ' . $path . ', continuing', OutputInterface::VERBOSITY_VERBOSE); continue; diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php index 463ca4e22bb21..f80daec728a4d 100644 --- a/apps/encryption/lib/Crypto/Crypt.php +++ b/apps/encryption/lib/Crypto/Crypt.php @@ -342,9 +342,8 @@ public function encryptPrivateKey($privateKey, $password, $uid = '') { * @param string $privateKey * @param string $password * @param string $uid for regular users, empty for system keys - * @return false|string */ - public function decryptPrivateKey($privateKey, $password = '', $uid = '') { + public function decryptPrivateKey($privateKey, $password = '', $uid = '') : string|false { $header = $this->parseHeader($privateKey); if (isset($header['cipher'])) { diff --git a/apps/encryption/lib/Crypto/DecryptAll.php b/apps/encryption/lib/Crypto/DecryptAll.php index 362f43b86723c..7123d0f9a6403 100644 --- a/apps/encryption/lib/Crypto/DecryptAll.php +++ b/apps/encryption/lib/Crypto/DecryptAll.php @@ -1,10 +1,13 @@ util->isMasterKeyEnabled()) { @@ -52,7 +42,7 @@ public function prepare(InputInterface $input, OutputInterface $output, $user) { $password = $this->keyManager->getMasterKeyPassword(); } else { $recoveryKeyId = $this->keyManager->getRecoveryKeyId(); - if (!empty($user)) { + if ($user !== null && $user !== '') { $output->writeln('You can only decrypt the users files if you know'); $output->writeln('the users password or if they activated the recovery key.'); $output->writeln(''); @@ -96,12 +86,9 @@ public function prepare(InputInterface $input, OutputInterface $output, $user) { /** * get the private key which will be used to decrypt all files * - * @param string $user - * @param string $password - * @return bool|string * @throws PrivateKeyMissingException */ - protected function getPrivateKey($user, $password) { + protected function getPrivateKey(string $user, string $password): string|false { $recoveryKeyId = $this->keyManager->getRecoveryKeyId(); $masterKeyId = $this->keyManager->getMasterKeyId(); if ($user === $recoveryKeyId) { @@ -118,7 +105,7 @@ protected function getPrivateKey($user, $password) { return $privateKey; } - protected function updateSession($user, $privateKey) { + protected function updateSession(string $user, string $privateKey): void { $this->session->prepareDecryptAll($user, $privateKey); } } diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php index 4ed75b85a939b..db6135787ef64 100644 --- a/apps/encryption/lib/Crypto/EncryptAll.php +++ b/apps/encryption/lib/Crypto/EncryptAll.php @@ -1,10 +1,13 @@ input = $input; $this->output = $output; @@ -111,7 +111,7 @@ public function encryptAll(InputInterface $input, OutputInterface $output) { /** * create key-pair for every user */ - protected function createKeyPairs() { + protected function createKeyPairs(): void { $this->output->writeln("\n"); $progress = new ProgressBar($this->output); $progress->setFormat(" %message% \n [%bar%]"); @@ -146,7 +146,7 @@ protected function createKeyPairs() { /** * iterate over all user and encrypt their files */ - protected function encryptAllUsersFiles() { + protected function encryptAllUsersFiles(): void { $this->output->writeln("\n"); $progress = new ProgressBar($this->output); $progress->setFormat(" %message% \n [%bar%]"); @@ -168,10 +168,8 @@ protected function encryptAllUsersFiles() { /** * encrypt all user files with the master key - * - * @param ProgressBar $progress */ - protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress) { + protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress): void { $userNo = 1; foreach ($this->userManager->getBackends() as $backend) { $limit = 500; @@ -190,12 +188,8 @@ protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress) { /** * encrypt files from the given user - * - * @param string $uid - * @param ProgressBar $progress - * @param string $userCount */ - protected function encryptUsersFiles($uid, ProgressBar $progress, $userCount) { + protected function encryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void { $this->setupUserFS($uid); $directories = []; $directories[] = '/' . $uid . '/files'; @@ -268,7 +262,7 @@ protected function encryptFile(FileInfo $fileInfo, string $path): bool { /** * output one-time encryption passwords */ - protected function outputPasswords() { + protected function outputPasswords(): void { $table = new Table($this->output); $table->setHeaders(['Username', 'Private key password']); @@ -309,10 +303,8 @@ protected function outputPasswords() { /** * write one-time encryption passwords to a csv file - * - * @param array $passwords */ - protected function writePasswordsToFile(array $passwords) { + protected function writePasswordsToFile(array $passwords): void { $fp = $this->rootView->fopen('oneTimeEncryptionPasswords.csv', 'w'); foreach ($passwords as $pwd) { fputcsv($fp, $pwd); @@ -330,10 +322,8 @@ protected function writePasswordsToFile(array $passwords) { /** * setup user file system - * - * @param string $uid */ - protected function setupUserFS($uid) { + protected function setupUserFS(string $uid): void { \OC_Util::tearDownFS(); \OC_Util::setupFS($uid); } @@ -341,10 +331,9 @@ protected function setupUserFS($uid) { /** * generate one time password for the user and store it in a array * - * @param string $uid * @return string password */ - protected function generateOneTimePassword($uid) { + protected function generateOneTimePassword(string $uid): string { $password = $this->secureRandom->generate(16, ISecureRandom::CHAR_HUMAN_READABLE); $this->userPasswords[$uid] = $password; return $password; @@ -353,7 +342,7 @@ protected function generateOneTimePassword($uid) { /** * send encryption key passwords to the users by mail */ - protected function sendPasswordsByMail() { + protected function sendPasswordsByMail(): void { $noMail = []; $this->output->writeln(''); diff --git a/apps/encryption/lib/Crypto/Encryption.php b/apps/encryption/lib/Crypto/Encryption.php index 68bc7df808d9a..96f1d0d9141a8 100644 --- a/apps/encryption/lib/Crypto/Encryption.php +++ b/apps/encryption/lib/Crypto/Encryption.php @@ -438,7 +438,7 @@ public function getUnencryptedBlockSize($signed = false) { * @throws DecryptionFailedException */ public function isReadable($path, $uid) { - $fileKey = $this->keyManager->getFileKey($path, $uid, null); + $fileKey = $this->keyManager->getFileKey($path, null); if (empty($fileKey)) { $owner = $this->util->getOwner($path); if ($owner !== $uid) { diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php index f694e6550f1f7..fe5542cfcdbb6 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -136,7 +136,11 @@ public function validateMasterKey() { if (!$this->session->isPrivateKeySet()) { $masterKey = $this->getSystemPrivateKey($this->masterKeyId); $decryptedMasterKey = $this->crypt->decryptPrivateKey($masterKey, $this->getMasterKeyPassword(), $this->masterKeyId); - $this->session->setPrivateKey($decryptedMasterKey); + if ($decryptedMasterKey === false) { + $this->logger->error('A public master key is available but decrypting it failed. This should never happen.'); + } else { + $this->session->setPrivateKey($decryptedMasterKey); + } } // after the encryption key is available we are ready to go diff --git a/apps/encryption/lib/Recovery.php b/apps/encryption/lib/Recovery.php index 38e78f5e8224c..38d0c48ad8935 100644 --- a/apps/encryption/lib/Recovery.php +++ b/apps/encryption/lib/Recovery.php @@ -67,12 +67,8 @@ public function enableAdminRecovery($password) { /** * change recovery key id - * - * @param string $newPassword - * @param string $oldPassword - * @return bool */ - public function changeRecoveryKeyPassword($newPassword, $oldPassword) { + public function changeRecoveryKeyPassword(string $newPassword, string $oldPassword): bool { $recoveryKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId()); $decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $oldPassword); if ($decryptedRecoveryKey === false) { @@ -80,7 +76,7 @@ public function changeRecoveryKeyPassword($newPassword, $oldPassword) { } $encryptedRecoveryKey = $this->crypt->encryptPrivateKey($decryptedRecoveryKey, $newPassword); $header = $this->crypt->generateHeader(); - if ($encryptedRecoveryKey) { + if ($encryptedRecoveryKey !== false) { $this->keyManager->setSystemPrivateKey($this->keyManager->getRecoveryKeyId(), $header . $encryptedRecoveryKey); return true; } @@ -163,7 +159,7 @@ private function addRecoveryKeys(string $path): void { if ($item['type'] === 'dir') { $this->addRecoveryKeys($filePath . '/'); } else { - $fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID(), null); + $fileKey = $this->keyManager->getFileKey($filePath, null); if (!empty($fileKey)) { $accessList = $this->file->getAccessList($filePath); $publicKeys = []; diff --git a/apps/encryption/lib/Session.php b/apps/encryption/lib/Session.php index e2731d9611dc9..1b641e04f5cac 100644 --- a/apps/encryption/lib/Session.php +++ b/apps/encryption/lib/Session.php @@ -1,24 +1,23 @@ session->set('encryptionInitialized', $status); } @@ -38,7 +37,7 @@ public function setStatus($status) { * * @return string init status INIT_SUCCESSFUL, INIT_EXECUTED, NOT_INITIALIZED */ - public function getStatus() { + public function getStatus(): string { $status = $this->session->get('encryptionInitialized'); if (is_null($status)) { $status = self::NOT_INITIALIZED; @@ -49,10 +48,8 @@ public function getStatus() { /** * check if encryption was initialized successfully - * - * @return bool */ - public function isReady() { + public function isReady(): bool { $status = $this->getStatus(); return $status === self::INIT_SUCCESSFUL; } @@ -63,7 +60,7 @@ public function isReady() { * @return string $privateKey The user's plaintext private key * @throws Exceptions\PrivateKeyMissingException */ - public function getPrivateKey() { + public function getPrivateKey(): string { $key = $this->session->get('privateKey'); if (is_null($key)) { throw new PrivateKeyMissingException('please try to log-out and log-in again', 0); @@ -73,10 +70,8 @@ public function getPrivateKey() { /** * check if private key is set - * - * @return boolean */ - public function isPrivateKeySet() { + public function isPrivateKeySet(): bool { $key = $this->session->get('privateKey'); if (is_null($key)) { return false; @@ -92,17 +87,14 @@ public function isPrivateKeySet() { * * @note this should only be set on login */ - public function setPrivateKey($key) { + public function setPrivateKey(string $key): void { $this->session->set('privateKey', $key); } /** * store data needed for the decrypt all operation in the session - * - * @param string $user - * @param string $key */ - public function prepareDecryptAll($user, $key) { + public function prepareDecryptAll(string $user, string $key): void { $this->session->set('decryptAll', true); $this->session->set('decryptAllKey', $key); $this->session->set('decryptAllUid', $user); @@ -110,10 +102,8 @@ public function prepareDecryptAll($user, $key) { /** * check if we are in decrypt all mode - * - * @return bool */ - public function decryptAllModeActivated() { + public function decryptAllModeActivated(): bool { $decryptAll = $this->session->get('decryptAll'); return ($decryptAll === true); } @@ -121,10 +111,9 @@ public function decryptAllModeActivated() { /** * get uid used for decrypt all operation * - * @return string * @throws \Exception */ - public function getDecryptAllUid() { + public function getDecryptAllUid(): string { $uid = $this->session->get('decryptAllUid'); if (is_null($uid) && $this->decryptAllModeActivated()) { throw new \Exception('No uid found while in decrypt all mode'); @@ -138,10 +127,9 @@ public function getDecryptAllUid() { /** * get private key for decrypt all operation * - * @return string * @throws PrivateKeyMissingException */ - public function getDecryptAllKey() { + public function getDecryptAllKey(): string { $privateKey = $this->session->get('decryptAllKey'); if (is_null($privateKey) && $this->decryptAllModeActivated()) { throw new PrivateKeyMissingException('No private key found while in decrypt all mode'); @@ -155,7 +143,7 @@ public function getDecryptAllKey() { /** * remove keys from session */ - public function clear() { + public function clear(): void { $this->session->remove('publicSharePrivateKey'); $this->session->remove('privateKey'); $this->session->remove('encryptionInitialized'); diff --git a/apps/encryption/tests/Controller/RecoveryControllerTest.php b/apps/encryption/tests/Controller/RecoveryControllerTest.php index 9de583b387c75..19f99b71a36f2 100644 --- a/apps/encryption/tests/Controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/Controller/RecoveryControllerTest.php @@ -86,7 +86,7 @@ public function testChangeRecoveryPassword($password, $confirmPassword, $oldPass ->method('changeRecoveryKeyPassword') ->with($password, $oldPassword) ->willReturnMap([ - ['test', 'oldTestFail', false], + ['test', 'oldtestFail', false], ['test', 'oldtest', true] ]); diff --git a/apps/encryption/tests/Crypto/EncryptionTest.php b/apps/encryption/tests/Crypto/EncryptionTest.php index 7f3d9d1f348c4..a17fcca0219fe 100644 --- a/apps/encryption/tests/Crypto/EncryptionTest.php +++ b/apps/encryption/tests/Crypto/EncryptionTest.php @@ -250,7 +250,7 @@ public function testBeginDecryptAll(): void { ->willReturn(true); $this->keyManagerMock->expects($this->once()) ->method('getFileKey') - ->with($path, 'user', null, true) + ->with($path, null, true) ->willReturn($fileKey); $this->instance->begin($path, 'user', 'r', [], []); diff --git a/apps/encryption/tests/KeyManagerTest.php b/apps/encryption/tests/KeyManagerTest.php index 8f5c9a78faf3a..e20415dd92720 100644 --- a/apps/encryption/tests/KeyManagerTest.php +++ b/apps/encryption/tests/KeyManagerTest.php @@ -353,20 +353,20 @@ public function dataTestGetFileKey() { return [ ['user1', false, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], ['user1', false, 'privateKey', '', 'multiKeyDecryptResult'], - ['user1', false, false, 'legacyKey', ''], - ['user1', false, false, '', ''], + ['user1', false, '', 'legacyKey', ''], + ['user1', false, '', '', ''], ['user1', true, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], ['user1', true, 'privateKey', '', 'multiKeyDecryptResult'], - ['user1', true, false, 'legacyKey', ''], - ['user1', true, false, '', ''], + ['user1', true, '', 'legacyKey', ''], + ['user1', true, '', '', ''], [null, false, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], [null, false, 'privateKey', '', 'multiKeyDecryptResult'], - [null, false, false, 'legacyKey', ''], - [null, false, false, '', ''], + [null, false, '', 'legacyKey', ''], + [null, false, '', '', ''], [null, true, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], [null, true, 'privateKey', '', 'multiKeyDecryptResult'], - [null, true, false, 'legacyKey', ''], - [null, true, false, '', ''], + [null, true, '', 'legacyKey', ''], + [null, true, '', '', ''], ]; } @@ -392,6 +392,7 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $encrypte } $this->invokePrivate($this->instance, 'masterKeyId', ['masterKeyId']); + $this->invokePrivate($this->instance, 'keyUid', [$uid]); $this->keyStorageMock->expects($this->exactly(2)) ->method('getFileKey') @@ -445,7 +446,7 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $encrypte } $this->assertSame($expected, - $this->instance->getFileKey($path, $uid, null) + $this->instance->getFileKey($path, null) ); } diff --git a/apps/encryption/tests/RecoveryTest.php b/apps/encryption/tests/RecoveryTest.php index 17ad385eca001..b7ad0d220dd92 100644 --- a/apps/encryption/tests/RecoveryTest.php +++ b/apps/encryption/tests/RecoveryTest.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\Encryption\Tests; use OC\Files\View; @@ -20,38 +21,16 @@ class RecoveryTest extends TestCase { private static $tempStorage = []; - /** - * @var IFile|\PHPUnit\Framework\MockObject\MockObject - */ - private $fileMock; - /** - * @var View|\PHPUnit\Framework\MockObject\MockObject - */ - private $viewMock; - /** - * @var IUserSession|\PHPUnit\Framework\MockObject\MockObject - */ - private $userSessionMock; - /** - * @var MockObject|IUser - */ - private $user; - /** - * @var KeyManager|\PHPUnit\Framework\MockObject\MockObject - */ - private $keyManagerMock; - /** - * @var IConfig|\PHPUnit\Framework\MockObject\MockObject - */ - private $configMock; - /** - * @var Crypt|\PHPUnit\Framework\MockObject\MockObject - */ - private $cryptMock; - /** - * @var Recovery - */ - private $instance; + + private IFile&MockObject $fileMock; + private View&MockObject $viewMock; + private IUserSession&MockObject $userSessionMock; + private IUser&MockObject $user; + private KeyManager&MockObject $keyManagerMock; + private IConfig&MockObject $configMock; + private Crypt&MockObject $cryptMock; + + private Recovery $instance; public function testEnableAdminRecoverySuccessful(): void { $this->keyManagerMock->expects($this->exactly(2)) @@ -120,21 +99,23 @@ public function testEnableAdminRecoveryCouldNotCreateKey(): void { } public function testChangeRecoveryKeyPasswordSuccessful(): void { - $this->assertFalse($this->instance->changeRecoveryKeyPassword('password', - 'passwordOld')); + $this->assertFalse($this->instance->changeRecoveryKeyPassword('password', 'passwordOld')); $this->keyManagerMock->expects($this->once()) - ->method('getSystemPrivateKey'); + ->method('getSystemPrivateKey') + ->willReturn('privateKey'); $this->cryptMock->expects($this->once()) - ->method('decryptPrivateKey'); + ->method('decryptPrivateKey') + ->with('privateKey', 'passwordOld') + ->willReturn('decryptedPrivateKey'); $this->cryptMock->expects($this->once()) ->method('encryptPrivateKey') - ->willReturn(true); + ->with('decryptedPrivateKey', 'password') + ->willReturn('privateKey'); - $this->assertTrue($this->instance->changeRecoveryKeyPassword('password', - 'passwordOld')); + $this->assertTrue($this->instance->changeRecoveryKeyPassword('password', 'passwordOld')); } public function testChangeRecoveryKeyPasswordCouldNotDecryptPrivateRecoveryKey(): void { diff --git a/apps/encryption/tests/SessionTest.php b/apps/encryption/tests/SessionTest.php index 3072bc6ef3bc2..16998bf1c6b72 100644 --- a/apps/encryption/tests/SessionTest.php +++ b/apps/encryption/tests/SessionTest.php @@ -77,7 +77,7 @@ public function testGetDecryptAllUidException(): void { public function testGetDecryptAllUidException2(): void { $this->expectException(\Exception::class); - $this->instance->prepareDecryptAll(null, 'key'); + $this->instance->prepareDecryptAll('', 'key'); $this->instance->getDecryptAllUid(); } @@ -96,7 +96,7 @@ public function testGetDecryptAllKeyException(): void { public function testGetDecryptAllKeyException2(): void { $this->expectException(PrivateKeyMissingException::class); - $this->instance->prepareDecryptAll('user', null); + $this->instance->prepareDecryptAll('user', ''); $this->instance->getDecryptAllKey(); } diff --git a/build/integration/features/bootstrap/CommandLine.php b/build/integration/features/bootstrap/CommandLine.php index 84b3dfd447f19..59d3409ff2506 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -25,7 +25,7 @@ trait CommandLine { * @param []string $args OCC command, the part behind "occ". For example: "files:transfer-ownership" * @return int exit code */ - public function runOcc($args = []) { + public function runOcc($args = [], string $inputString = '') { $args = array_map(function ($arg) { return escapeshellarg($arg); }, $args); @@ -38,6 +38,10 @@ public function runOcc($args = []) { 2 => ['pipe', 'w'], ]; $process = proc_open('php console.php ' . $args, $descriptor, $pipes, $this->ocPath); + if ($inputString !== '') { + fwrite($pipes[0], $inputString . "\n"); + fclose($pipes[0]); + } $this->lastStdOut = stream_get_contents($pipes[1]); $this->lastStdErr = stream_get_contents($pipes[2]); $this->lastCode = proc_close($process); @@ -57,6 +61,14 @@ public function invokingTheCommand($cmd) { $this->runOcc($args); } + /** + * @Given /^invoking occ with "([^"]*)" with input "([^"]+)"$/ + */ + public function invokingTheCommandWith($cmd, $inputString) { + $args = explode(' ', $cmd); + $this->runOcc($args, $inputString); + } + /** * Find exception texts in stderr */ @@ -125,6 +137,13 @@ public function theCommandOutputContainsTheText($text) { Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout'); } + /** + * @Then /^the command output does not contain the text "([^"]*)"$/ + */ + public function theCommandOutputDoesNotContainTheText($text) { + Assert::assertStringNotContainsString($text, $this->lastStdOut, 'The command did output the not expected text on stdout'); + } + /** * @Then /^the command error output contains the text "([^"]*)"$/ */ diff --git a/build/integration/files_features/encryption.feature b/build/integration/files_features/encryption.feature new file mode 100644 index 0000000000000..d961f15267151 --- /dev/null +++ b/build/integration/files_features/encryption.feature @@ -0,0 +1,42 @@ +# SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: AGPL-3.0-or-later +Feature: encryption + + Scenario: encryption tests + # Setup encryption + Given using new dav path + And user "user0" exists + And User "user0" uploads file with content "BLABLABLA" to "/non-encrypted.txt" + And invoking occ with "app:enable encryption" + And the command was successful + And invoking occ with "encryption:enable" + And the command was successful + And As an "user0" + And User "user0" uploads file with content "BLABLABLA" to "/encrypted.txt" + # Check both encrypted and non-encrypted files can be read + When Downloading file "/encrypted.txt" with range "bytes=0-8" + Then Downloaded content should be "BLABLABLA" + When Downloading file "/non-encrypted.txt" with range "bytes=0-8" + Then Downloaded content should be "BLABLABLA" + When invoking occ with "info:file user0/files/encrypted.txt" + And the command was successful + Then the command output contains the text "server-side encrypted: yes" + When invoking occ with "info:file user0/files/non-encrypted.txt" + And the command was successful + Then the command output does not contain the text "server-side encrypted: yes" + # Run encryption:encrypt-all and checks that non-encrypted file gets encrypted + When invoking occ with "encryption:encrypt-all" with input "y" + And the command was successful + And invoking occ with "info:file user0/files/non-encrypted.txt" + And the command was successful + Then the command output contains the text "server-side encrypted: yes" + And Downloading file "/non-encrypted.txt" with range "bytes=0-8" + And Downloaded content should be "BLABLABLA" + # Run encryption:decrypt-all and checks that files gets decrypted + When invoking occ with "encryption:decrypt-all" with input "y" + And the command was successful + And invoking occ with "info:file user0/files/non-encrypted.txt" + And the command was successful + 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" diff --git a/lib/private/Encryption/DecryptAll.php b/lib/private/Encryption/DecryptAll.php index 70dd0c0f0b0c2..59d0eb03d28e1 100644 --- a/lib/private/Encryption/DecryptAll.php +++ b/lib/private/Encryption/DecryptAll.php @@ -1,10 +1,13 @@ > files which couldn't be decrypted */ + protected array $failed = []; public function __construct( protected IManager $encryptionManager, protected IUserManager $userManager, protected View $rootView, ) { - $this->failed = []; } /** * start to decrypt all files * - * @param InputInterface $input - * @param OutputInterface $output * @param string $user which users data folder should be decrypted, default = all users - * @return bool * @throws \Exception */ - public function decryptAll(InputInterface $input, OutputInterface $output, $user = '') { - $this->input = $input; - $this->output = $output; - + public function decryptAll(InputInterface $input, OutputInterface $output, string $user = ''): bool { if ($user !== '' && $this->userManager->userExists($user) === false) { - $this->output->writeln('User "' . $user . '" does not exist. Please check the username and try again'); + $output->writeln('User "' . $user . '" does not exist. Please check the username and try again'); return false; } - $this->output->writeln('prepare encryption modules...'); - if ($this->prepareEncryptionModules($user) === false) { + $output->writeln('prepare encryption modules...'); + if ($this->prepareEncryptionModules($input, $output, $user) === false) { return false; } - $this->output->writeln(' done.'); + $output->writeln(' done.'); - $this->decryptAllUsersFiles($user); + $this->failed = []; + $this->decryptAllUsersFiles($output, $user); + /** @psalm-suppress RedundantCondition $this->failed is modified by decryptAllUsersFiles, not clear why psalm fails to see it */ if (empty($this->failed)) { - $this->output->writeln('all files could be decrypted successfully!'); + $output->writeln('all files could be decrypted successfully!'); } else { - $this->output->writeln('Files for following users couldn\'t be decrypted, '); - $this->output->writeln('maybe the user is not set up in a way that supports this operation: '); + $output->writeln('Files for following users couldn\'t be decrypted, '); + $output->writeln('maybe the user is not set up in a way that supports this operation: '); foreach ($this->failed as $uid => $paths) { - $this->output->writeln(' ' . $uid); + $output->writeln(' ' . $uid); foreach ($paths as $path) { - $this->output->writeln(' ' . $path); + $output->writeln(' ' . $path); } } - $this->output->writeln(''); + $output->writeln(''); } return true; @@ -79,21 +71,18 @@ public function decryptAll(InputInterface $input, OutputInterface $output, $user /** * prepare encryption modules to perform the decrypt all function - * - * @param $user - * @return bool */ - protected function prepareEncryptionModules($user) { + protected function prepareEncryptionModules(InputInterface $input, OutputInterface $output, string $user): bool { // prepare all encryption modules for decrypt all $encryptionModules = $this->encryptionManager->getEncryptionModules(); foreach ($encryptionModules as $moduleDesc) { /** @var IEncryptionModule $module */ $module = call_user_func($moduleDesc['callback']); - $this->output->writeln(''); - $this->output->writeln('Prepare "' . $module->getDisplayName() . '"'); - $this->output->writeln(''); - if ($module->prepareDecryptAll($this->input, $this->output, $user) === false) { - $this->output->writeln('Module "' . $moduleDesc['displayName'] . '" does not support the functionality to decrypt all files again or the initialization of the module failed!'); + $output->writeln(''); + $output->writeln('Prepare "' . $module->getDisplayName() . '"'); + $output->writeln(''); + if ($module->prepareDecryptAll($input, $output, $user) === false) { + $output->writeln('Module "' . $moduleDesc['displayName'] . '" does not support the functionality to decrypt all files again or the initialization of the module failed!'); return false; } } @@ -106,12 +95,12 @@ protected function prepareEncryptionModules($user) { * * @param string $user which users files should be decrypted, default = all users */ - protected function decryptAllUsersFiles($user = '') { - $this->output->writeln("\n"); + protected function decryptAllUsersFiles(OutputInterface $output, string $user = ''): void { + $output->writeln("\n"); $userList = []; if ($user === '') { - $fetchUsersProgress = new ProgressBar($this->output); + $fetchUsersProgress = new ProgressBar($output); $fetchUsersProgress->setFormat(" %message% \n [%bar%]"); $fetchUsersProgress->start(); $fetchUsersProgress->setMessage('Fetch list of users...'); @@ -135,9 +124,9 @@ protected function decryptAllUsersFiles($user = '') { $userList[] = $user; } - $this->output->writeln("\n\n"); + $output->writeln("\n\n"); - $progress = new ProgressBar($this->output); + $progress = new ProgressBar($output); $progress->setFormat(" %message% \n [%bar%]"); $progress->start(); $progress->setMessage('starting to decrypt files...'); @@ -154,17 +143,13 @@ protected function decryptAllUsersFiles($user = '') { $progress->setMessage('starting to decrypt files... finished'); $progress->finish(); - $this->output->writeln("\n\n"); + $output->writeln("\n\n"); } /** * encrypt files from the given user - * - * @param string $uid - * @param ProgressBar $progress - * @param string $userCount */ - protected function decryptUsersFiles($uid, ProgressBar $progress, $userCount) { + protected function decryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void { $this->setupUserFS($uid); $directories = []; $directories[] = '/' . $uid . '/files'; @@ -207,11 +192,8 @@ protected function decryptUsersFiles($uid, ProgressBar $progress, $userCount) { /** * encrypt file - * - * @param string $path - * @return bool */ - protected function decryptFile($path) { + protected function decryptFile(string $path): bool { // skip already decrypted files $fileInfo = $this->rootView->getFileInfo($path); if ($fileInfo !== false && !$fileInfo->isEncrypted()) { @@ -237,20 +219,15 @@ protected function decryptFile($path) { /** * get current timestamp - * - * @return int */ - protected function getTimestamp() { + protected function getTimestamp(): int { return time(); } - /** * setup user file system - * - * @param string $uid */ - protected function setupUserFS($uid) { + protected function setupUserFS(string $uid): void { \OC_Util::tearDownFS(); \OC_Util::setupFS($uid); } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index cb160115851d8..f38ee8b782373 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -646,6 +646,13 @@ protected function hasEncryptionWrapper(): bool { return $this->storage->instanceOfStorage(Encryption::class); } + protected function shouldEncrypt(string $targetPath): bool { + if (!$this->storage->instanceOfStorage(Encryption::class)) { + return false; + } + return $this->storage->shouldEncrypt($targetPath); + } + /** * Move a file or folder in the cache * @@ -1164,7 +1171,9 @@ public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, str $data = $this->cacheEntryToArray($sourceEntry); // when moving from an encrypted storage to a non-encrypted storage remove the `encrypted` mark - if ($sourceCache instanceof Cache && $sourceCache->hasEncryptionWrapper() && !$this->hasEncryptionWrapper()) { + if ($sourceCache instanceof Cache + && $sourceCache->hasEncryptionWrapper() + && !$this->shouldEncrypt($targetPath)) { $data['encrypted'] = 0; } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 03d4038186fef..b76bf3cb49589 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -337,7 +337,7 @@ public function fopen(string $path, string $mode) { } // encryption disabled on write of new file and write to existing unencrypted file -> don't encrypt - if (!$encryptionEnabled || !$this->shouldEncrypt($path)) { + if (!$this->shouldEncrypt($path)) { if (!$targetExists || !$targetIsEncrypted) { $shouldEncrypt = false; } @@ -573,7 +573,7 @@ private function updateEncryptedVersion( bool $isRename, bool $keepEncryptionVersion, ): void { - $isEncrypted = $this->encryptionManager->isEnabled() && $this->shouldEncrypt($targetInternalPath); + $isEncrypted = $this->shouldEncrypt($targetInternalPath); $cacheInformation = [ 'encrypted' => $isEncrypted, ]; @@ -872,7 +872,10 @@ protected function isVersion(string $path): bool { /** * check if the given storage should be encrypted or not */ - protected function shouldEncrypt(string $path): bool { + public function shouldEncrypt(string $path): bool { + if (!$this->encryptionManager->isEnabled()) { + return false; + } $fullPath = $this->getFullPath($path); $mountPointConfig = $this->mount->getOption('encrypt', true); if ($mountPointConfig === false) { diff --git a/tests/lib/Encryption/DecryptAllTest.php b/tests/lib/Encryption/DecryptAllTest.php index 6a8453bcaf8b8..3a4d4a072ca59 100644 --- a/tests/lib/Encryption/DecryptAllTest.php +++ b/tests/lib/Encryption/DecryptAllTest.php @@ -15,6 +15,7 @@ use OCP\Files\Storage\IStorage; use OCP\IUserManager; use OCP\UserInterface; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Formatter\OutputFormatterInterface; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; @@ -29,8 +30,12 @@ * @package Test\Encryption */ class DecryptAllTest extends TestCase { - /** @var \PHPUnit\Framework\MockObject\MockObject | IUserManager */ - protected $userManager; + private IUserManager&MockObject $userManager; + private Manager&MockObject $encryptionManager; + private View&MockObject $view; + private InputInterface&MockObject $inputInterface; + private OutputInterface&MockObject $outputInterface; + private UserInterface&MockObject $userInterface; /** @var \PHPUnit\Framework\MockObject\MockObject | Manager */ protected $encryptionManager; @@ -104,7 +109,7 @@ public function testDecryptAll($prepareResult, $user, $userExistsChecked): void } else { $this->userManager->expects($this->never())->method('userExists'); } - /** @var DecryptAll | \PHPUnit\Framework\MockObject\MockObject | $instance */ + /** @var DecryptAll&MockObject $instance */ $instance = $this->getMockBuilder('OC\Encryption\DecryptAll') ->setConstructorArgs( [ @@ -118,13 +123,13 @@ public function testDecryptAll($prepareResult, $user, $userExistsChecked): void $instance->expects($this->once()) ->method('prepareEncryptionModules') - ->with($user) + ->with($this->inputInterface, $this->outputInterface, $user) ->willReturn($prepareResult); if ($prepareResult) { $instance->expects($this->once()) ->method('decryptAllUsersFiles') - ->with($user); + ->with($this->outputInterface, $user); } else { $instance->expects($this->never())->method('decryptAllUsersFiles'); } @@ -181,7 +186,7 @@ public function testPrepareEncryptionModules($success): void { ->willReturn([$moduleDescription]); $this->assertSame($success, - $this->invokePrivate($this->instance, 'prepareEncryptionModules', [$user]) + $this->invokePrivate($this->instance, 'prepareEncryptionModules', [$this->inputInterface, $this->outputInterface, $user]) ); } @@ -189,7 +194,7 @@ public function testPrepareEncryptionModules($success): void { * @dataProvider dataTestDecryptAllUsersFiles */ public function testDecryptAllUsersFiles($user): void { - /** @var DecryptAll | \PHPUnit\Framework\MockObject\MockObject | $instance */ + /** @var DecryptAll&MockObject $instance */ $instance = $this->getMockBuilder('OC\Encryption\DecryptAll') ->setConstructorArgs( [ @@ -201,9 +206,6 @@ public function testDecryptAllUsersFiles($user): void { ->setMethods(['decryptUsersFiles']) ->getMock(); - $this->invokePrivate($instance, 'input', [$this->inputInterface]); - $this->invokePrivate($instance, 'output', [$this->outputInterface]); - if (empty($user)) { $this->userManager->expects($this->once()) ->method('getBackends') @@ -223,7 +225,7 @@ public function testDecryptAllUsersFiles($user): void { ->with($user); } - $this->invokePrivate($instance, 'decryptAllUsersFiles', [$user]); + $this->invokePrivate($instance, 'decryptAllUsersFiles', [$this->outputInterface, $user]); } public function dataTestDecryptAllUsersFiles() { @@ -311,7 +313,7 @@ function ($path) { public function testDecryptFile($isEncrypted): void { $path = 'test.txt'; - /** @var DecryptAll | \PHPUnit\Framework\MockObject\MockObject $instance */ + /** @var DecryptAll&MockObject $instance */ $instance = $this->getMockBuilder('OC\Encryption\DecryptAll') ->setConstructorArgs( [ @@ -351,7 +353,7 @@ public function testDecryptFile($isEncrypted): void { public function testDecryptFileFailure(): void { $path = 'test.txt'; - /** @var DecryptAll | \PHPUnit\Framework\MockObject\MockObject $instance */ + /** @var DecryptAll&MockObject $instance */ $instance = $this->getMockBuilder('OC\Encryption\DecryptAll') ->setConstructorArgs( [ diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index bb3df36dec2bf..866d9da107104 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -959,10 +959,11 @@ public function dataTestIsVersion() { * @param bool $expected */ public function testShouldEncrypt( - $encryptMountPoint, - $encryptionModule, - $encryptionModuleShouldEncrypt, - $expected, + bool $encryptionEnabled, + bool $encryptMountPoint, + ?bool $encryptionModule, + bool $encryptionModuleShouldEncrypt, + bool $expected, ): void { $encryptionManager = $this->createMock(\OC\Encryption\Manager::class); $util = $this->createMock(Util::class); @@ -994,13 +995,15 @@ public function testShouldEncrypt( ->setMethods(['getFullPath', 'getEncryptionModule']) ->getMock(); + $encryptionManager->method('isEnabled')->willReturn($encryptionEnabled); + if ($encryptionModule === true) { /** @var IEncryptionModule|MockObject $encryptionModule */ $encryptionModule = $this->createMock(IEncryptionModule::class); } $wrapper->method('getFullPath')->with($path)->willReturn($fullPath); - $wrapper->expects($encryptMountPoint ? $this->once() : $this->never()) + $wrapper->expects(($encryptionEnabled && $encryptMountPoint) ? $this->once() : $this->never()) ->method('getEncryptionModule') ->with($fullPath) ->willReturnCallback( @@ -1011,7 +1014,8 @@ function () use ($encryptionModule) { return $encryptionModule; } ); - $mount->expects($this->once())->method('getOption')->with('encrypt', true) + $mount->expects($encryptionEnabled ? $this->once() : $this->never()) + ->method('getOption')->with('encrypt', true) ->willReturn($encryptMountPoint); if ($encryptionModule !== null && $encryptionModule !== false) { @@ -1035,11 +1039,12 @@ function () use ($encryptionModule) { public function dataTestShouldEncrypt() { return [ - [false, false, false, false], - [true, false, false, false], - [true, true, false, false], - [true, true, true, true], - [true, null, false, true], + [true, false, false, false, false], + [true, true, false, false, false], + [true, true, true, false, false], + [true, true, true, true, true], + [true, true, null, false, true], + [false, true, true, true, false], ]; } } diff --git a/tests/lib/Traits/EncryptionTrait.php b/tests/lib/Traits/EncryptionTrait.php index e435a28668530..c0aa143bb9e3b 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -76,6 +76,7 @@ protected function setupForUser($name, $password) { $encryptionManager = $container->query(IManager::class); $this->encryptionApp->setUp($encryptionManager); $keyManager->init($name, $password); + $this->invokePrivate($keyManager, 'keyUid', [$name]); } protected function postLogin() {