refactor(encryption): Migrate appconfig keys to typed bool IAppConfig with repair step

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

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,27 +53,16 @@ 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);
 	}
 
 	/**
 	 * set the home storage encryption on/off
 	 *
 	 * @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);
 	}
 
 	/**

apps/encryption/tests/Settings/AdminTest.php

@@ -62,17 +62,15 @@ public function testGetForm(): void {
 		$this->appConfig
 			->method('getValueBool')
 			->willReturnMap([
-				['encryption', 'recoveryAdminEnabled', true]
+				['encryption', 'recoveryAdminEnabled', true],
+				['encryption', 'encryptHomeStorage', 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;
 			});
 

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]
 		];
 	}
 

build/psalm-baseline.xml

@@ -1300,10 +1300,8 @@
   </file>
   <file src="apps/encryption/lib/Util.php">
     <DeprecatedMethod>
-      <code><![CDATA[getAppValue]]></code>
       <code><![CDATA[getAppValue]]></code>
       <code><![CDATA[getUserValue]]></code>
-      <code><![CDATA[setAppValue]]></code>
       <code><![CDATA[setUserValue]]></code>
     </DeprecatedMethod>
     <InternalMethod>
@@ -3017,19 +3015,6 @@
       <code><![CDATA[\OC_Util::tearDownFS()]]></code>
     </DeprecatedMethod>
   </file>
-  <file src="core/Command/Encryption/Disable.php">
-    <DeprecatedMethod>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[setAppValue]]></code>
-    </DeprecatedMethod>
-  </file>
-  <file src="core/Command/Encryption/Enable.php">
-    <DeprecatedMethod>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[setAppValue]]></code>
-    </DeprecatedMethod>
-  </file>
   <file src="core/Command/Encryption/MigrateKeyStorage.php">
     <DeprecatedClass>
       <code><![CDATA[\OC_Util::setupFS($uid)]]></code>

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');
+		$originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
 		try {
 			if ($originallyEnabled) {
 				$output->write('Disable server side encryption... ');

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('<info>Encryption disabled</info>');
 		}
 		return 0;

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', false);
+		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('<info>Encryption enabled</info>');
 		}
 		$output->writeln('');
@@ -46,7 +47,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$output->writeln('<error>No encryption module is loaded</error>');
 			return 1;
 		}
-		$defaultModule = $this->config->getAppValue('core', 'default_encryption_module');
+		$defaultModule = $this->appConfig->getValueString('core', 'default_encryption_module', '');
 		if ($defaultModule === '') {
 			$output->writeln('<error>No default module is set</error>');
 			return 1;

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', false);
+		} catch (\Throwable) {
+			// DB not ready (e.g. oc_appconfig does not yet exist during install).
+			return false;
+		}
 	}
 
 	/**

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,15 +168,15 @@ 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());
 				return true;
 			});
 		$this->appConfig->expects($this->once())
 			->method('getValueBool')
-			->with('core', 'encryption_enabled')
+			->with('core', 'encryption_enabled', false)
 			->willReturn(true);
 
 		$this->consoleInput->expects($this->any())

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,45 +29,45 @@ 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())
 			->method('writeln')
 			->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]);