diff --git a/apps/encryption/lib/Controller/RecoveryController.php b/apps/encryption/lib/Controller/RecoveryController.php index cc172d18b3035..9053a54423439 100644 --- a/apps/encryption/lib/Controller/RecoveryController.php +++ b/apps/encryption/lib/Controller/RecoveryController.php @@ -124,7 +124,7 @@ public function changeRecoveryPassword(string $newPassword, string $oldPassword, #[NoAdminRequired] public function userSetRecovery($userEnableRecovery) { if ($userEnableRecovery === '0' || $userEnableRecovery === '1') { - $result = $this->recovery->setRecoveryForUser($userEnableRecovery); + $result = $this->recovery->setRecoveryForUser($userEnableRecovery === '1'); if ($result) { if ($userEnableRecovery === '0') { diff --git a/apps/encryption/lib/Recovery.php b/apps/encryption/lib/Recovery.php index 38d0c48ad8935..bfccc1770823f 100644 --- a/apps/encryption/lib/Recovery.php +++ b/apps/encryption/lib/Recovery.php @@ -9,8 +9,9 @@ use OC\Files\View; use OCA\Encryption\Crypto\Crypt; +use OCP\Config\IUserConfig; use OCP\Encryption\IFile; -use OCP\IConfig; +use OCP\IAppConfig; use OCP\IUser; use OCP\IUserSession; use OCP\PreConditionNotMetException; @@ -21,19 +22,12 @@ class Recovery { */ protected $user; - /** - * @param IUserSession $userSession - * @param Crypt $crypt - * @param KeyManager $keyManager - * @param IConfig $config - * @param IFile $file - * @param View $view - */ public function __construct( IUserSession $userSession, protected Crypt $crypt, private KeyManager $keyManager, - private IConfig $config, + private IAppConfig $appConfig, + private IUserConfig $userConfig, private IFile $file, private View $view, ) { @@ -45,10 +39,7 @@ public function __construct( * @return bool */ public function enableAdminRecovery($password) { - $appConfig = $this->config; - $keyManager = $this->keyManager; - - if (!$keyManager->recoveryKeyExists()) { + if (!$this->keyManager->recoveryKeyExists()) { $keyPair = $this->crypt->createKeyPair(); if (!is_array($keyPair)) { return false; @@ -57,8 +48,8 @@ public function enableAdminRecovery($password) { $this->keyManager->setRecoveryKey($password, $keyPair); } - if ($keyManager->checkRecoveryPassword($password)) { - $appConfig->setAppValue('encryption', 'recoveryAdminEnabled', '1'); + if ($this->keyManager->checkRecoveryPassword($password)) { + $this->appConfig->setValueBool('encryption', 'recoveryAdminEnabled', true); return true; } @@ -92,7 +83,7 @@ public function disableAdminRecovery($recoveryPassword) { if ($keyManager->checkRecoveryPassword($recoveryPassword)) { // Set recoveryAdmin as disabled - $this->config->setAppValue('encryption', 'recoveryAdminEnabled', '0'); + $this->appConfig->setValueBool('encryption', 'recoveryAdminEnabled', false); return true; } return false; @@ -107,12 +98,7 @@ public function disableAdminRecovery($recoveryPassword) { */ public function isRecoveryEnabledForUser($user = '') { $uid = $user === '' ? $this->user->getUID() : $user; - $recoveryMode = $this->config->getUserValue($uid, - 'encryption', - 'recoveryEnabled', - 0); - - return ($recoveryMode === '1'); + return $this->userConfig->getValueBool($uid, 'encryption', 'recoveryEnabled'); } /** @@ -121,23 +107,18 @@ public function isRecoveryEnabledForUser($user = '') { * @return bool */ public function isRecoveryKeyEnabled() { - $enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', '0'); - - return ($enabled === '1'); + return $this->appConfig->getValueBool('encryption', 'recoveryAdminEnabled'); } /** - * @param string $value + * @param bool $value * @return bool */ - public function setRecoveryForUser($value) { + public function setRecoveryForUser(bool $value): bool { try { - $this->config->setUserValue($this->user->getUID(), - 'encryption', - 'recoveryEnabled', - $value); + $this->userConfig->setValueBool($this->user->getUID(), 'encryption', 'recoveryEnabled', $value); - if ($value === '1') { + if ($value) { $this->addRecoveryKeys('/' . $this->user->getUID() . '/files/'); } else { $this->removeRecoveryKeys('/' . $this->user->getUID() . '/files/'); diff --git a/apps/encryption/lib/Settings/Admin.php b/apps/encryption/lib/Settings/Admin.php index 422d338d45e03..8035506a7ac0f 100644 --- a/apps/encryption/lib/Settings/Admin.php +++ b/apps/encryption/lib/Settings/Admin.php @@ -15,6 +15,7 @@ use OCA\Encryption\Util; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; +use OCP\Config\IUserConfig; use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; @@ -34,6 +35,7 @@ public function __construct( private ISession $session, private IInitialState $initialState, private IAppConfig $appConfig, + private IUserConfig $userConfig, ) { } @@ -52,7 +54,8 @@ public function getForm() { new View(), $crypt, $this->userSession, - $this->config, + $this->appConfig, + $this->userConfig, $this->userManager); // Check if an adminRecovery account is enabled for recovering files after lost pwd diff --git a/apps/encryption/lib/Util.php b/apps/encryption/lib/Util.php index ccbdcdcb242b1..255753fe0a188 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -10,8 +10,9 @@ use OC\Files\Storage\Storage; use OC\Files\View; use OCA\Encryption\Crypto\Crypt; +use OCP\Config\IUserConfig; use OCP\Files\Storage\IStorage; -use OCP\IConfig; +use OCP\IAppConfig; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -24,7 +25,8 @@ public function __construct( private View $files, private Crypt $crypt, IUserSession $userSession, - private IConfig $config, + private IAppConfig $appConfig, + private IUserConfig $userConfig, private IUserManager $userManager, ) { $this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false; @@ -37,12 +39,7 @@ public function __construct( * @return bool */ public function isRecoveryEnabledForUser($uid) { - $recoveryMode = $this->config->getUserValue($uid, - 'encryption', - 'recoveryEnabled', - '0'); - - return ($recoveryMode === '1'); + return $this->userConfig->getValueBool($uid, 'encryption', 'recoveryEnabled'); } /** @@ -51,13 +48,7 @@ public function isRecoveryEnabledForUser($uid) { * @return bool */ public function shouldEncryptHomeStorage() { - $encryptHomeStorage = $this->config->getAppValue( - 'encryption', - 'encryptHomeStorage', - '1' - ); - - return ($encryptHomeStorage === '1'); + return $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true); } /** @@ -65,35 +56,24 @@ public function shouldEncryptHomeStorage() { * * @param bool $encryptHomeStorage */ - public function setEncryptHomeStorage($encryptHomeStorage) { - $value = $encryptHomeStorage ? '1' : '0'; - $this->config->setAppValue( - 'encryption', - 'encryptHomeStorage', - $value - ); + public function setEncryptHomeStorage(bool $encryptHomeStorage) { + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', $encryptHomeStorage); } /** * check if master key is enabled */ public function isMasterKeyEnabled(): bool { - $userMasterKey = $this->config->getAppValue('encryption', 'useMasterKey', '1'); - return ($userMasterKey === '1'); + return $this->appConfig->getValueBool('encryption', 'useMasterKey', true); } /** * @param $enabled * @return bool */ - public function setRecoveryForUser($enabled) { - $value = $enabled ? '1' : '0'; - + public function setRecoveryForUser(bool $enabled): bool { try { - $this->config->setUserValue($this->user->getUID(), - 'encryption', - 'recoveryEnabled', - $value); + $this->userConfig->setValueBool($this->user->getUID(), 'encryption', 'recoveryEnabled', $enabled); return true; } catch (PreConditionNotMetException $e) { return false; diff --git a/apps/encryption/tests/Controller/RecoveryControllerTest.php b/apps/encryption/tests/Controller/RecoveryControllerTest.php index c1a3fdf9a92d2..a0464e9505767 100644 --- a/apps/encryption/tests/Controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/Controller/RecoveryControllerTest.php @@ -114,10 +114,10 @@ public static function userSetRecoveryProvider(): array { public function testUserSetRecovery($enableRecovery, $expectedMessage, $expectedStatus): void { $this->recoveryMock->expects($this->any()) ->method('setRecoveryForUser') - ->with($enableRecovery) + ->with($enableRecovery === '1') ->willReturnMap([ - ['1', true], - ['0', false] + [true, true], + [false, false] ]); diff --git a/apps/encryption/tests/RecoveryTest.php b/apps/encryption/tests/RecoveryTest.php index 74d472adeec52..67bae025088d0 100644 --- a/apps/encryption/tests/RecoveryTest.php +++ b/apps/encryption/tests/RecoveryTest.php @@ -14,8 +14,9 @@ use OCA\Encryption\Crypto\Crypt; use OCA\Encryption\KeyManager; use OCA\Encryption\Recovery; +use OCP\Config\IUserConfig; use OCP\Encryption\IFile; -use OCP\IConfig; +use OCP\IAppConfig; use OCP\IUser; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; @@ -29,7 +30,8 @@ class RecoveryTest extends TestCase { private IUserSession&MockObject $userSessionMock; private IUser&MockObject $user; private KeyManager&MockObject $keyManagerMock; - private IConfig&MockObject $configMock; + private IAppConfig&MockObject $appConfigMock; + private IUserConfig&MockObject $userConfigMock; private Crypt&MockObject $cryptMock; private Recovery $instance; @@ -56,7 +58,7 @@ public function testEnableAdminRecoverySuccessful(): void { $this->assertTrue($this->instance->enableAdminRecovery('password')); $this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage); - $this->assertEquals(1, self::$tempStorage['recoveryAdminEnabled']); + $this->assertTrue(self::$tempStorage['recoveryAdminEnabled']); $this->assertTrue($this->instance->enableAdminRecovery('password')); } @@ -83,7 +85,7 @@ public function testEnableAdminRecoveryCouldNotCheckPassword(): void { $this->assertTrue($this->instance->enableAdminRecovery('password')); $this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage); - $this->assertEquals(1, self::$tempStorage['recoveryAdminEnabled']); + $this->assertTrue(self::$tempStorage['recoveryAdminEnabled']); $this->assertFalse($this->instance->enableAdminRecovery('password')); } @@ -140,15 +142,15 @@ public function testDisableAdminRecovery(): void { $this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage); $this->assertTrue($this->instance->disableAdminRecovery('password')); - $this->assertEquals(0, self::$tempStorage['recoveryAdminEnabled']); + $this->assertFalse(self::$tempStorage['recoveryAdminEnabled']); $this->assertFalse($this->instance->disableAdminRecovery('password')); } public function testIsRecoveryEnabledForUser(): void { - $this->configMock->expects($this->exactly(2)) - ->method('getUserValue') - ->willReturnOnConsecutiveCalls('1', '0'); + $this->userConfigMock->expects($this->exactly(2)) + ->method('getValueBool') + ->willReturnOnConsecutiveCalls(true, false); $this->assertTrue($this->instance->isRecoveryEnabledForUser()); $this->assertFalse($this->instance->isRecoveryEnabledForUser('admin')); @@ -156,7 +158,7 @@ public function testIsRecoveryEnabledForUser(): void { public function testIsRecoveryKeyEnabled(): void { $this->assertFalse($this->instance->isRecoveryKeyEnabled()); - self::$tempStorage['recoveryAdminEnabled'] = '1'; + self::$tempStorage['recoveryAdminEnabled'] = true; $this->assertTrue($this->instance->isRecoveryKeyEnabled()); } @@ -164,8 +166,8 @@ public function testSetRecoveryFolderForUser(): void { $this->viewMock->expects($this->exactly(2)) ->method('getDirectoryContent') ->willReturn([]); - $this->assertTrue($this->instance->setRecoveryForUser(0)); - $this->assertTrue($this->instance->setRecoveryForUser('1')); + $this->assertTrue($this->instance->setRecoveryForUser(false)); + $this->assertTrue($this->instance->setRecoveryForUser(true)); } public function testRecoverUserFiles(): void { @@ -239,52 +241,41 @@ protected function setUp(): void { $this->cryptMock = $this->getMockBuilder(Crypt::class)->disableOriginalConstructor()->getMock(); $this->keyManagerMock = $this->getMockBuilder(KeyManager::class)->disableOriginalConstructor()->getMock(); - $this->configMock = $this->createMock(IConfig::class); + $this->appConfigMock = $this->createMock(IAppConfig::class); + $this->userConfigMock = $this->createMock(IUserConfig::class); $this->fileMock = $this->createMock(IFile::class); $this->viewMock = $this->createMock(View::class); - $this->configMock->expects($this->any()) - ->method('setAppValue') - ->willReturnCallback([$this, 'setValueTester']); + $this->appConfigMock->expects($this->any()) + ->method('setValueBool') + ->willReturnCallback(function (string $app, string $key, bool $value): bool { + self::$tempStorage[$key] = $value; + return true; + }); - $this->configMock->expects($this->any()) - ->method('getAppValue') + $this->appConfigMock->expects($this->any()) + ->method('getValueBool') ->willReturnCallback([$this, 'getValueTester']); $this->instance = new Recovery($this->userSessionMock, $this->cryptMock, $this->keyManagerMock, - $this->configMock, + $this->appConfigMock, + $this->userConfigMock, $this->fileMock, $this->viewMock); } - /** - * @param $app - * @param $key - * @param $value - */ - public function setValueTester($app, $key, $value) { + public function setValueTester(string $app, string $key, bool $value): void { self::$tempStorage[$key] = $value; } - /** - * @param $key - */ - public function removeValueTester($key) { + public function removeValueTester(string $key): void { unset(self::$tempStorage[$key]); } - /** - * @param $app - * @param $key - * @return mixed - */ - public function getValueTester($app, $key) { - if (!empty(self::$tempStorage[$key])) { - return self::$tempStorage[$key]; - } - return null; + public function getValueTester(string $app, string $key, bool $default = false): bool { + return self::$tempStorage[$key] ?? $default; } } diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php index 10d2a61c5279e..b5c865e0eebd4 100644 --- a/apps/encryption/tests/Settings/AdminTest.php +++ b/apps/encryption/tests/Settings/AdminTest.php @@ -11,6 +11,7 @@ use OCA\Encryption\Settings\Admin; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; +use OCP\Config\IUserConfig; use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; @@ -33,6 +34,7 @@ class AdminTest extends TestCase { protected ISession&MockObject $session; protected IInitialState&MockObject $initialState; protected IAppConfig&MockObject $appConfig; + protected IUserConfig&MockObject $userConfig; protected function setUp(): void { parent::setUp(); @@ -45,6 +47,7 @@ protected function setUp(): void { $this->session = $this->createMock(ISession::class); $this->initialState = $this->createMock(IInitialState::class); $this->appConfig = $this->createMock(IAppConfig::class); + $this->userConfig = $this->createMock(IUserConfig::class); $this->admin = new Admin( $this->l, @@ -55,6 +58,7 @@ protected function setUp(): void { $this->session, $this->initialState, $this->appConfig, + $this->userConfig, ); } @@ -62,19 +66,10 @@ public function testGetForm(): void { $this->appConfig ->method('getValueBool') ->willReturnMap([ - ['encryption', 'recoveryAdminEnabled', true] + ['encryption', 'recoveryAdminEnabled', true], + ['encryption', 'encryptHomeStorage', true, true], + ['encryption', 'useMasterKey', true, true], ]); - $this->config - ->method('getAppValue') - ->willReturnCallback(function ($app, $key, $default) { - if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') { - return '1'; - } - if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') { - return '1'; - } - return $default; - }); $this->initialState ->expects(self::once()) diff --git a/apps/encryption/tests/UtilTest.php b/apps/encryption/tests/UtilTest.php index ed274f74ab5ca..0a7ace6955d4a 100644 --- a/apps/encryption/tests/UtilTest.php +++ b/apps/encryption/tests/UtilTest.php @@ -12,9 +12,10 @@ use OC\Files\View; use OCA\Encryption\Crypto\Crypt; use OCA\Encryption\Util; +use OCP\Config\IUserConfig; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; -use OCP\IConfig; +use OCP\IAppConfig; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -26,13 +27,14 @@ class UtilTest extends TestCase { protected Util $instance; protected static $tempStorage = []; - protected IConfig&MockObject $configMock; + protected IAppConfig&MockObject $appConfigMock; + protected IUserConfig&MockObject $userConfigMock; protected View&MockObject $filesMock; protected IUserManager&MockObject $userManagerMock; protected IMountPoint&MockObject $mountMock; public function testSetRecoveryForUser(): void { - $this->instance->setRecoveryForUser('1'); + $this->instance->setRecoveryForUser(true); $this->assertArrayHasKey('recoveryEnabled', self::$tempStorage); } @@ -41,7 +43,7 @@ public function testIsRecoveryEnabledForUser(): void { // Assert recovery will return default value if not set unset(self::$tempStorage['recoveryEnabled']); - $this->assertEquals(0, $this->instance->isRecoveryEnabledForUser('admin')); + $this->assertFalse($this->instance->isRecoveryEnabledForUser('admin')); } public function testUserHasFiles(): void { @@ -77,41 +79,29 @@ protected function setUp(): void { ->method('isLoggedIn') ->willReturn(true); - $this->configMock = $this->createMock(IConfig::class); + $this->appConfigMock = $this->createMock(IAppConfig::class); + $this->userConfigMock = $this->createMock(IUserConfig::class); - $this->configMock->expects($this->any()) - ->method('getUserValue') + $this->userConfigMock->expects($this->any()) + ->method('getValueBool') ->willReturnCallback([$this, 'getValueTester']); - $this->configMock->expects($this->any()) - ->method('setUserValue') - ->willReturnCallback([$this, 'setValueTester']); + $this->userConfigMock->expects($this->any()) + ->method('setValueBool') + ->willReturnCallback(function (string $userId, string $app, string $key, bool $value): bool { + self::$tempStorage[$key] = $value; + return true; + }); - $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->userManagerMock); + $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->appConfigMock, $this->userConfigMock, $this->userManagerMock); } - /** - * @param $userId - * @param $app - * @param $key - * @param $value - */ - public function setValueTester($userId, $app, $key, $value) { + public function setValueTester(string $userId, string $app, string $key, bool $value): void { self::$tempStorage[$key] = $value; } - /** - * @param $userId - * @param $app - * @param $key - * @param $default - * @return mixed - */ - public function getValueTester($userId, $app, $key, $default) { - if (!empty(self::$tempStorage[$key])) { - return self::$tempStorage[$key]; - } - return $default ?: null; + public function getValueTester(string $userId, string $app, string $key, bool $default = false): bool { + return self::$tempStorage[$key] ?? $default; } /** @@ -120,9 +110,9 @@ public function getValueTester($userId, $app, $key, $default) { * @param bool $expect */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestIsMasterKeyEnabled')] - public function testIsMasterKeyEnabled($value, $expect): void { - $this->configMock->expects($this->once())->method('getAppValue') - ->with('encryption', 'useMasterKey', '1')->willReturn($value); + public function testIsMasterKeyEnabled(bool $value, bool $expect): void { + $this->appConfigMock->expects($this->once())->method('getValueBool') + ->with('encryption', 'useMasterKey', true)->willReturn($value); $this->assertSame($expect, $this->instance->isMasterKeyEnabled() ); @@ -130,19 +120,19 @@ public function testIsMasterKeyEnabled($value, $expect): void { public static function dataTestIsMasterKeyEnabled(): array { return [ - ['0', false], - ['1', true] + [false, false], + [true, true] ]; } /** - * @param string $returnValue return value from getAppValue() + * @param bool $returnValue return value from getValueBool() * @param bool $expected */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestShouldEncryptHomeStorage')] - public function testShouldEncryptHomeStorage($returnValue, $expected): void { - $this->configMock->expects($this->once())->method('getAppValue') - ->with('encryption', 'encryptHomeStorage', '1') + public function testShouldEncryptHomeStorage(bool $returnValue, bool $expected): void { + $this->appConfigMock->expects($this->once())->method('getValueBool') + ->with('encryption', 'encryptHomeStorage', true) ->willReturn($returnValue); $this->assertSame($expected, @@ -151,26 +141,26 @@ public function testShouldEncryptHomeStorage($returnValue, $expected): void { public static function dataTestShouldEncryptHomeStorage(): array { return [ - ['1', true], - ['0', false] + [true, true], + [false, false] ]; } /** - * @param $value - * @param $expected + * @param bool $value + * @param bool $expected */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestSetEncryptHomeStorage')] - public function testSetEncryptHomeStorage($value, $expected): void { - $this->configMock->expects($this->once())->method('setAppValue') + public function testSetEncryptHomeStorage(bool $value, bool $expected): void { + $this->appConfigMock->expects($this->once())->method('setValueBool') ->with('encryption', 'encryptHomeStorage', $expected); $this->instance->setEncryptHomeStorage($value); } public static function dataTestSetEncryptHomeStorage(): array { return [ - [true, '1'], - [false, '0'] + [true, true], + [false, false] ]; } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 3d97d80124ced..2375bd3c88105 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1286,13 +1286,6 @@ - - - - - - - @@ -1309,13 +1302,6 @@ - - - - - - - @@ -3027,19 +3013,6 @@ - - - - - - - - - - - - - diff --git a/core/Command/Encryption/Disable.php b/core/Command/Encryption/Disable.php index fe280daa111b4..a50960a4b0488 100644 --- a/core/Command/Encryption/Disable.php +++ b/core/Command/Encryption/Disable.php @@ -9,14 +9,14 @@ */ namespace OC\Core\Command\Encryption; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Disable extends Command { public function __construct( - protected IConfig $config, + protected IAppConfig $appConfig, ) { parent::__construct(); } @@ -31,10 +31,11 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + if (!$isEnabled) { $output->writeln('Encryption is already disabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'no'); + $this->appConfig->setValueBool('core', 'encryption_enabled', false); $output->writeln('Encryption disabled'); } return 0; diff --git a/core/Command/Encryption/Enable.php b/core/Command/Encryption/Enable.php index 02c610250011c..6ad91be947c49 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -10,14 +10,14 @@ namespace OC\Core\Command\Encryption; use OCP\Encryption\IManager; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Enable extends Command { public function __construct( - protected IConfig $config, + protected IAppConfig $appConfig, protected IManager $encryptionManager, ) { parent::__construct(); @@ -33,10 +33,11 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); + if ($isEnabled) { $output->writeln('Encryption is already enabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'yes'); + $this->appConfig->setValueBool('core', 'encryption_enabled', true); $output->writeln('Encryption enabled'); } $output->writeln(''); @@ -46,7 +47,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('No encryption module is loaded'); return 1; } - $defaultModule = $this->config->getAppValue('core', 'default_encryption_module'); + $defaultModule = $this->appConfig->getValueString('core', 'default_encryption_module'); if ($defaultModule === '') { $output->writeln('No default module is set'); return 1; diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index ac27f0911b8da..83d010dbe8752 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -20,6 +20,7 @@ use OCP\Files\Storage\IStorage; use OCP\IConfig; use OCP\IL10N; +use OCP\Server; use Psr\Log\LoggerInterface; class Manager implements IManager { @@ -48,8 +49,12 @@ public function isEnabled() { return false; } - $enabled = $this->config->getAppValue('core', 'encryption_enabled', 'no'); - return $enabled === 'yes'; + try { + return Server::get(\OCP\IAppConfig::class)->getValueBool('core', 'encryption_enabled'); + } catch (\Throwable) { + // DB not ready (e.g. oc_appconfig does not yet exist during install). + return false; + } } /** diff --git a/tests/Core/Command/Encryption/DecryptAllTest.php b/tests/Core/Command/Encryption/DecryptAllTest.php index 009eb36eb7fe2..74ccf0563bfbc 100644 --- a/tests/Core/Command/Encryption/DecryptAllTest.php +++ b/tests/Core/Command/Encryption/DecryptAllTest.php @@ -103,7 +103,7 @@ public function testExecute($encryptionEnabled, $continue): void { $this->appConfig->expects($this->once()) ->method('getValueBool') - ->with('core', 'encryption_enabled') + ->with('core', 'encryption_enabled', false) ->willReturn($encryptionEnabled); $this->consoleInput->expects($this->any()) @@ -168,7 +168,7 @@ public function testExecuteFailure(): void { ['core', 'encryption_enabled', true, false], ]; $this->appConfig->expects($this->exactly(2)) - ->method('setValuebool') + ->method('setValueBool') ->willReturnCallback(function () use (&$calls): bool { $expected = array_shift($calls); $this->assertEquals($expected, func_get_args()); @@ -176,7 +176,7 @@ public function testExecuteFailure(): void { }); $this->appConfig->expects($this->once()) ->method('getValueBool') - ->with('core', 'encryption_enabled') + ->with('core', 'encryption_enabled', false) ->willReturn(true); $this->consoleInput->expects($this->any()) diff --git a/tests/Core/Command/Encryption/DisableTest.php b/tests/Core/Command/Encryption/DisableTest.php index 96310a6c75ba3..b2ef86c2921f2 100644 --- a/tests/Core/Command/Encryption/DisableTest.php +++ b/tests/Core/Command/Encryption/DisableTest.php @@ -9,14 +9,14 @@ namespace Tests\Core\Command\Encryption; use OC\Core\Command\Encryption\Disable; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class DisableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ - protected $config; + protected $appConfig; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $consoleInput; /** @var \PHPUnit\Framework\MockObject\MockObject */ @@ -29,35 +29,35 @@ class DisableTest extends TestCase { protected function setUp(): void { parent::setUp(); - $config = $this->config = $this->getMockBuilder(IConfig::class) + $appConfig = $this->appConfig = $this->getMockBuilder(IAppConfig::class) ->disableOriginalConstructor() ->getMock(); $this->consoleInput = $this->getMockBuilder(InputInterface::class)->getMock(); $this->consoleOutput = $this->getMockBuilder(OutputInterface::class)->getMock(); - /** @var IConfig $config */ - $this->command = new Disable($config); + /** @var IAppConfig $appConfig */ + $this->command = new Disable($appConfig); } public static function dataDisable(): array { return [ - ['yes', true, 'Encryption disabled'], - ['no', false, 'Encryption is already disabled'], + [true, true, 'Encryption disabled'], + [false, false, 'Encryption is already disabled'], ]; } /** * - * @param string $oldStatus + * @param bool $oldStatus * @param bool $isUpdating * @param string $expectedString */ #[\PHPUnit\Framework\Attributes\DataProvider('dataDisable')] - public function testDisable($oldStatus, $isUpdating, $expectedString): void { - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('core', 'encryption_enabled', $this->anything()) + public function testDisable(bool $oldStatus, bool $isUpdating, string $expectedString): void { + $this->appConfig->expects($this->once()) + ->method('getValueBool') + ->with('core', 'encryption_enabled', false) ->willReturn($oldStatus); $this->consoleOutput->expects($this->once()) @@ -65,9 +65,9 @@ public function testDisable($oldStatus, $isUpdating, $expectedString): void { ->with($this->stringContains($expectedString)); if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') - ->with('core', 'encryption_enabled', 'no'); + $this->appConfig->expects($this->once()) + ->method('setValueBool') + ->with('core', 'encryption_enabled', false); } self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]); diff --git a/tests/Core/Command/Encryption/EnableTest.php b/tests/Core/Command/Encryption/EnableTest.php index 234984b1c0954..2acc1458e81aa 100644 --- a/tests/Core/Command/Encryption/EnableTest.php +++ b/tests/Core/Command/Encryption/EnableTest.php @@ -10,14 +10,14 @@ use OC\Core\Command\Encryption\Enable; use OCP\Encryption\IManager; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class EnableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ - protected $config; + protected $appConfig; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $manager; /** @var \PHPUnit\Framework\MockObject\MockObject */ @@ -32,7 +32,7 @@ class EnableTest extends TestCase { protected function setUp(): void { parent::setUp(); - $config = $this->config = $this->getMockBuilder(IConfig::class) + $appConfig = $this->appConfig = $this->getMockBuilder(IAppConfig::class) ->disableOriginalConstructor() ->getMock(); $manager = $this->manager = $this->getMockBuilder(IManager::class) @@ -41,47 +41,44 @@ protected function setUp(): void { $this->consoleInput = $this->getMockBuilder(InputInterface::class)->getMock(); $this->consoleOutput = $this->getMockBuilder(OutputInterface::class)->getMock(); - /** @var \OCP\IConfig $config */ + /** @var \OCP\IAppConfig $appConfig */ /** @var \OCP\Encryption\IManager $manager */ - $this->command = new Enable($config, $manager); + $this->command = new Enable($appConfig, $manager); } public static function dataEnable(): array { return [ - ['no', '', [], true, 'Encryption enabled', 'No encryption module is loaded'], - ['yes', '', [], false, 'Encryption is already enabled', 'No encryption module is loaded'], - ['no', '', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'No default module is set'], - ['no', 'OC_NO_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'The current default module does not exist: OC_NO_MODULE'], - ['no', 'OC_TEST_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'Default module: OC_TEST_MODULE'], + [false, '', [], true, 'Encryption enabled', 'No encryption module is loaded'], + [true, '', [], false, 'Encryption is already enabled', 'No encryption module is loaded'], + [false, '', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'No default module is set'], + [false, 'OC_NO_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'The current default module does not exist: OC_NO_MODULE'], + [false, 'OC_TEST_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'Default module: OC_TEST_MODULE'], ]; } #[\PHPUnit\Framework\Attributes\DataProvider('dataEnable')] - public function testEnable(string $oldStatus, ?string $defaultModule, array $availableModules, bool $isUpdating, string $expectedString, string $expectedDefaultModuleString): void { + public function testEnable(bool $oldStatus, ?string $defaultModule, array $availableModules, bool $isUpdating, string $expectedString, string $expectedDefaultModuleString): void { if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') - ->with('core', 'encryption_enabled', 'yes'); + $this->appConfig->expects($this->once()) + ->method('setValueBool') + ->with('core', 'encryption_enabled', true); } $this->manager->expects($this->atLeastOnce()) ->method('getEncryptionModules') ->willReturn($availableModules); - if (empty($availableModules)) { - $this->config->expects($this->once()) - ->method('getAppValue') - ->willReturnMap([ - ['core', 'encryption_enabled', 'no', $oldStatus], - ]); - } else { - $this->config->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['core', 'encryption_enabled', 'no', $oldStatus], - ['core', 'default_encryption_module', '', $defaultModule], - ]); + $this->appConfig->expects($this->once()) + ->method('getValueBool') + ->with('core', 'encryption_enabled', false) + ->willReturn($oldStatus); + + if (!empty($availableModules)) { + $this->appConfig->expects($this->once()) + ->method('getValueString') + ->with('core', 'default_encryption_module', '') + ->willReturn((string)$defaultModule); } $calls = [ diff --git a/tests/lib/Encryption/ManagerTest.php b/tests/lib/Encryption/ManagerTest.php index 5f3d1987dc3dc..b1ecaa02f490b 100644 --- a/tests/lib/Encryption/ManagerTest.php +++ b/tests/lib/Encryption/ManagerTest.php @@ -14,8 +14,10 @@ use OC\Files\View; use OC\Memcache\ArrayCache; use OCP\Encryption\IEncryptionModule; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; +use OCP\Server; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -73,10 +75,16 @@ public function testManagerIsDisabledIfDisabledButModules(): void { $this->assertFalse($this->manager->isEnabled()); } + #[Group(name: 'DB')] public function testManagerIsEnabled(): void { + $appConfig = Server::get(IAppConfig::class); + $appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->expects($this->any())->method('getSystemValueBool')->willReturn(true); - $this->config->expects($this->any())->method('getAppValue')->willReturn('yes'); - $this->assertTrue($this->manager->isEnabled()); + $result = $this->manager->isEnabled(); + + $appConfig->deleteKey('core', 'encryption_enabled'); + $this->assertTrue($result); } public function testModuleRegistration() {