Skip to content

Commit 2a704a5

Browse files
cuppettclaude
cuppett
and
claude
committed
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) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
1 parent c26de18 commit 2a704a5

20 files changed

Lines changed: 311 additions & 110 deletions

File tree

‎apps/encryption/lib/Settings/Admin.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public function getForm() {
5353
$crypt,
5454
$this->userSession,
5555
$this->config,
56+
$this->appConfig,
5657
$this->userManager);
5758

5859
// Check if an adminRecovery account is enabled for recovering files after lost pwd

‎apps/encryption/lib/Util.php‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OC\Files\View;
1212
use OCA\Encryption\Crypto\Crypt;
1313
use OCP\Files\Storage\IStorage;
14+
use OCP\IAppConfig;
1415
use OCP\IConfig;
1516
use OCP\IUser;
1617
use OCP\IUserManager;
@@ -25,6 +26,7 @@ public function __construct(
2526
private Crypt $crypt,
2627
IUserSession $userSession,
2728
private IConfig $config,
29+
private IAppConfig $appConfig,
2830
private IUserManager $userManager,
2931
) {
3032
$this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false;
@@ -51,13 +53,7 @@ public function isRecoveryEnabledForUser($uid) {
5153
* @return bool
5254
*/
5355
public function shouldEncryptHomeStorage() {
54-
$encryptHomeStorage = $this->config->getAppValue(
55-
'encryption',
56-
'encryptHomeStorage',
57-
'1'
58-
);
59-
60-
return ($encryptHomeStorage === '1');
56+
return $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true);
6157
}
6258

6359
/**
@@ -66,12 +62,7 @@ public function shouldEncryptHomeStorage() {
6662
* @param bool $encryptHomeStorage
6763
*/
6864
public function setEncryptHomeStorage($encryptHomeStorage) {
69-
$value = $encryptHomeStorage ? '1' : '0';
70-
$this->config->setAppValue(
71-
'encryption',
72-
'encryptHomeStorage',
73-
$value
74-
);
65+
$this->appConfig->setValueBool('encryption', 'encryptHomeStorage', (bool)$encryptHomeStorage);
7566
}
7667

7768
/**

‎apps/encryption/tests/Settings/AdminTest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,15 @@ public function testGetForm(): void {
6262
$this->appConfig
6363
->method('getValueBool')
6464
->willReturnMap([
65-
['encryption', 'recoveryAdminEnabled', true]
65+
['encryption', 'recoveryAdminEnabled', true],
66+
['encryption', 'encryptHomeStorage', true, true],
6667
]);
6768
$this->config
6869
->method('getAppValue')
6970
->willReturnCallback(function ($app, $key, $default) {
7071
if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') {
7172
return '1';
7273
}
73-
if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') {
74-
return '1';
75-
}
7674
return $default;
7775
});
7876

‎apps/encryption/tests/UtilTest.php‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCA\Encryption\Util;
1515
use OCP\Files\Mount\IMountPoint;
1616
use OCP\Files\Storage\IStorage;
17+
use OCP\IAppConfig;
1718
use OCP\IConfig;
1819
use OCP\IUser;
1920
use OCP\IUserManager;
@@ -26,6 +27,7 @@ class UtilTest extends TestCase {
2627
protected Util $instance;
2728
protected static $tempStorage = [];
2829

30+
protected IAppConfig&MockObject $appConfigMock;
2931
protected IConfig&MockObject $configMock;
3032
protected View&MockObject $filesMock;
3133
protected IUserManager&MockObject $userManagerMock;
@@ -78,6 +80,7 @@ protected function setUp(): void {
7880
->willReturn(true);
7981

8082
$this->configMock = $this->createMock(IConfig::class);
83+
$this->appConfigMock = $this->createMock(IAppConfig::class);
8184

8285
$this->configMock->expects($this->any())
8386
->method('getUserValue')
@@ -87,7 +90,7 @@ protected function setUp(): void {
8790
->method('setUserValue')
8891
->willReturnCallback([$this, 'setValueTester']);
8992

90-
$this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->userManagerMock);
93+
$this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->appConfigMock, $this->userManagerMock);
9194
}
9295

9396
/**
@@ -136,13 +139,13 @@ public static function dataTestIsMasterKeyEnabled(): array {
136139
}
137140

138141
/**
139-
* @param string $returnValue return value from getAppValue()
142+
* @param bool $returnValue return value from getValueBool()
140143
* @param bool $expected
141144
*/
142145
#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestShouldEncryptHomeStorage')]
143-
public function testShouldEncryptHomeStorage($returnValue, $expected): void {
144-
$this->configMock->expects($this->once())->method('getAppValue')
145-
->with('encryption', 'encryptHomeStorage', '1')
146+
public function testShouldEncryptHomeStorage(bool $returnValue, bool $expected): void {
147+
$this->appConfigMock->expects($this->once())->method('getValueBool')
148+
->with('encryption', 'encryptHomeStorage', true)
146149
->willReturn($returnValue);
147150

148151
$this->assertSame($expected,
@@ -151,26 +154,26 @@ public function testShouldEncryptHomeStorage($returnValue, $expected): void {
151154

152155
public static function dataTestShouldEncryptHomeStorage(): array {
153156
return [
154-
['1', true],
155-
['0', false]
157+
[true, true],
158+
[false, false]
156159
];
157160
}
158161

159162
/**
160-
* @param $value
161-
* @param $expected
163+
* @param bool $value
164+
* @param bool $expected
162165
*/
163166
#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestSetEncryptHomeStorage')]
164-
public function testSetEncryptHomeStorage($value, $expected): void {
165-
$this->configMock->expects($this->once())->method('setAppValue')
167+
public function testSetEncryptHomeStorage(bool $value, bool $expected): void {
168+
$this->appConfigMock->expects($this->once())->method('setValueBool')
166169
->with('encryption', 'encryptHomeStorage', $expected);
167170
$this->instance->setEncryptHomeStorage($value);
168171
}
169172

170173
public static function dataTestSetEncryptHomeStorage(): array {
171174
return [
172-
[true, '1'],
173-
[false, '0']
175+
[true, true],
176+
[false, false]
174177
];
175178
}
176179

‎apps/provisioning_api/lib/Controller/AppConfigController.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ protected function verifyConfigKey(string $app, string $key, string $value) {
203203
throw new \InvalidArgumentException('The given key can not be set');
204204
}
205205

206-
if ($app === 'core' && $key === 'encryption_enabled' && $value !== 'yes') {
206+
if ($app === 'core' && $key === 'encryption_enabled'
207+
&& !in_array(strtolower(trim($value)), ['yes', '1', 'true', 'on'], true)) {
207208
throw new \InvalidArgumentException('The given key can not be set');
208209
}
209210

‎apps/provisioning_api/tests/Controller/AppConfigControllerTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,10 @@ public static function dataVerifyConfigKey(): array {
367367
['dav', 'public_route', ''],
368368
['files', 'remote_route', ''],
369369
['core', 'encryption_enabled', 'yes'],
370+
['core', 'encryption_enabled', '1'],
371+
['core', 'encryption_enabled', 'true'],
372+
['core', 'encryption_enabled', 'YES'],
373+
['core', 'encryption_enabled', 'on'],
370374
];
371375
}
372376

@@ -384,6 +388,8 @@ public static function dataVerifyConfigKeyThrows(): array {
384388
['contacts', 'types', ''],
385389
['core', 'encryption_enabled', 'no'],
386390
['core', 'encryption_enabled', ''],
391+
['core', 'encryption_enabled', '0'],
392+
['core', 'encryption_enabled', 'false'],
387393
['core', 'public_files', ''],
388394
['core', 'public_dav', ''],
389395
['core', 'remote_files', ''],

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,10 +1300,8 @@
13001300
</file>
13011301
<file src="apps/encryption/lib/Util.php">
13021302
<DeprecatedMethod>
1303-
<code><![CDATA[getAppValue]]></code>
13041303
<code><![CDATA[getAppValue]]></code>
13051304
<code><![CDATA[getUserValue]]></code>
1306-
<code><![CDATA[setAppValue]]></code>
13071305
<code><![CDATA[setUserValue]]></code>
13081306
</DeprecatedMethod>
13091307
<InternalMethod>
@@ -3023,19 +3021,6 @@
30233021
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
30243022
</DeprecatedMethod>
30253023
</file>
3026-
<file src="core/Command/Encryption/Disable.php">
3027-
<DeprecatedMethod>
3028-
<code><![CDATA[getAppValue]]></code>
3029-
<code><![CDATA[setAppValue]]></code>
3030-
</DeprecatedMethod>
3031-
</file>
3032-
<file src="core/Command/Encryption/Enable.php">
3033-
<DeprecatedMethod>
3034-
<code><![CDATA[getAppValue]]></code>
3035-
<code><![CDATA[getAppValue]]></code>
3036-
<code><![CDATA[setAppValue]]></code>
3037-
</DeprecatedMethod>
3038-
</file>
30393024
<file src="core/Command/Encryption/MigrateKeyStorage.php">
30403025
<DeprecatedClass>
30413026
<code><![CDATA[\OC_Util::setupFS($uid)]]></code>

‎core/Command/Encryption/DecryptAll.php‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\Core\Command\Encryption;
1010

1111
use OCP\App\IAppManager;
12+
use OCP\Exceptions\AppConfigTypeConflictException;
1213
use OCP\IAppConfig;
1314
use OCP\IConfig;
1415
use Symfony\Component\Console\Command\Command;
@@ -91,11 +92,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
9192
return 1;
9293
}
9394

94-
$originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled');
95+
try {
96+
$originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
97+
} catch (AppConfigTypeConflictException) {
98+
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
99+
$originallyEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
100+
}
95101
try {
96102
if ($originallyEnabled) {
97103
$output->write('Disable server side encryption... ');
98-
$this->appConfig->setValueBool('core', 'encryption_enabled', false);
104+
$this->writeEncryptionEnabled(false);
99105
$output->writeln('done.');
100106
} else {
101107
$output->writeln('Server side encryption not enabled. Nothing to do.');
@@ -123,29 +129,37 @@ protected function execute(InputInterface $input, OutputInterface $output): int
123129
$output->writeln(' aborted.');
124130
if ($originallyEnabled) {
125131
$output->writeln('Server side encryption remains enabled');
126-
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
132+
$this->writeEncryptionEnabled(true);
127133
}
128134
} elseif (($uid !== '') && $originallyEnabled) {
129135
$output->writeln('Server side encryption remains enabled');
130-
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
136+
$this->writeEncryptionEnabled(true);
131137
}
132138
$this->resetMaintenanceAndTrashbin();
133139
return 0;
134140
}
135141
if ($originallyEnabled) {
136142
$output->write('Enable server side encryption... ');
137-
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
143+
$this->writeEncryptionEnabled(true);
138144
$output->writeln('done.');
139145
}
140146
$output->writeln('aborted');
141147
return 1;
142148
} catch (\Exception $e) {
143149
// enable server side encryption again if something went wrong
144150
if ($originallyEnabled) {
145-
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
151+
$this->writeEncryptionEnabled(true);
146152
}
147153
$this->resetMaintenanceAndTrashbin();
148154
throw $e;
149155
}
150156
}
157+
158+
private function writeEncryptionEnabled(bool $enabled): void {
159+
try {
160+
$this->appConfig->setValueBool('core', 'encryption_enabled', $enabled);
161+
} catch (AppConfigTypeConflictException) {
162+
$this->appConfig->setValueString('core', 'encryption_enabled', $enabled ? 'yes' : 'no');
163+
}
164+
}
151165
}

‎core/Command/Encryption/Disable.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
*/
1010
namespace OC\Core\Command\Encryption;
1111

12-
use OCP\IConfig;
12+
use OCP\Exceptions\AppConfigTypeConflictException;
13+
use OCP\IAppConfig;
1314
use Symfony\Component\Console\Command\Command;
1415
use Symfony\Component\Console\Input\InputInterface;
1516
use Symfony\Component\Console\Output\OutputInterface;
1617

1718
class Disable extends Command {
1819
public function __construct(
19-
protected IConfig $config,
20+
protected IAppConfig $appConfig,
2021
) {
2122
parent::__construct();
2223
}
@@ -31,10 +32,21 @@ protected function configure() {
3132

3233
#[\Override]
3334
protected function execute(InputInterface $input, OutputInterface $output): int {
34-
if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') {
35+
try {
36+
$isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
37+
} catch (AppConfigTypeConflictException) {
38+
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
39+
$isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
40+
}
41+
42+
if (!$isEnabled) {
3543
$output->writeln('Encryption is already disabled');
3644
} else {
37-
$this->config->setAppValue('core', 'encryption_enabled', 'no');
45+
try {
46+
$this->appConfig->setValueBool('core', 'encryption_enabled', false);
47+
} catch (AppConfigTypeConflictException) {
48+
$this->appConfig->setValueString('core', 'encryption_enabled', 'no');
49+
}
3850
$output->writeln('<info>Encryption disabled</info>');
3951
}
4052
return 0;

‎core/Command/Encryption/Enable.php‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@
1010
namespace OC\Core\Command\Encryption;
1111

1212
use OCP\Encryption\IManager;
13-
use OCP\IConfig;
13+
use OCP\Exceptions\AppConfigTypeConflictException;
14+
use OCP\IAppConfig;
1415
use Symfony\Component\Console\Command\Command;
1516
use Symfony\Component\Console\Input\InputInterface;
1617
use Symfony\Component\Console\Output\OutputInterface;
1718

1819
class Enable extends Command {
1920
public function __construct(
20-
protected IConfig $config,
21+
protected IAppConfig $appConfig,
2122
protected IManager $encryptionManager,
2223
) {
2324
parent::__construct();
@@ -33,10 +34,21 @@ protected function configure() {
3334

3435
#[\Override]
3536
protected function execute(InputInterface $input, OutputInterface $output): int {
36-
if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') {
37+
try {
38+
$isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
39+
} catch (AppConfigTypeConflictException) {
40+
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
41+
$isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
42+
}
43+
44+
if ($isEnabled) {
3745
$output->writeln('Encryption is already enabled');
3846
} else {
39-
$this->config->setAppValue('core', 'encryption_enabled', 'yes');
47+
try {
48+
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
49+
} catch (AppConfigTypeConflictException) {
50+
$this->appConfig->setValueString('core', 'encryption_enabled', 'yes');
51+
}
4052
$output->writeln('<info>Encryption enabled</info>');
4153
}
4254
$output->writeln('');
@@ -46,7 +58,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
4658
$output->writeln('<error>No encryption module is loaded</error>');
4759
return 1;
4860
}
49-
$defaultModule = $this->config->getAppValue('core', 'default_encryption_module');
61+
$defaultModule = $this->appConfig->getValueString('core', 'default_encryption_module', '');
5062
if ($defaultModule === '') {
5163
$output->writeln('<error>No default module is set</error>');
5264
return 1;

0 commit comments

Comments
 (0)