diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index 5c6d0f03334..ec285363507 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -1,11 +1,13 @@ 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 463ca4e22bb..f80daec728a 100644 --- a/apps/encryption/lib/Crypto/Crypt.php +++ b/apps/encryption/lib/Crypto/Crypt.php @@ -342,9 +342,8 @@ class Crypt { * @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 362f43b8672..7123d0f9a64 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 @@ class DecryptAll { $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 @@ class DecryptAll { /** * 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 @@ class DecryptAll { 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 4ed75b85a93..db6135787ef 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 @@ class EncryptAll { /** * 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 6d388624e48..daeb9859b41 100644 --- a/apps/encryption/lib/Crypto/Encryption.php +++ b/apps/encryption/lib/Crypto/Encryption.php @@ -127,7 +127,7 @@ class Encryption implements IEncryptionModule { /* If useLegacyFileKey is not specified in header, auto-detect, to be safe */ $useLegacyFileKey = (($header['useLegacyFileKey'] ?? '') == 'false' ? false : null); - $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user, $useLegacyFileKey, $this->session->decryptAllModeActivated()); + $this->fileKey = $this->keyManager->getFileKey($this->path, $useLegacyFileKey, $this->session->decryptAllModeActivated()); // always use the version from the original file, also part files // need to have a correct version number if they get moved over to the @@ -322,7 +322,7 @@ class Encryption implements IEncryptionModule { * update encrypted file, e.g. give additional users access to the file * * @param string $path path to the file which should be updated - * @param string $uid of the user who performs the operation + * @param string $uid ignored * @param array $accessList who has access to the file contains the key 'users' and 'public' * @return bool */ @@ -335,7 +335,7 @@ class Encryption implements IEncryptionModule { return false; } - $fileKey = $this->keyManager->getFileKey($path, $uid, null); + $fileKey = $this->keyManager->getFileKey($path, null); if (!empty($fileKey)) { $publicKeys = []; @@ -438,7 +438,7 @@ class Encryption implements IEncryptionModule { * @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 f9c1ef94634..b3e49c8124c 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -23,7 +23,7 @@ class KeyManager { private string $recoveryKeyId; private string $publicShareKeyId; private string $masterKeyId; - private string $keyId; + private ?string $keyUid; private string $publicKeyId = 'publicKey'; private string $privateKeyId = 'privateKey'; private string $shareKeyId = 'shareKey'; @@ -62,7 +62,7 @@ class KeyManager { $this->config->setAppValue('encryption', 'masterKeyId', $this->masterKeyId); } - $this->keyId = $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : false; + $this->keyUid = $userSession->isLoggedIn() ? $userSession->getUser()?->getUID() : null; } /** @@ -136,7 +136,11 @@ class KeyManager { 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 @@ -347,11 +351,8 @@ class KeyManager { /** * @param ?bool $useLegacyFileKey null means try both */ - public function getFileKey(string $path, ?string $uid, ?bool $useLegacyFileKey, bool $useDecryptAll = false): string { - if ($uid === '') { - $uid = null; - } - $publicAccess = is_null($uid); + public function getFileKey(string $path, ?bool $useLegacyFileKey, bool $useDecryptAll = false): string { + $publicAccess = ($this->keyUid === null); $encryptedFileKey = ''; if ($useLegacyFileKey ?? true) { $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID); @@ -380,6 +381,7 @@ class KeyManager { $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.' . $this->privateKeyId, Encryption::ID); $privateKey = $this->crypt->decryptPrivateKey($privateKey); } else { + $uid = $this->keyUid; $shareKey = $this->getShareKey($path, $uid); $privateKey = $this->session->getPrivateKey(); } diff --git a/apps/encryption/lib/Recovery.php b/apps/encryption/lib/Recovery.php index 38e78f5e822..38d0c48ad89 100644 --- a/apps/encryption/lib/Recovery.php +++ b/apps/encryption/lib/Recovery.php @@ -67,12 +67,8 @@ class Recovery { /** * 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 @@ class Recovery { } $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 @@ class Recovery { 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 df1e5d664ad..437450b7e7c 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 @@ class Session { * * @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 @@ class Session { /** * 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 @@ class Session { * @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'); @@ -73,10 +70,8 @@ class Session { /** * 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 @@ class Session { * * @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 @@ class Session { /** * 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 @@ class Session { /** * 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 @@ class Session { /** * 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 @@ class Session { /** * 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 0fec3f4d6a9..717e31cf53b 100644 --- a/apps/encryption/tests/Controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/Controller/RecoveryControllerTest.php @@ -85,7 +85,7 @@ class RecoveryControllerTest extends TestCase { ->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 37e484550ef..df9655cd2eb 100644 --- a/apps/encryption/tests/Crypto/EncryptionTest.php +++ b/apps/encryption/tests/Crypto/EncryptionTest.php @@ -232,7 +232,7 @@ class EncryptionTest extends TestCase { ->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 3fe76fc4f59..14d2a13e045 100644 --- a/apps/encryption/tests/KeyManagerTest.php +++ b/apps/encryption/tests/KeyManagerTest.php @@ -335,32 +335,25 @@ class KeyManagerTest extends TestCase { 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, '', '', ''], ]; } - /** - * - * @param $uid - * @param $isMasterKeyEnabled - * @param $privateKey - * @param $expected - */ #[\PHPUnit\Framework\Attributes\DataProvider('dataTestGetFileKey')] - public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $encryptedFileKey, $expected): void { + public function testGetFileKey(?string $uid, bool $isMasterKeyEnabled, string $privateKey, string $encryptedFileKey, string $expected): void { $path = '/foo.txt'; if ($isMasterKeyEnabled) { @@ -374,6 +367,7 @@ class KeyManagerTest extends TestCase { } $this->invokePrivate($this->instance, 'masterKeyId', ['masterKeyId']); + $this->invokePrivate($this->instance, 'keyUid', [$uid]); $this->keyStorageMock->expects($this->exactly(2)) ->method('getFileKey') @@ -423,7 +417,7 @@ class KeyManagerTest extends TestCase { } $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 0627724a856..74d472adeec 100644 --- a/apps/encryption/tests/RecoveryTest.php +++ b/apps/encryption/tests/RecoveryTest.php @@ -7,6 +7,7 @@ declare(strict_types=1); * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\Encryption\Tests; use OC\Files\View; @@ -22,38 +23,16 @@ use Test\TestCase; 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)) @@ -122,21 +101,23 @@ class RecoveryTest extends TestCase { } 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 986502749c8..c24855e1ac4 100644 --- a/apps/encryption/tests/SessionTest.php +++ b/apps/encryption/tests/SessionTest.php @@ -76,7 +76,7 @@ class SessionTest extends TestCase { public function testGetDecryptAllUidException2(): void { $this->expectException(\Exception::class); - $this->instance->prepareDecryptAll(null, 'key'); + $this->instance->prepareDecryptAll('', 'key'); $this->instance->getDecryptAllUid(); } @@ -95,7 +95,7 @@ class SessionTest extends TestCase { 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 924d723daa6..0ffa1b9623d 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -26,7 +26,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); @@ -39,6 +39,10 @@ trait CommandLine { 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); @@ -58,6 +62,14 @@ trait CommandLine { $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 */ @@ -126,6 +138,13 @@ trait CommandLine { 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 00000000000..d961f152671 --- /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 70dd0c0f0b0..59d0eb03d28 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 @@ class DecryptAll { /** * 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 @@ class DecryptAll { * * @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 @@ class DecryptAll { $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 @@ class DecryptAll { $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 @@ class DecryptAll { /** * 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 @@ class DecryptAll { /** * 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/Encryption/EncryptionEventListener.php b/lib/private/Encryption/EncryptionEventListener.php index d51b4b0d531..4560c4796df 100644 --- a/lib/private/Encryption/EncryptionEventListener.php +++ b/lib/private/Encryption/EncryptionEventListener.php @@ -67,21 +67,16 @@ class EncryptionEventListener implements IEventListener { } private function getUpdate(?IUser $owner = null): Update { - if (is_null($this->updater)) { - $user = $this->userSession->getUser(); - if (!$user && ($owner !== null)) { - $user = $owner; - } - if (!$user) { - throw new \Exception('Inconsistent data, File unshared, but owner not found. Should not happen'); - } - - $uid = $user->getUID(); - + $user = $this->userSession->getUser(); + if (!$user && ($owner !== null)) { + $user = $owner; + } + if ($user) { if (!$this->setupManager->isSetupComplete($user)) { $this->setupManager->setupForUser($user); } - + } + if (is_null($this->updater)) { $this->updater = new Update( new Util( new View(), @@ -91,7 +86,6 @@ class EncryptionEventListener implements IEventListener { \OC::$server->getEncryptionManager(), \OC::$server->get(IFile::class), \OC::$server->get(LoggerInterface::class), - $uid ); } diff --git a/lib/private/Encryption/Update.php b/lib/private/Encryption/Update.php index 293a1ce653c..fb54e640be5 100644 --- a/lib/private/Encryption/Update.php +++ b/lib/private/Encryption/Update.php @@ -27,7 +27,6 @@ class Update { protected Manager $encryptionManager, protected File $file, protected LoggerInterface $logger, - protected string $uid, ) { } @@ -108,10 +107,10 @@ class Update { foreach ($allFiles as $file) { $usersSharing = $this->file->getAccessList($file); try { - $encryptionModule->update($file, $this->uid, $usersSharing); + $encryptionModule->update($file, '', $usersSharing); } catch (GenericEncryptionException $e) { // If the update of an individual file fails e.g. due to a corrupt key we should continue the operation and just log the failure - $this->logger->error('Failed to update encryption module for ' . $this->uid . ' ' . $file, [ 'exception' => $e ]); + $this->logger->error('Failed to update encryption module for ' . $file, [ 'exception' => $e ]); } } } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index b6b550150f1..c76d0276a47 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -655,6 +655,13 @@ class Cache implements ICache { 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 * @@ -1173,7 +1180,9 @@ class Cache implements ICache { $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 fb6cdf80c6c..ac83d90f81a 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -337,7 +337,7 @@ class Encryption extends Wrapper { } // 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; } @@ -585,7 +585,7 @@ class Encryption extends Wrapper { bool $isRename, bool $keepEncryptionVersion, ): void { - $isEncrypted = $this->encryptionManager->isEnabled() && $this->shouldEncrypt($targetInternalPath); + $isEncrypted = $this->shouldEncrypt($targetInternalPath); $cacheInformation = [ 'encrypted' => $isEncrypted, ]; @@ -884,7 +884,10 @@ class Encryption extends Wrapper { /** * 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 979e12e03b3..5b56ac271c5 100644 --- a/tests/lib/Encryption/DecryptAllTest.php +++ b/tests/lib/Encryption/DecryptAllTest.php @@ -1,5 +1,7 @@ userManager->expects($this->once())->method('userExists')->willReturn(true); } 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( [ @@ -119,13 +105,13 @@ class DecryptAllTest extends TestCase { $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'); } @@ -182,13 +168,13 @@ class DecryptAllTest extends TestCase { ->willReturn([$moduleDescription]); $this->assertSame($success, - $this->invokePrivate($this->instance, 'prepareEncryptionModules', [$user]) + $this->invokePrivate($this->instance, 'prepareEncryptionModules', [$this->inputInterface, $this->outputInterface, $user]) ); } #[\PHPUnit\Framework\Attributes\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( [ @@ -200,9 +186,6 @@ class DecryptAllTest extends TestCase { ->onlyMethods(['decryptUsersFiles']) ->getMock(); - $this->invokePrivate($instance, 'input', [$this->inputInterface]); - $this->invokePrivate($instance, 'output', [$this->outputInterface]); - if (empty($user)) { $this->userManager->expects($this->once()) ->method('getBackends') @@ -226,7 +209,7 @@ class DecryptAllTest extends TestCase { ->with($user); } - $this->invokePrivate($instance, 'decryptAllUsersFiles', [$user]); + $this->invokePrivate($instance, 'decryptAllUsersFiles', [$this->outputInterface, $user]); } public static function dataTestDecryptAllUsersFiles(): array { @@ -296,9 +279,10 @@ class DecryptAllTest extends TestCase { ]; $instance->expects($this->exactly(2)) ->method('decryptFile') - ->willReturnCallback(function ($path) use (&$calls): void { + ->willReturnCallback(function ($path) use (&$calls): bool { $expected = array_shift($calls); $this->assertEquals($expected, $path); + return true; }); @@ -320,7 +304,7 @@ class DecryptAllTest extends TestCase { 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( [ @@ -360,7 +344,7 @@ class DecryptAllTest extends TestCase { 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 3e643714300..cd82d200de8 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -936,19 +936,13 @@ class EncryptionTest extends Storage { ]; } - /** - * - * @param bool $encryptMountPoint - * @param mixed $encryptionModule - * @param bool $encryptionModuleShouldEncrypt - * @param bool $expected - */ #[\PHPUnit\Framework\Attributes\DataProvider('dataTestShouldEncrypt')] 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); @@ -978,13 +972,15 @@ class EncryptionTest extends Storage { ->onlyMethods(['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( @@ -995,7 +991,8 @@ class EncryptionTest extends Storage { 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) { @@ -1019,11 +1016,12 @@ class EncryptionTest extends Storage { public static function dataTestShouldEncrypt(): array { 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 44d5dcab8cb..2dc9213ff24 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -82,6 +82,7 @@ trait EncryptionTrait { $encryptionManager = $container->query(IManager::class); $this->encryptionApp->setUp($encryptionManager); $keyManager->init($name, $password); + $this->invokePrivate($keyManager, 'keyUid', [$name]); } protected function postLogin() {