From a8f6d10cf2f47aaca35e5373c5c99e3294d74bcf Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Wed, 29 Apr 2026 11:59:33 -0400 Subject: [PATCH 01/10] refactor(encryption): Migrate appconfig keys to typed bool IAppConfig with repair step Switch all encryption config reads/writes from deprecated string-typed IConfig to bool-typed IAppConfig (getValueBool/setValueBool). Adds RetypeEncryptionConfigKeys repair step to retype existing string values to bool on upgrade. Includes lazy IAppConfig resolution in Manager and AppConfigTypeConflictException fallbacks throughout for safety during the upgrade window. Co-Authored-By: Claude Sonnet 4.6 (1M context) Signed-off-by: Stephen Cuppett --- apps/encryption/lib/Settings/Admin.php | 1 + apps/encryption/lib/Util.php | 17 +-- apps/encryption/tests/Settings/AdminTest.php | 6 +- apps/encryption/tests/UtilTest.php | 29 ++--- .../lib/Controller/AppConfigController.php | 3 +- .../Controller/AppConfigControllerTest.php | 6 ++ build/psalm-baseline.xml | 15 --- core/Command/Encryption/DecryptAll.php | 26 +++-- core/Command/Encryption/Disable.php | 20 +++- core/Command/Encryption/Enable.php | 22 +++- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Encryption/Manager.php | 15 ++- lib/private/Repair.php | 2 + .../Repair/RetypeEncryptionConfigKeys.php | 55 ++++++++++ .../Command/Encryption/DecryptAllTest.php | 6 +- tests/Core/Command/Encryption/DisableTest.php | 30 +++--- tests/Core/Command/Encryption/EnableTest.php | 51 +++++---- tests/lib/Encryption/ManagerTest.php | 14 ++- .../Repair/RetypeEncryptionConfigKeysTest.php | 101 ++++++++++++++++++ 20 files changed, 311 insertions(+), 110 deletions(-) create mode 100644 lib/private/Repair/RetypeEncryptionConfigKeys.php create mode 100644 tests/lib/Repair/RetypeEncryptionConfigKeysTest.php diff --git a/apps/encryption/lib/Settings/Admin.php b/apps/encryption/lib/Settings/Admin.php index 422d338d45e03..13cd9a4017e1a 100644 --- a/apps/encryption/lib/Settings/Admin.php +++ b/apps/encryption/lib/Settings/Admin.php @@ -53,6 +53,7 @@ public function getForm() { $crypt, $this->userSession, $this->config, + $this->appConfig, $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..bc6d50b86aa09 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -11,6 +11,7 @@ use OC\Files\View; use OCA\Encryption\Crypto\Crypt; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; @@ -25,6 +26,7 @@ public function __construct( private Crypt $crypt, IUserSession $userSession, private IConfig $config, + private IAppConfig $appConfig, private IUserManager $userManager, ) { $this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false; @@ -51,13 +53,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); } /** @@ -66,12 +62,7 @@ public function shouldEncryptHomeStorage() { * @param bool $encryptHomeStorage */ public function setEncryptHomeStorage($encryptHomeStorage) { - $value = $encryptHomeStorage ? '1' : '0'; - $this->config->setAppValue( - 'encryption', - 'encryptHomeStorage', - $value - ); + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', (bool)$encryptHomeStorage); } /** diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php index 10d2a61c5279e..f81dfe3e87115 100644 --- a/apps/encryption/tests/Settings/AdminTest.php +++ b/apps/encryption/tests/Settings/AdminTest.php @@ -62,7 +62,8 @@ public function testGetForm(): void { $this->appConfig ->method('getValueBool') ->willReturnMap([ - ['encryption', 'recoveryAdminEnabled', true] + ['encryption', 'recoveryAdminEnabled', true], + ['encryption', 'encryptHomeStorage', true, true], ]); $this->config ->method('getAppValue') @@ -70,9 +71,6 @@ public function testGetForm(): void { if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') { return '1'; } - if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') { - return '1'; - } return $default; }); diff --git a/apps/encryption/tests/UtilTest.php b/apps/encryption/tests/UtilTest.php index ed274f74ab5ca..5f42f18ab4632 100644 --- a/apps/encryption/tests/UtilTest.php +++ b/apps/encryption/tests/UtilTest.php @@ -14,6 +14,7 @@ use OCA\Encryption\Util; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; @@ -26,6 +27,7 @@ class UtilTest extends TestCase { protected Util $instance; protected static $tempStorage = []; + protected IAppConfig&MockObject $appConfigMock; protected IConfig&MockObject $configMock; protected View&MockObject $filesMock; protected IUserManager&MockObject $userManagerMock; @@ -78,6 +80,7 @@ protected function setUp(): void { ->willReturn(true); $this->configMock = $this->createMock(IConfig::class); + $this->appConfigMock = $this->createMock(IAppConfig::class); $this->configMock->expects($this->any()) ->method('getUserValue') @@ -87,7 +90,7 @@ protected function setUp(): void { ->method('setUserValue') ->willReturnCallback([$this, 'setValueTester']); - $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->userManagerMock); + $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->appConfigMock, $this->userManagerMock); } /** @@ -136,13 +139,13 @@ public static function dataTestIsMasterKeyEnabled(): array { } /** - * @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 +154,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/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php index 0c53e3c009110..374d74ea9cd24 100644 --- a/apps/provisioning_api/lib/Controller/AppConfigController.php +++ b/apps/provisioning_api/lib/Controller/AppConfigController.php @@ -203,7 +203,8 @@ protected function verifyConfigKey(string $app, string $key, string $value) { throw new \InvalidArgumentException('The given key can not be set'); } - if ($app === 'core' && $key === 'encryption_enabled' && $value !== 'yes') { + if ($app === 'core' && $key === 'encryption_enabled' + && !in_array(strtolower(trim($value)), ['yes', '1', 'true', 'on'], true)) { throw new \InvalidArgumentException('The given key can not be set'); } diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php index 55885aabf0a42..a1253411871b0 100644 --- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php @@ -367,6 +367,10 @@ public static function dataVerifyConfigKey(): array { ['dav', 'public_route', ''], ['files', 'remote_route', ''], ['core', 'encryption_enabled', 'yes'], + ['core', 'encryption_enabled', '1'], + ['core', 'encryption_enabled', 'true'], + ['core', 'encryption_enabled', 'YES'], + ['core', 'encryption_enabled', 'on'], ]; } @@ -384,6 +388,8 @@ public static function dataVerifyConfigKeyThrows(): array { ['contacts', 'types', ''], ['core', 'encryption_enabled', 'no'], ['core', 'encryption_enabled', ''], + ['core', 'encryption_enabled', '0'], + ['core', 'encryption_enabled', 'false'], ['core', 'public_files', ''], ['core', 'public_dav', ''], ['core', 'remote_files', ''], diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 3d97d80124ced..de3d900ee39a5 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1310,10 +1310,8 @@ - - @@ -3027,19 +3025,6 @@ - - - - - - - - - - - - - diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 85a70b49cec24..56fd22d3b2a6b 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -9,6 +9,7 @@ namespace OC\Core\Command\Encryption; use OCP\App\IAppManager; +use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; @@ -91,11 +92,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); + try { + $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $originallyEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } try { if ($originallyEnabled) { $output->write('Disable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', false); + $this->writeEncryptionEnabled(false); $output->writeln('done.'); } else { $output->writeln('Server side encryption not enabled. Nothing to do.'); @@ -123,18 +129,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(' aborted.'); if ($originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } } elseif (($uid !== '') && $originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } $this->resetMaintenanceAndTrashbin(); return 0; } if ($originallyEnabled) { $output->write('Enable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); $output->writeln('done.'); } $output->writeln('aborted'); @@ -142,10 +148,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int } catch (\Exception $e) { // enable server side encryption again if something went wrong if ($originallyEnabled) { - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } $this->resetMaintenanceAndTrashbin(); throw $e; } } + + private function writeEncryptionEnabled(bool $enabled): void { + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', $enabled); + } catch (AppConfigTypeConflictException) { + $this->appConfig->setValueString('core', 'encryption_enabled', $enabled ? 'yes' : 'no'); + } + } } diff --git a/core/Command/Encryption/Disable.php b/core/Command/Encryption/Disable.php index fe280daa111b4..0f69be38f1011 100644 --- a/core/Command/Encryption/Disable.php +++ b/core/Command/Encryption/Disable.php @@ -9,14 +9,15 @@ */ namespace OC\Core\Command\Encryption; -use OCP\IConfig; +use OCP\Exceptions\AppConfigTypeConflictException; +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 +32,21 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') { + try { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } + + if (!$isEnabled) { $output->writeln('Encryption is already disabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'no'); + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $this->appConfig->setValueString('core', 'encryption_enabled', 'no'); + } $output->writeln('Encryption disabled'); } return 0; diff --git a/core/Command/Encryption/Enable.php b/core/Command/Encryption/Enable.php index 02c610250011c..66d30032f5842 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -10,14 +10,15 @@ namespace OC\Core\Command\Encryption; use OCP\Encryption\IManager; -use OCP\IConfig; +use OCP\Exceptions\AppConfigTypeConflictException; +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 +34,21 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') { + try { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } + + if ($isEnabled) { $output->writeln('Encryption is already enabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'yes'); + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', true); + } catch (AppConfigTypeConflictException) { + $this->appConfig->setValueString('core', 'encryption_enabled', 'yes'); + } $output->writeln('Encryption enabled'); } $output->writeln(''); @@ -46,7 +58,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/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 28373a5af8597..98c8afb6d8c8c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2094,6 +2094,7 @@ 'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php', + 'OC\\Repair\\RetypeEncryptionConfigKeys' => $baseDir . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e28dc90763681..4bc245fcde81b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2135,6 +2135,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php', + 'OC\\Repair\\RetypeEncryptionConfigKeys' => __DIR__ . '/../../..' . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php', diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index ac27f0911b8da..f8b278d3d3b9e 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -16,10 +16,12 @@ use OC\ServiceUnavailableException; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; +use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\IConfig; use OCP\IL10N; +use OCP\Server; use Psr\Log\LoggerInterface; class Manager implements IManager { @@ -48,8 +50,17 @@ 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', false); + } catch (AppConfigTypeConflictException) { + // Stored as VALUE_STRING from a pre-upgrade installation. + // RetypeEncryptionConfigKeys repair step will fix the type on occ upgrade. + $raw = Server::get(\OCP\IAppConfig::class)->getValueString('core', 'encryption_enabled', 'no'); + return in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } catch (\Throwable) { + // DB not ready (e.g. oc_appconfig does not yet exist during install). + return false; + } } /** diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 90e209cfe9085..3bc5827e4c44e 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -56,6 +56,7 @@ use OC\Repair\RepairInvalidShares; use OC\Repair\RepairLogoDimension; use OC\Repair\RepairMimeTypes; +use OC\Repair\RetypeEncryptionConfigKeys; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; @@ -157,6 +158,7 @@ public function addStep(IRepairStep|string $repairStep, bool $includeExpensive = */ public static function getRepairSteps(bool $includeExpensive = false): array { $repairSteps = [ + Server::get(RetypeEncryptionConfigKeys::class), new Collation(Server::get(IConfig::class), Server::get(LoggerInterface::class), Server::get(IDBConnection::class), false), Server::get(CleanTags::class), Server::get(RepairInvalidShares::class), diff --git a/lib/private/Repair/RetypeEncryptionConfigKeys.php b/lib/private/Repair/RetypeEncryptionConfigKeys.php new file mode 100644 index 0000000000000..48f0ce17c12c6 --- /dev/null +++ b/lib/private/Repair/RetypeEncryptionConfigKeys.php @@ -0,0 +1,55 @@ +appConfig->getValueType($app, $key); + } catch (AppConfigUnknownKeyException) { + continue; + } + + if (($type & IAppConfig::VALUE_BOOL) === IAppConfig::VALUE_BOOL) { + $output->info("$app.$key is already typed as boolean, skipping."); + continue; + } + + $raw = strtolower(trim($this->appConfig->getValueString($app, $key, $defaultBool ? '1' : '0'))); + $bool = in_array($raw, ['1', 'true', 'yes', 'on'], true); + + $this->appConfig->deleteKey($app, $key); + $this->appConfig->setValueBool($app, $key, $bool); + $output->info("Re-typed $app.$key from string '$raw' to boolean " . ($bool ? 'true' : '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..2b414f0c64f91 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,18 @@ public function testManagerIsDisabledIfDisabledButModules(): void { $this->assertFalse($this->manager->isEnabled()); } + /** + * @group 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() { diff --git a/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php b/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php new file mode 100644 index 0000000000000..48bea2cb97739 --- /dev/null +++ b/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php @@ -0,0 +1,101 @@ +appConfig = Server::get(IAppConfig::class); + $this->output = $this->createMock(IOutput::class); + $this->repair = new RetypeEncryptionConfigKeys($this->appConfig); + // Clean slate in case previous test runs or occ invocations left residue + $this->appConfig->deleteKey('core', 'encryption_enabled'); + $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); + } + + protected function tearDown(): void { + $this->appConfig->deleteKey('core', 'encryption_enabled'); + $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); + parent::tearDown(); + } + + public function testAbsentKeyIsNoOp(): void { + $this->output->expects($this->never())->method('info'); + $this->repair->run($this->output); + // No exception, no write + $this->assertTrue(true); + } + + public static function dataStringValues(): array { + return [ + ['yes', true], + ['no', false], + ['1', true], + ['0', false], + ['true', true], + ['false', false], + ['on', true], + ['YES', true], + ]; + } + + /** + * @dataProvider dataStringValues + */ + public function testEncryptionEnabledIsRetypedFromString(string $raw, bool $expected): void { + $this->appConfig->setValueString('core', 'encryption_enabled', $raw); + + $this->repair->run($this->output); + + $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('core', 'encryption_enabled')); + $this->assertSame($expected, $this->appConfig->getValueBool('core', 'encryption_enabled', !$expected)); + } + + /** + * @dataProvider dataStringValues + */ + public function testEncryptHomeStorageIsRetypedFromString(string $raw, bool $expected): void { + $this->appConfig->setValueString('encryption', 'encryptHomeStorage', $raw); + + $this->repair->run($this->output); + + $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('encryption', 'encryptHomeStorage')); + $this->assertSame($expected, $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', !$expected)); + } + + public function testAlreadyBoolIsNoOp(): void { + $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', false); + + // Should log "already typed" messages but not re-write + $this->output->expects($this->exactly(2)) + ->method('info') + ->with($this->stringContains('already typed')); + + $this->repair->run($this->output); + + $this->assertTrue($this->appConfig->getValueBool('core', 'encryption_enabled', false)); + $this->assertFalse($this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true)); + } +} From cb9e16783cfe4d3b93c97791b287bfd632eec564 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Thu, 30 Apr 2026 07:09:50 -0400 Subject: [PATCH 02/10] fix(encryption): Assume boolValue for commands Signed-off-by: Stephen Cuppett --- core/Command/Encryption/DecryptAll.php | 26 ++++++-------------------- core/Command/Encryption/Disable.php | 15 ++------------- core/Command/Encryption/Enable.php | 15 ++------------- 3 files changed, 10 insertions(+), 46 deletions(-) diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 56fd22d3b2a6b..1fd15e9cba476 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -9,7 +9,6 @@ namespace OC\Core\Command\Encryption; use OCP\App\IAppManager; -use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; @@ -92,16 +91,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - try { - $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); - } catch (AppConfigTypeConflictException) { - $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); - $originallyEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); - } + $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); try { if ($originallyEnabled) { $output->write('Disable server side encryption... '); - $this->writeEncryptionEnabled(false); + $this->appConfig->setValueBool('core', 'encryption_enabled', false); $output->writeln('done.'); } else { $output->writeln('Server side encryption not enabled. Nothing to do.'); @@ -129,18 +123,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(' aborted.'); if ($originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->writeEncryptionEnabled(true); + $this->appConfig->setValueBool('core', 'encryption_enabled', true); } } elseif (($uid !== '') && $originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->writeEncryptionEnabled(true); + $this->appConfig->setValueBool('core', 'encryption_enabled', true); } $this->resetMaintenanceAndTrashbin(); return 0; } if ($originallyEnabled) { $output->write('Enable server side encryption... '); - $this->writeEncryptionEnabled(true); + $this->appConfig->setValueBool('core', 'encryption_enabled', true); $output->writeln('done.'); } $output->writeln('aborted'); @@ -148,18 +142,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int } catch (\Exception $e) { // enable server side encryption again if something went wrong if ($originallyEnabled) { - $this->writeEncryptionEnabled(true); + $this->appConfig->setValueBool('core', 'encryption_enabled', true); } $this->resetMaintenanceAndTrashbin(); throw $e; } } - - private function writeEncryptionEnabled(bool $enabled): void { - try { - $this->appConfig->setValueBool('core', 'encryption_enabled', $enabled); - } catch (AppConfigTypeConflictException) { - $this->appConfig->setValueString('core', 'encryption_enabled', $enabled ? 'yes' : 'no'); - } - } } diff --git a/core/Command/Encryption/Disable.php b/core/Command/Encryption/Disable.php index 0f69be38f1011..a50960a4b0488 100644 --- a/core/Command/Encryption/Disable.php +++ b/core/Command/Encryption/Disable.php @@ -9,7 +9,6 @@ */ namespace OC\Core\Command\Encryption; -use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -32,21 +31,11 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - try { - $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); - } catch (AppConfigTypeConflictException) { - $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); - $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); - } - + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); if (!$isEnabled) { $output->writeln('Encryption is already disabled'); } else { - try { - $this->appConfig->setValueBool('core', 'encryption_enabled', false); - } catch (AppConfigTypeConflictException) { - $this->appConfig->setValueString('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 66d30032f5842..7df3851b5c4c3 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -10,7 +10,6 @@ namespace OC\Core\Command\Encryption; use OCP\Encryption\IManager; -use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -34,21 +33,11 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - try { - $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); - } catch (AppConfigTypeConflictException) { - $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); - $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); - } - + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); if ($isEnabled) { $output->writeln('Encryption is already enabled'); } else { - try { - $this->appConfig->setValueBool('core', 'encryption_enabled', true); - } catch (AppConfigTypeConflictException) { - $this->appConfig->setValueString('core', 'encryption_enabled', 'yes'); - } + $this->appConfig->setValueBool('core', 'encryption_enabled', true); $output->writeln('Encryption enabled'); } $output->writeln(''); From cd6d0d9cd913a9b96ba41c840121501841b578a6 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Thu, 30 Apr 2026 19:08:35 -0400 Subject: [PATCH 03/10] fix(encryption): Missed explicit typing Apply suggestion from @artonge Co-authored-by: Louis Signed-off-by: Stephen Cuppett --- apps/encryption/lib/Util.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/encryption/lib/Util.php b/apps/encryption/lib/Util.php index bc6d50b86aa09..58163085f8227 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -61,8 +61,8 @@ public function shouldEncryptHomeStorage() { * * @param bool $encryptHomeStorage */ - public function setEncryptHomeStorage($encryptHomeStorage) { - $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', (bool)$encryptHomeStorage); + public function setEncryptHomeStorage(bool $encryptHomeStorage) { + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', $encryptHomeStorage); } /** From 5240705879b09841fdd2882ac78923e06946a5c2 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Fri, 1 May 2026 07:43:06 -0400 Subject: [PATCH 04/10] fix(encryption): Simplify evaluation in AppConfigController Co-authored-by: Louis Signed-off-by: Stephen Cuppett --- apps/provisioning_api/lib/Controller/AppConfigController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php index 374d74ea9cd24..0c53e3c009110 100644 --- a/apps/provisioning_api/lib/Controller/AppConfigController.php +++ b/apps/provisioning_api/lib/Controller/AppConfigController.php @@ -203,8 +203,7 @@ protected function verifyConfigKey(string $app, string $key, string $value) { throw new \InvalidArgumentException('The given key can not be set'); } - if ($app === 'core' && $key === 'encryption_enabled' - && !in_array(strtolower(trim($value)), ['yes', '1', 'true', 'on'], true)) { + if ($app === 'core' && $key === 'encryption_enabled' && $value !== 'yes') { throw new \InvalidArgumentException('The given key can not be set'); } From f78df12d7cd05868575ad49fa8d09672ced66c71 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Fri, 1 May 2026 07:53:19 -0400 Subject: [PATCH 05/10] refactor(encryption): Drop RetypeEncryptionConfigKeys repair step The IAppConfig API converts stored values to bool on read (getValueBool) and re-stamps the type on write (setValueBool), so legacy string-typed encryption config keys migrate lazily without an explicit repair step. Per PR review feedback, drop the repair step, its test, and the related AppConfigTypeConflictException fallback in Encryption\Manager::isEnabled that only existed to bridge the now-unneeded migration window. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Stephen Cuppett --- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/Encryption/Manager.php | 6 -- lib/private/Repair.php | 2 - .../Repair/RetypeEncryptionConfigKeys.php | 55 ---------- .../Repair/RetypeEncryptionConfigKeysTest.php | 101 ------------------ 6 files changed, 166 deletions(-) delete mode 100644 lib/private/Repair/RetypeEncryptionConfigKeys.php delete mode 100644 tests/lib/Repair/RetypeEncryptionConfigKeysTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 98c8afb6d8c8c..28373a5af8597 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2094,7 +2094,6 @@ 'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php', - 'OC\\Repair\\RetypeEncryptionConfigKeys' => $baseDir . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 4bc245fcde81b..e28dc90763681 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2135,7 +2135,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php', - 'OC\\Repair\\RetypeEncryptionConfigKeys' => __DIR__ . '/../../..' . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php', diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index f8b278d3d3b9e..540b832bab3ac 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -16,7 +16,6 @@ use OC\ServiceUnavailableException; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; -use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\IConfig; @@ -52,11 +51,6 @@ public function isEnabled() { try { return Server::get(\OCP\IAppConfig::class)->getValueBool('core', 'encryption_enabled', false); - } catch (AppConfigTypeConflictException) { - // Stored as VALUE_STRING from a pre-upgrade installation. - // RetypeEncryptionConfigKeys repair step will fix the type on occ upgrade. - $raw = Server::get(\OCP\IAppConfig::class)->getValueString('core', 'encryption_enabled', 'no'); - return in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); } catch (\Throwable) { // DB not ready (e.g. oc_appconfig does not yet exist during install). return false; diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 3bc5827e4c44e..90e209cfe9085 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -56,7 +56,6 @@ use OC\Repair\RepairInvalidShares; use OC\Repair\RepairLogoDimension; use OC\Repair\RepairMimeTypes; -use OC\Repair\RetypeEncryptionConfigKeys; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; @@ -158,7 +157,6 @@ public function addStep(IRepairStep|string $repairStep, bool $includeExpensive = */ public static function getRepairSteps(bool $includeExpensive = false): array { $repairSteps = [ - Server::get(RetypeEncryptionConfigKeys::class), new Collation(Server::get(IConfig::class), Server::get(LoggerInterface::class), Server::get(IDBConnection::class), false), Server::get(CleanTags::class), Server::get(RepairInvalidShares::class), diff --git a/lib/private/Repair/RetypeEncryptionConfigKeys.php b/lib/private/Repair/RetypeEncryptionConfigKeys.php deleted file mode 100644 index 48f0ce17c12c6..0000000000000 --- a/lib/private/Repair/RetypeEncryptionConfigKeys.php +++ /dev/null @@ -1,55 +0,0 @@ -appConfig->getValueType($app, $key); - } catch (AppConfigUnknownKeyException) { - continue; - } - - if (($type & IAppConfig::VALUE_BOOL) === IAppConfig::VALUE_BOOL) { - $output->info("$app.$key is already typed as boolean, skipping."); - continue; - } - - $raw = strtolower(trim($this->appConfig->getValueString($app, $key, $defaultBool ? '1' : '0'))); - $bool = in_array($raw, ['1', 'true', 'yes', 'on'], true); - - $this->appConfig->deleteKey($app, $key); - $this->appConfig->setValueBool($app, $key, $bool); - $output->info("Re-typed $app.$key from string '$raw' to boolean " . ($bool ? 'true' : 'false') . '.'); - } - } -} diff --git a/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php b/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php deleted file mode 100644 index 48bea2cb97739..0000000000000 --- a/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php +++ /dev/null @@ -1,101 +0,0 @@ -appConfig = Server::get(IAppConfig::class); - $this->output = $this->createMock(IOutput::class); - $this->repair = new RetypeEncryptionConfigKeys($this->appConfig); - // Clean slate in case previous test runs or occ invocations left residue - $this->appConfig->deleteKey('core', 'encryption_enabled'); - $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); - } - - protected function tearDown(): void { - $this->appConfig->deleteKey('core', 'encryption_enabled'); - $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); - parent::tearDown(); - } - - public function testAbsentKeyIsNoOp(): void { - $this->output->expects($this->never())->method('info'); - $this->repair->run($this->output); - // No exception, no write - $this->assertTrue(true); - } - - public static function dataStringValues(): array { - return [ - ['yes', true], - ['no', false], - ['1', true], - ['0', false], - ['true', true], - ['false', false], - ['on', true], - ['YES', true], - ]; - } - - /** - * @dataProvider dataStringValues - */ - public function testEncryptionEnabledIsRetypedFromString(string $raw, bool $expected): void { - $this->appConfig->setValueString('core', 'encryption_enabled', $raw); - - $this->repair->run($this->output); - - $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('core', 'encryption_enabled')); - $this->assertSame($expected, $this->appConfig->getValueBool('core', 'encryption_enabled', !$expected)); - } - - /** - * @dataProvider dataStringValues - */ - public function testEncryptHomeStorageIsRetypedFromString(string $raw, bool $expected): void { - $this->appConfig->setValueString('encryption', 'encryptHomeStorage', $raw); - - $this->repair->run($this->output); - - $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('encryption', 'encryptHomeStorage')); - $this->assertSame($expected, $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', !$expected)); - } - - public function testAlreadyBoolIsNoOp(): void { - $this->appConfig->setValueBool('core', 'encryption_enabled', true); - $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', false); - - // Should log "already typed" messages but not re-write - $this->output->expects($this->exactly(2)) - ->method('info') - ->with($this->stringContains('already typed')); - - $this->repair->run($this->output); - - $this->assertTrue($this->appConfig->getValueBool('core', 'encryption_enabled', false)); - $this->assertFalse($this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true)); - } -} From c9894fe1799fa456844e07ddb8dd123c78fbfee4 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Fri, 1 May 2026 09:11:54 -0400 Subject: [PATCH 06/10] test(provisioning_api): Revert AppConfigControllerTest data providers to match strict 'yes'-only validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verifyConfigKey check on core.encryption_enabled was reverted to master's strict $value !== 'yes' in 626fadde0a7 per review feedback, but the test data providers still asserted the broader truthy set (1/true/YES/on). Drop those entries so the tests match the controller. This is validation, not storage — IAppConfig::setValueBool's broader input handling is unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Stephen Cuppett --- .../tests/Controller/AppConfigControllerTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php index a1253411871b0..55885aabf0a42 100644 --- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php @@ -367,10 +367,6 @@ public static function dataVerifyConfigKey(): array { ['dav', 'public_route', ''], ['files', 'remote_route', ''], ['core', 'encryption_enabled', 'yes'], - ['core', 'encryption_enabled', '1'], - ['core', 'encryption_enabled', 'true'], - ['core', 'encryption_enabled', 'YES'], - ['core', 'encryption_enabled', 'on'], ]; } @@ -388,8 +384,6 @@ public static function dataVerifyConfigKeyThrows(): array { ['contacts', 'types', ''], ['core', 'encryption_enabled', 'no'], ['core', 'encryption_enabled', ''], - ['core', 'encryption_enabled', '0'], - ['core', 'encryption_enabled', 'false'], ['core', 'public_files', ''], ['core', 'public_dav', ''], ['core', 'remote_files', ''], From 1dc1cf4b874f7d0c79d5a3233db70962b7085f8d Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Wed, 6 May 2026 19:26:00 -0400 Subject: [PATCH 07/10] fix(tests): Update tests/lib/Encryption/ManagerTest.php Update to annotation from docblock comment Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Stephen Cuppett --- tests/lib/Encryption/ManagerTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/lib/Encryption/ManagerTest.php b/tests/lib/Encryption/ManagerTest.php index 2b414f0c64f91..b1ecaa02f490b 100644 --- a/tests/lib/Encryption/ManagerTest.php +++ b/tests/lib/Encryption/ManagerTest.php @@ -75,9 +75,7 @@ public function testManagerIsDisabledIfDisabledButModules(): void { $this->assertFalse($this->manager->isEnabled()); } - /** - * @group DB - */ + #[Group(name: 'DB')] public function testManagerIsEnabled(): void { $appConfig = Server::get(IAppConfig::class); $appConfig->setValueBool('core', 'encryption_enabled', true); From 6a8a1af064876a19ccbd4357ab349c6b162b90e5 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Wed, 6 May 2026 19:26:40 -0400 Subject: [PATCH 08/10] fix(encryption): No need for default values to be explicitly specified. Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Stephen Cuppett --- core/Command/Encryption/DecryptAll.php | 2 +- core/Command/Encryption/Enable.php | 4 ++-- lib/private/Encryption/Manager.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 1fd15e9cba476..85a70b49cec24 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -91,7 +91,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); try { if ($originallyEnabled) { $output->write('Disable server side encryption... '); diff --git a/core/Command/Encryption/Enable.php b/core/Command/Encryption/Enable.php index 7df3851b5c4c3..6ad91be947c49 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -33,7 +33,7 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); if ($isEnabled) { $output->writeln('Encryption is already enabled'); } else { @@ -47,7 +47,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('No encryption module is loaded'); return 1; } - $defaultModule = $this->appConfig->getValueString('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 540b832bab3ac..83d010dbe8752 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -50,7 +50,7 @@ public function isEnabled() { } try { - return Server::get(\OCP\IAppConfig::class)->getValueBool('core', 'encryption_enabled', false); + 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; From 44f11e868ec0356d5b36291bd73d8aff5dc15918 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Wed, 6 May 2026 19:36:09 -0400 Subject: [PATCH 09/10] fix(tests): Remove dead getAppValue mock for recoveryAdminEnabled in AdminTest The production code already uses getValueBool for this key; the IConfig mock branch was unreachable. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Stephen Cuppett --- apps/encryption/tests/Settings/AdminTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php index f81dfe3e87115..a3f90913e9e61 100644 --- a/apps/encryption/tests/Settings/AdminTest.php +++ b/apps/encryption/tests/Settings/AdminTest.php @@ -68,9 +68,6 @@ public function testGetForm(): void { $this->config ->method('getAppValue') ->willReturnCallback(function ($app, $key, $default) { - if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') { - return '1'; - } return $default; }); From ce3532d83d9c4a11c3179409fcbbdf89ab002bd7 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Wed, 6 May 2026 20:03:17 -0400 Subject: [PATCH 10/10] refactor(encryption): Migrate remaining deprecated IConfig calls to typed APIs Replace IConfig::{get,set}UserValue for the per-user 'recoveryEnabled' key with IUserConfig::{getValueBool,setValueBool}, and IConfig::getAppValue for 'useMasterKey' with IAppConfig::getValueBool. IConfig is removed from Util and Recovery constructors entirely. Clears the DeprecatedMethod psalm-baseline entries for apps/encryption/lib/Util.php and the string-typed recoveryAdminEnabled calls that were still in Recovery.php. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Stephen Cuppett --- .../lib/Controller/RecoveryController.php | 2 +- apps/encryption/lib/Recovery.php | 47 ++++--------- apps/encryption/lib/Settings/Admin.php | 4 +- apps/encryption/lib/Util.php | 23 ++----- .../Controller/RecoveryControllerTest.php | 6 +- apps/encryption/tests/RecoveryTest.php | 67 ++++++++----------- apps/encryption/tests/Settings/AdminTest.php | 10 +-- apps/encryption/tests/UtilTest.php | 57 ++++++---------- build/psalm-baseline.xml | 12 ---- 9 files changed, 83 insertions(+), 145 deletions(-) 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 13cd9a4017e1a..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,8 +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 58163085f8227..255753fe0a188 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -10,9 +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\IAppConfig; -use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -25,8 +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; @@ -39,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'); } /** @@ -69,22 +64,16 @@ public function setEncryptHomeStorage(bool $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 a3f90913e9e61..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, ); } @@ -64,12 +68,8 @@ public function testGetForm(): void { ->willReturnMap([ ['encryption', 'recoveryAdminEnabled', true], ['encryption', 'encryptHomeStorage', true, true], + ['encryption', 'useMasterKey', true, true], ]); - $this->config - ->method('getAppValue') - ->willReturnCallback(function ($app, $key, $default) { - return $default; - }); $this->initialState ->expects(self::once()) diff --git a/apps/encryption/tests/UtilTest.php b/apps/encryption/tests/UtilTest.php index 5f42f18ab4632..0a7ace6955d4a 100644 --- a/apps/encryption/tests/UtilTest.php +++ b/apps/encryption/tests/UtilTest.php @@ -12,10 +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\IAppConfig; -use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -28,13 +28,13 @@ class UtilTest extends TestCase { protected static $tempStorage = []; protected IAppConfig&MockObject $appConfigMock; - protected IConfig&MockObject $configMock; + 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); } @@ -43,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 { @@ -79,42 +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->appConfigMock, $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; } /** @@ -123,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() ); @@ -133,8 +120,8 @@ public function testIsMasterKeyEnabled($value, $expect): void { public static function dataTestIsMasterKeyEnabled(): array { return [ - ['0', false], - ['1', true] + [false, false], + [true, true] ]; } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index de3d900ee39a5..2375bd3c88105 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1286,13 +1286,6 @@ - - - - - - - @@ -1309,11 +1302,6 @@ - - - - -