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

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') {

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/');

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

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,49 +48,32 @@ 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);
 	}
 
 	/**
 	 * 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;

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

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,32 +142,32 @@ 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'));
 	}
 
 	public function testIsRecoveryKeyEnabled(): void {
 		$this->assertFalse($this->instance->isRecoveryKeyEnabled());
-		self::$tempStorage['recoveryAdminEnabled'] = '1';
+		self::$tempStorage['recoveryAdminEnabled'] = true;
 		$this->assertTrue($this->instance->isRecoveryKeyEnabled());
 	}
 
 	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;
 	}
 }