From a3442be054f87e8128d1a3e9255c8bbbcd2ace16 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 1 Oct 2025 15:00:20 +0200 Subject: [PATCH] refactor: fix psalm issues from encryption commands Signed-off-by: Carl Schwan --- .../lib/Command/DisableMasterKey.php | 6 +- .../lib/Command/DropLegacyFileKey.php | 29 ++--- .../lib/Command/EnableMasterKey.php | 6 +- .../lib/Command/FixEncryptedVersion.php | 54 ++++---- .../encryption/lib/Command/FixKeyLocation.php | 15 ++- .../lib/Command/ScanLegacyFormat.php | 33 ++--- apps/encryption/lib/Crypto/EncryptAll.php | 120 ++++++++---------- .../tests/Command/FixEncryptedVersionTest.php | 19 +-- .../tests/Command/TestEnableMasterKey.php | 40 ++---- .../tests/Crypto/EncryptAllTest.php | 58 ++++++--- build/psalm-baseline.xml | 54 -------- 11 files changed, 185 insertions(+), 249 deletions(-) diff --git a/apps/encryption/lib/Command/DisableMasterKey.php b/apps/encryption/lib/Command/DisableMasterKey.php index 0b8b8e39e78..53902e42ed1 100644 --- a/apps/encryption/lib/Command/DisableMasterKey.php +++ b/apps/encryption/lib/Command/DisableMasterKey.php @@ -7,7 +7,7 @@ namespace OCA\Encryption\Command; use OCA\Encryption\Util; -use OCP\IConfig; +use OCP\AppFramework\Services\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputInterface; @@ -17,7 +17,7 @@ use Symfony\Component\Console\Question\ConfirmationQuestion; class DisableMasterKey extends Command { public function __construct( protected Util $util, - protected IConfig $config, + protected IAppConfig $config, protected QuestionHelper $questionHelper, ) { parent::__construct(); @@ -45,7 +45,7 @@ class DisableMasterKey extends Command { . 'Do you really want to switch to per-user keys? (y/n) ', false); if ($this->questionHelper->ask($input, $output, $question)) { - $this->config->setAppValue('encryption', 'useMasterKey', '0'); + $this->config->setAppValueBool('useMasterKey', false); $output->writeln('Master key successfully disabled.'); return self::SUCCESS; } diff --git a/apps/encryption/lib/Command/DropLegacyFileKey.php b/apps/encryption/lib/Command/DropLegacyFileKey.php index e487a0aa013..96f9485100a 100644 --- a/apps/encryption/lib/Command/DropLegacyFileKey.php +++ b/apps/encryption/lib/Command/DropLegacyFileKey.php @@ -11,9 +11,11 @@ namespace OCA\Encryption\Command; use OC\Encryption\Exceptions\DecryptionFailedException; use OC\Files\FileInfo; +use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\KeyManager; use OCP\Encryption\Exceptions\GenericEncryptionException; +use OCP\IUser; use OCP\IUserManager; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -23,8 +25,9 @@ class DropLegacyFileKey extends Command { private View $rootView; public function __construct( - private IUserManager $userManager, - private KeyManager $keyManager, + private readonly IUserManager $userManager, + private readonly KeyManager $keyManager, + private readonly SetupManager $setupManager, ) { parent::__construct(); @@ -42,18 +45,10 @@ class DropLegacyFileKey extends Command { $output->writeln('Scanning all files for legacy filekey'); - foreach ($this->userManager->getBackends() as $backend) { - $limit = 500; - $offset = 0; - do { - $users = $backend->getUsers('', $limit, $offset); - foreach ($users as $user) { - $output->writeln('Scanning all files for ' . $user); - $this->setupUserFS($user); - $result = $result && $this->scanFolder($output, '/' . $user); - } - $offset += $limit; - } while (count($users) >= $limit); + foreach ($this->userManager->getSeenUsers() as $user) { + $output->writeln('Scanning all files for ' . $user->getUID()); + $this->setupUserFileSystem($user); + $result = $result && $this->scanFolder($output, '/' . $user->getUID()); } if ($result) { @@ -143,8 +138,8 @@ class DropLegacyFileKey extends Command { /** * setup user file system */ - protected function setupUserFS(string $uid): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); + protected function setupUserFileSystem(IUser $user): void { + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); } } diff --git a/apps/encryption/lib/Command/EnableMasterKey.php b/apps/encryption/lib/Command/EnableMasterKey.php index 0d8b893e0e2..96b4eb218a4 100644 --- a/apps/encryption/lib/Command/EnableMasterKey.php +++ b/apps/encryption/lib/Command/EnableMasterKey.php @@ -8,7 +8,7 @@ namespace OCA\Encryption\Command; use OCA\Encryption\Util; -use OCP\IConfig; +use OCP\AppFramework\Services\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputInterface; @@ -18,7 +18,7 @@ use Symfony\Component\Console\Question\ConfirmationQuestion; class EnableMasterKey extends Command { public function __construct( protected Util $util, - protected IConfig $config, + protected IAppConfig $config, protected QuestionHelper $questionHelper, ) { parent::__construct(); @@ -43,7 +43,7 @@ class EnableMasterKey extends Command { . 'There is also no way to disable it again. Do you want to continue? (y/n) ', false); if ($this->questionHelper->ask($input, $output, $question)) { - $this->config->setAppValue('encryption', 'useMasterKey', '1'); + $this->config->setAppValueBool('useMasterKey', true); $output->writeln('Master key successfully enabled.'); return self::SUCCESS; } diff --git a/apps/encryption/lib/Command/FixEncryptedVersion.php b/apps/encryption/lib/Command/FixEncryptedVersion.php index 462e3a5cc2a..fd5f670a9c2 100644 --- a/apps/encryption/lib/Command/FixEncryptedVersion.php +++ b/apps/encryption/lib/Command/FixEncryptedVersion.php @@ -8,12 +8,12 @@ namespace OCA\Encryption\Command; +use OC\Files\SetupManager; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; use OC\ServerNotAvailableException; use OCA\Encryption\Util; use OCP\Encryption\Exceptions\InvalidHeaderException; -use OCP\Files\IRootFolder; use OCP\HintException; use OCP\IConfig; use OCP\IUser; @@ -29,12 +29,12 @@ class FixEncryptedVersion extends Command { private bool $supportLegacy = false; public function __construct( - private IConfig $config, - private LoggerInterface $logger, - private IRootFolder $rootFolder, - private IUserManager $userManager, - private Util $util, - private View $view, + private readonly IConfig $config, + private readonly LoggerInterface $logger, + private readonly IUserManager $userManager, + private readonly Util $util, + private readonly View $view, + private readonly SetupManager $setupManager, ) { parent::__construct(); } @@ -91,45 +91,43 @@ class FixEncryptedVersion extends Command { return self::FAILURE; } - if ($this->userManager->get($user) === null) { + $user = $this->userManager->get($user); + if ($user === null) { $output->writeln("User id $user does not exist. Please provide a valid user id"); return self::FAILURE; } - return $this->runForUser($user, $pathOption, $output); + return $this->runForUser($user, $pathOption, $output) ? self::SUCCESS : self::FAILURE; } - $result = 0; - $this->userManager->callForSeenUsers(function (IUser $user) use ($pathOption, $output, &$result) { + foreach ($this->userManager->getSeenUsers() as $user) { $output->writeln('Processing files for ' . $user->getUID()); - $result = $this->runForUser($user->getUID(), $pathOption, $output); - return $result === 0; - }); - return $result; + if (!$this->runForUser($user, $pathOption, $output)) { + return self::FAILURE; + } + } + return self::SUCCESS; } - private function runForUser(string $user, string $pathOption, OutputInterface $output): int { - $pathToWalk = "/$user/files"; + private function runForUser(IUser $user, string $pathOption, OutputInterface $output): bool { + $pathToWalk = '/' . $user->getUID() . '/files'; if ($pathOption !== '') { $pathToWalk = "$pathToWalk/$pathOption"; } return $this->walkPathOfUser($user, $pathToWalk, $output); } - /** - * @return int 0 for success, 1 for error - */ - private function walkPathOfUser(string $user, string $path, OutputInterface $output): int { - $this->setupUserFs($user); + private function walkPathOfUser(IUser $user, string $path, OutputInterface $output): bool { + $this->setupUserFileSystem($user); if (!$this->view->file_exists($path)) { $output->writeln("Path \"$path\" does not exist. Please provide a valid path."); - return self::FAILURE; + return false; } if ($this->view->is_file($path)) { $output->writeln("Verifying the content of file \"$path\""); $this->verifyFileContent($path, $output); - return self::SUCCESS; + return true; } $directories = []; $directories[] = $path; @@ -145,7 +143,7 @@ class FixEncryptedVersion extends Command { } } } - return self::SUCCESS; + return true; } /** @@ -309,8 +307,8 @@ class FixEncryptedVersion extends Command { /** * Setup user file system */ - private function setupUserFs(string $uid): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); + private function setupUserFileSystem(IUser $user): void { + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); } } diff --git a/apps/encryption/lib/Command/FixKeyLocation.php b/apps/encryption/lib/Command/FixKeyLocation.php index da529a4be2f..1ad48f5b5e8 100644 --- a/apps/encryption/lib/Command/FixKeyLocation.php +++ b/apps/encryption/lib/Command/FixKeyLocation.php @@ -10,6 +10,7 @@ namespace OCA\Encryption\Command; use OC\Encryption\Manager; use OC\Encryption\Util; +use OC\Files\SetupManager; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; use OCP\Encryption\IManager; @@ -33,10 +34,11 @@ class FixKeyLocation extends Command { private Manager $encryptionManager; public function __construct( - private IUserManager $userManager, - private IUserMountCache $userMountCache, - private Util $encryptionUtil, - private IRootFolder $rootFolder, + private readonly IUserManager $userManager, + private readonly IUserMountCache $userMountCache, + private readonly Util $encryptionUtil, + private readonly IRootFolder $rootFolder, + private readonly SetupManager $setupManager, IManager $encryptionManager, ) { $this->keyRootDirectory = rtrim($this->encryptionUtil->getKeyStorageRoot(), '/'); @@ -69,7 +71,7 @@ class FixKeyLocation extends Command { return self::FAILURE; } - \OC_Util::setupFS($user->getUID()); + $this->setupManager->setupForUser($user); $mounts = $this->getSystemMountsForUser($user); foreach ($mounts as $mount) { @@ -179,11 +181,12 @@ class FixKeyLocation extends Command { * * @return \Generator */ - private function getAllEncryptedFiles(Folder $folder) { + private function getAllEncryptedFiles(Folder $folder): \Generator { foreach ($folder->getDirectoryListing() as $child) { if ($child instanceof Folder) { yield from $this->getAllEncryptedFiles($child); } else { + /** @var File $child */ if (substr($child->getName(), -4) !== '.bak' && $child->isEncrypted()) { yield $child; } diff --git a/apps/encryption/lib/Command/ScanLegacyFormat.php b/apps/encryption/lib/Command/ScanLegacyFormat.php index 1e46a3d7545..56601354d0a 100644 --- a/apps/encryption/lib/Command/ScanLegacyFormat.php +++ b/apps/encryption/lib/Command/ScanLegacyFormat.php @@ -8,9 +8,11 @@ declare(strict_types=1); */ namespace OCA\Encryption\Command; +use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\Util; use OCP\IConfig; +use OCP\IUser; use OCP\IUserManager; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; @@ -21,10 +23,11 @@ class ScanLegacyFormat extends Command { private View $rootView; public function __construct( - protected Util $util, - protected IConfig $config, - protected QuestionHelper $questionHelper, - private IUserManager $userManager, + protected readonly Util $util, + protected readonly IConfig $config, + protected readonly QuestionHelper $questionHelper, + private readonly IUserManager $userManager, + private readonly SetupManager $setupManager, ) { parent::__construct(); @@ -42,18 +45,10 @@ class ScanLegacyFormat extends Command { $output->writeln('Scanning all files for legacy encryption'); - foreach ($this->userManager->getBackends() as $backend) { - $limit = 500; - $offset = 0; - do { - $users = $backend->getUsers('', $limit, $offset); - foreach ($users as $user) { - $output->writeln('Scanning all files for ' . $user); - $this->setupUserFS($user); - $result = $result && $this->scanFolder($output, '/' . $user); - } - $offset += $limit; - } while (count($users) >= $limit); + foreach ($this->userManager->getSeenUsers() as $user) { + $output->writeln('Scanning all files for ' . $user->getUID()); + $this->setupUserFileSystem($user); + $result = $result && $this->scanFolder($output, '/' . $user->getUID()); } if ($result) { @@ -93,8 +88,8 @@ class ScanLegacyFormat extends Command { /** * setup user file system */ - protected function setupUserFS(string $uid): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); + protected function setupUserFileSystem(IUser $user): void { + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); } } diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php index db6135787ef..3751111d988 100644 --- a/apps/encryption/lib/Crypto/EncryptAll.php +++ b/apps/encryption/lib/Crypto/EncryptAll.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace OCA\Encryption\Crypto; use OC\Encryption\Exceptions\DecryptionFailedException; +use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\KeyManager; use OCA\Encryption\Users\Setup; @@ -34,31 +35,26 @@ use Symfony\Component\Console\Question\ConfirmationQuestion; class EncryptAll { - /** @var array */ - protected $userPasswords; - - /** @var OutputInterface */ - protected $output; - - /** @var InputInterface */ - protected $input; + /** @var array $userCache store one time passwords for the users */ + protected array $userCache = []; + protected OutputInterface $output; + protected InputInterface $input; public function __construct( - protected Setup $userSetup, - protected IUserManager $userManager, - protected View $rootView, - protected KeyManager $keyManager, - protected Util $util, - protected IConfig $config, - protected IMailer $mailer, - protected IL10N $l, - protected IFactory $l10nFactory, - protected QuestionHelper $questionHelper, - protected ISecureRandom $secureRandom, - protected LoggerInterface $logger, + protected readonly Setup $userSetup, + protected readonly IUserManager $userManager, + protected readonly View $rootView, + protected readonly KeyManager $keyManager, + protected readonly Util $util, + protected readonly IConfig $config, + protected readonly IMailer $mailer, + protected readonly IL10N $l, + protected readonly IFactory $l10nFactory, + protected readonly QuestionHelper $questionHelper, + protected readonly ISecureRandom $secureRandom, + protected readonly LoggerInterface $logger, + protected readonly SetupManager $setupManager, ) { - // store one time passwords for the users - $this->userPasswords = []; } /** @@ -117,26 +113,18 @@ class EncryptAll { $progress->setFormat(" %message% \n [%bar%]"); $progress->start(); - foreach ($this->userManager->getBackends() as $backend) { - $limit = 500; - $offset = 0; - do { - $users = $backend->getUsers('', $limit, $offset); - foreach ($users as $user) { - if ($this->keyManager->userHasKeys($user) === false) { - $progress->setMessage('Create key-pair for ' . $user); - $progress->advance(); - $this->setupUserFS($user); - $password = $this->generateOneTimePassword($user); - $this->userSetup->setupUser($user, $password); - } else { - // users which already have a key-pair will be stored with a - // empty password and filtered out later - $this->userPasswords[$user] = ''; - } - } - $offset += $limit; - } while (count($users) >= $limit); + foreach ($this->userManager->getSeenUsers() as $user) { + if ($this->keyManager->userHasKeys($user->getUID()) === false) { + $progress->setMessage('Create key-pair for ' . $user->getUID()); + $progress->advance(); + $this->setupUserFileSystem($user); + $password = $this->generateOneTimePassword($user); + $this->userSetup->setupUser($user->getUID(), $password); + } else { + // users which already have a key-pair will be stored with a + // empty password and filtered out later + $this->userCache[$user->getUID()] = ['password' => '', 'user' => $user]; + } } $progress->setMessage('Key-pair created for all users'); @@ -151,14 +139,15 @@ class EncryptAll { $progress = new ProgressBar($this->output); $progress->setFormat(" %message% \n [%bar%]"); $progress->start(); - $numberOfUsers = count($this->userPasswords); + $numberOfUsers = count($this->userCache); $userNo = 1; if ($this->util->isMasterKeyEnabled()) { $this->encryptAllUserFilesWithMasterKey($progress); } else { - foreach ($this->userPasswords as $uid => $password) { + foreach ($this->userCache as $uid => $cache) { + ['user' => $user, 'password' => $password] = $cache; $userCount = "$uid ($userNo of $numberOfUsers)"; - $this->encryptUsersFiles($uid, $progress, $userCount); + $this->encryptUsersFiles($user, $progress, $userCount); $userNo++; } } @@ -171,26 +160,19 @@ class EncryptAll { */ protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress): void { $userNo = 1; - foreach ($this->userManager->getBackends() as $backend) { - $limit = 500; - $offset = 0; - do { - $users = $backend->getUsers('', $limit, $offset); - foreach ($users as $user) { - $userCount = "$user ($userNo)"; - $this->encryptUsersFiles($user, $progress, $userCount); - $userNo++; - } - $offset += $limit; - } while (count($users) >= $limit); + foreach ($this->userManager->getSeenUsers() as $user) { + $userCount = $user->getUID() . " ($userNo)"; + $this->encryptUsersFiles($user, $progress, $userCount); + $userNo++; } } /** * encrypt files from the given user */ - protected function encryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void { - $this->setupUserFS($uid); + protected function encryptUsersFiles(IUser $user, ProgressBar $progress, string $userCount): void { + $this->setupUserFileSystem($user); + $uid = $user->getUID(); $directories = []; $directories[] = '/' . $uid . '/files'; @@ -269,7 +251,8 @@ class EncryptAll { //create rows $newPasswords = []; $unchangedPasswords = []; - foreach ($this->userPasswords as $uid => $password) { + foreach ($this->userCache as $uid => $cache) { + ['user' => $user, 'password' => $password] = $cache; if (empty($password)) { $unchangedPasswords[] = $uid; } else { @@ -323,9 +306,9 @@ class EncryptAll { /** * setup user file system */ - protected function setupUserFS(string $uid): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); + protected function setupUserFileSystem(IUser $user): void { + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); } /** @@ -333,9 +316,9 @@ class EncryptAll { * * @return string password */ - protected function generateOneTimePassword(string $uid): string { + protected function generateOneTimePassword(IUser $user): string { $password = $this->secureRandom->generate(16, ISecureRandom::CHAR_HUMAN_READABLE); - $this->userPasswords[$uid] = $password; + $this->userCache[$user->getUID()] = ['password' => $password, 'user' => $user]; return $password; } @@ -346,10 +329,11 @@ class EncryptAll { $noMail = []; $this->output->writeln(''); - $progress = new ProgressBar($this->output, count($this->userPasswords)); + $progress = new ProgressBar($this->output, count($this->userCache)); $progress->start(); - foreach ($this->userPasswords as $uid => $password) { + foreach ($this->userCache as $uid => $cache) { + ['user' => $user, 'password' => $password] = $cache; $progress->advance(); if (!empty($password)) { $recipient = $this->userManager->get($uid); @@ -409,7 +393,7 @@ class EncryptAll { $this->output->writeln("\n\nCould not send password to following users:\n"); $rows = []; foreach ($noMail as $uid) { - $rows[] = [$uid, $this->userPasswords[$uid]]; + $rows[] = [$uid, $this->userCache[$uid]['password']]; } $table->setRows($rows); $table->render(); diff --git a/apps/encryption/tests/Command/FixEncryptedVersionTest.php b/apps/encryption/tests/Command/FixEncryptedVersionTest.php index d0af359183b..6376e7293ab 100644 --- a/apps/encryption/tests/Command/FixEncryptedVersionTest.php +++ b/apps/encryption/tests/Command/FixEncryptedVersionTest.php @@ -10,10 +10,12 @@ declare(strict_types=1); namespace OCA\Encryption\Tests\Command; +use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\Command\FixEncryptedVersion; use OCA\Encryption\Util; -use OCP\Files\IRootFolder; +use OCP\Encryption\IManager; +use OCP\IAppConfig; use OCP\IConfig; use OCP\ITempManager; use OCP\IUserManager; @@ -48,7 +50,7 @@ class FixEncryptedVersionTest extends TestCase { public function setUp(): void { parent::setUp(); - Server::get(IConfig::class)->setAppValue('encryption', 'useMasterKey', '1'); + Server::get(IAppConfig::class)->setValueBool('encryption', 'useMasterKey', true); $this->util = $this->getMockBuilder(Util::class) ->disableOriginalConstructor()->getMock(); @@ -64,16 +66,17 @@ class FixEncryptedVersionTest extends TestCase { $this->fixEncryptedVersion = new FixEncryptedVersion( Server::get(IConfig::class), Server::get(LoggerInterface::class), - Server::get(IRootFolder::class), Server::get(IUserManager::class), $this->util, - new View('/') + new View('/'), + Server::get(SetupManager::class), ); $this->commandTester = new CommandTester($this->fixEncryptedVersion); - $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isEnabled()); - $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isReady()); - $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isReadyForUser($this->userId)); + $manager = Server::get(IManager::class); + $this->assertTrue($manager->isEnabled()); + $this->assertTrue($manager->isReady()); + $this->assertTrue($manager->isReadyForUser($this->userId)); } /** @@ -375,7 +378,7 @@ The file \"/$this->userId/files/sub/hello.txt\" is: OK", $output); * Test commands without master key */ public function testExecuteWithNoMasterKey(): void { - Server::get(IConfig::class)->setAppValue('encryption', 'useMasterKey', '0'); + Server::get(IAppConfig::class)->setValueBool('encryption', 'useMasterKey', false); $this->util->expects($this->once())->method('isMasterKeyEnabled') ->willReturn(false); diff --git a/apps/encryption/tests/Command/TestEnableMasterKey.php b/apps/encryption/tests/Command/TestEnableMasterKey.php index ead3dfd0195..fe1d635534b 100644 --- a/apps/encryption/tests/Command/TestEnableMasterKey.php +++ b/apps/encryption/tests/Command/TestEnableMasterKey.php @@ -11,38 +11,27 @@ namespace OCA\Encryption\Tests\Command; use OCA\Encryption\Command\EnableMasterKey; use OCA\Encryption\Util; -use OCP\IConfig; +use OCP\AppFramework\Services\IAppConfig; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class TestEnableMasterKey extends TestCase { - - /** @var EnableMasterKey */ - protected $enableMasterKey; - - /** @var Util | \PHPUnit\Framework\MockObject\MockObject */ - protected $util; - - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ - protected $config; - - /** @var \Symfony\Component\Console\Helper\QuestionHelper | \PHPUnit\Framework\MockObject\MockObject */ - protected $questionHelper; - - /** @var \Symfony\Component\Console\Output\OutputInterface | \PHPUnit\Framework\MockObject\MockObject */ - protected $output; - - /** @var \Symfony\Component\Console\Input\InputInterface | \PHPUnit\Framework\MockObject\MockObject */ - protected $input; + protected EnableMasterKey $enableMasterKey; + protected Util&MockObject $util; + protected IAppConfig&MockObject $config; + protected QuestionHelper&MockObject $questionHelper; + protected OutputInterface&MockObject $output; + protected InputInterface&MockObject $input; protected function setUp(): void { parent::setUp(); $this->util = $this->getMockBuilder(Util::class) ->disableOriginalConstructor()->getMock(); - $this->config = $this->getMockBuilder(IConfig::class) + $this->config = $this->getMockBuilder(IAppConfig::class) ->disableOriginalConstructor()->getMock(); $this->questionHelper = $this->getMockBuilder(QuestionHelper::class) ->disableOriginalConstructor()->getMock(); @@ -54,13 +43,8 @@ class TestEnableMasterKey extends TestCase { $this->enableMasterKey = new EnableMasterKey($this->util, $this->config, $this->questionHelper); } - /** - * - * @param bool $isAlreadyEnabled - * @param string $answer - */ #[\PHPUnit\Framework\Attributes\DataProvider('dataTestExecute')] - public function testExecute($isAlreadyEnabled, $answer): void { + public function testExecute(bool $isAlreadyEnabled, string $answer): void { $this->util->expects($this->once())->method('isMasterKeyEnabled') ->willReturn($isAlreadyEnabled); @@ -70,8 +54,8 @@ class TestEnableMasterKey extends TestCase { } else { if ($answer === 'y') { $this->questionHelper->expects($this->once())->method('ask')->willReturn(true); - $this->config->expects($this->once())->method('setAppValue') - ->with('encryption', 'useMasterKey', '1'); + $this->config->expects($this->once())->method('setAppValueBool') + ->with('useMasterKey', true); } else { $this->questionHelper->expects($this->once())->method('ask')->willReturn(false); $this->config->expects($this->never())->method('setAppValue'); diff --git a/apps/encryption/tests/Crypto/EncryptAllTest.php b/apps/encryption/tests/Crypto/EncryptAllTest.php index c56e3375a73..17af5dc52b4 100644 --- a/apps/encryption/tests/Crypto/EncryptAllTest.php +++ b/apps/encryption/tests/Crypto/EncryptAllTest.php @@ -9,6 +9,7 @@ declare(strict_types=1); */ namespace OCA\Encryption\Tests\Crypto; +use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\Crypto\EncryptAll; use OCA\Encryption\KeyManager; @@ -17,6 +18,7 @@ use OCA\Encryption\Util; use OCP\Files\FileInfo; use OCP\IConfig; use OCP\IL10N; +use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Mail\IMailer; @@ -48,6 +50,9 @@ class EncryptAllTest extends TestCase { protected UserInterface&MockObject $userInterface; protected ISecureRandom&MockObject $secureRandom; protected LoggerInterface&MockObject $logger; + protected SetupManager&MockObject $setupManager; + protected IUser&MockObject $user1; + protected IUser&MockObject $user2; protected EncryptAll $encryptAll; @@ -79,6 +84,7 @@ class EncryptAllTest extends TestCase { $this->userInterface = $this->getMockBuilder(UserInterface::class) ->disableOriginalConstructor()->getMock(); $this->logger = $this->createMock(LoggerInterface::class); + $this->setupManager = $this->createMock(SetupManager::class); /** * We need format method to return a string @@ -91,9 +97,16 @@ class EncryptAllTest extends TestCase { $this->outputInterface->expects($this->any())->method('getFormatter') ->willReturn($outputFormatter); - $this->userManager->expects($this->any())->method('getBackends')->willReturn([$this->userInterface]); - $this->userInterface->expects($this->any())->method('getUsers')->willReturn(['user1', 'user2']); + $this->user1 = $this->createMock(IUser::class); + $this->user1->method('getUID')->willReturn('user1'); + $this->user2 = $this->createMock(IUser::class); + $this->user2->method('getUID')->willReturn('user2'); + + $this->userManager->expects($this->any())->method('getSeenUsers')->will($this->returnCallback(function () { + yield $this->user1; + yield $this->user2; + })); $this->secureRandom = $this->getMockBuilder(ISecureRandom::class)->disableOriginalConstructor()->getMock(); $this->secureRandom->expects($this->any())->method('generate')->willReturn('12345678'); @@ -111,6 +124,7 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ); } @@ -138,6 +152,7 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ] ) ->onlyMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) @@ -168,6 +183,7 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ] ) ->onlyMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords']) @@ -199,9 +215,10 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ] ) - ->onlyMethods(['setupUserFS', 'generateOneTimePassword']) + ->onlyMethods(['generateOneTimePassword', 'setupUserFileSystem']) ->getMock(); @@ -218,19 +235,19 @@ class EncryptAllTest extends TestCase { } ); - $encryptAll->expects($this->once())->method('setupUserFS')->with('user1'); - $encryptAll->expects($this->once())->method('generateOneTimePassword')->with('user1')->willReturn('password'); + $encryptAll->expects($this->once())->method('setupUserFileSystem')->with($this->user1); + $encryptAll->expects($this->once())->method('generateOneTimePassword')->with($this->user1)->willReturn('password'); $this->setupUser->expects($this->once())->method('setupUser')->with('user1', 'password'); $this->invokePrivate($encryptAll, 'createKeyPairs'); - $userPasswords = $this->invokePrivate($encryptAll, 'userPasswords'); + $userPasswords = $this->invokePrivate($encryptAll, 'userCache'); // we only expect the skipped user, because generateOneTimePassword which // would set the user with the new password was mocked. // This method will be tested separately $this->assertSame(1, count($userPasswords)); - $this->assertSame('', $userPasswords['user2']); + $this->assertSame('', $userPasswords['user2']['password']); } public function testEncryptAllUsersFiles(): void { @@ -250,6 +267,7 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ] ) ->onlyMethods(['encryptUsersFiles']) @@ -259,7 +277,16 @@ class EncryptAllTest extends TestCase { // set protected property $output $this->invokePrivate($encryptAll, 'output', [$this->outputInterface]); - $this->invokePrivate($encryptAll, 'userPasswords', [['user1' => 'pwd1', 'user2' => 'pwd2']]); + $this->invokePrivate($encryptAll, 'userCache', [[ + 'user1' => [ + 'password' => 'pwd1', + 'user' => $this->user1, + ], + 'user2' => [ + 'password' => 'pwd2', + 'user' => $this->user2, + ] + ]]); $encryptAllCalls = []; $encryptAll->expects($this->exactly(2)) @@ -270,8 +297,8 @@ class EncryptAllTest extends TestCase { $this->invokePrivate($encryptAll, 'encryptAllUsersFiles'); self::assertEquals([ - 'user1', - 'user2', + $this->user1, + $this->user2, ], $encryptAllCalls); } @@ -292,9 +319,10 @@ class EncryptAllTest extends TestCase { $this->questionHelper, $this->secureRandom, $this->logger, + $this->setupManager, ] ) - ->onlyMethods(['encryptFile', 'setupUserFS']) + ->onlyMethods(['encryptFile']) ->getMock(); $this->util->expects($this->any())->method('isMasterKeyEnabled')->willReturn(false); @@ -335,7 +363,7 @@ class EncryptAllTest extends TestCase { ->willReturn($outputFormatter); $progressBar = new ProgressBar($this->outputInterface); - $this->invokePrivate($encryptAll, 'encryptUsersFiles', ['user1', $progressBar, '']); + $this->invokePrivate($encryptAll, 'encryptUsersFiles', [$this->user1, $progressBar, '']); self::assertEquals([ '/user1/files/bar', '/user1/files/foo/subfile', @@ -343,13 +371,13 @@ class EncryptAllTest extends TestCase { } public function testGenerateOneTimePassword(): void { - $password = $this->invokePrivate($this->encryptAll, 'generateOneTimePassword', ['user1']); + $password = $this->invokePrivate($this->encryptAll, 'generateOneTimePassword', [$this->user1]); $this->assertTrue(is_string($password)); $this->assertSame(8, strlen($password)); - $userPasswords = $this->invokePrivate($this->encryptAll, 'userPasswords'); + $userPasswords = $this->invokePrivate($this->encryptAll, 'userCache'); $this->assertSame(1, count($userPasswords)); - $this->assertSame($password, $userPasswords['user1']); + $this->assertSame($password, $userPasswords['user1']['password']); } /** diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 92479b887aa..e76797a0969 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1015,51 +1015,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - getUID())]]> - - - ]]> - - - - - - - - - - - @@ -1068,15 +1023,6 @@ - - - - - - - - -