Skip to content

Commit edc951c

Browse files
cuppettclaude
andcommitted
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) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
1 parent b4e000a commit edc951c

9 files changed

Lines changed: 83 additions & 145 deletions

File tree

apps/encryption/lib/Controller/RecoveryController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public function changeRecoveryPassword(string $newPassword, string $oldPassword,
124124
#[NoAdminRequired]
125125
public function userSetRecovery($userEnableRecovery) {
126126
if ($userEnableRecovery === '0' || $userEnableRecovery === '1') {
127-
$result = $this->recovery->setRecoveryForUser($userEnableRecovery);
127+
$result = $this->recovery->setRecoveryForUser($userEnableRecovery === '1');
128128

129129
if ($result) {
130130
if ($userEnableRecovery === '0') {

apps/encryption/lib/Recovery.php

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99

1010
use OC\Files\View;
1111
use OCA\Encryption\Crypto\Crypt;
12+
use OCP\Config\IUserConfig;
1213
use OCP\Encryption\IFile;
13-
use OCP\IConfig;
14+
use OCP\IAppConfig;
1415
use OCP\IUser;
1516
use OCP\IUserSession;
1617
use OCP\PreConditionNotMetException;
@@ -21,19 +22,12 @@ class Recovery {
2122
*/
2223
protected $user;
2324

24-
/**
25-
* @param IUserSession $userSession
26-
* @param Crypt $crypt
27-
* @param KeyManager $keyManager
28-
* @param IConfig $config
29-
* @param IFile $file
30-
* @param View $view
31-
*/
3225
public function __construct(
3326
IUserSession $userSession,
3427
protected Crypt $crypt,
3528
private KeyManager $keyManager,
36-
private IConfig $config,
29+
private IAppConfig $appConfig,
30+
private IUserConfig $userConfig,
3731
private IFile $file,
3832
private View $view,
3933
) {
@@ -45,10 +39,7 @@ public function __construct(
4539
* @return bool
4640
*/
4741
public function enableAdminRecovery($password) {
48-
$appConfig = $this->config;
49-
$keyManager = $this->keyManager;
50-
51-
if (!$keyManager->recoveryKeyExists()) {
42+
if (!$this->keyManager->recoveryKeyExists()) {
5243
$keyPair = $this->crypt->createKeyPair();
5344
if (!is_array($keyPair)) {
5445
return false;
@@ -57,8 +48,8 @@ public function enableAdminRecovery($password) {
5748
$this->keyManager->setRecoveryKey($password, $keyPair);
5849
}
5950

60-
if ($keyManager->checkRecoveryPassword($password)) {
61-
$appConfig->setAppValue('encryption', 'recoveryAdminEnabled', '1');
51+
if ($this->keyManager->checkRecoveryPassword($password)) {
52+
$this->appConfig->setValueBool('encryption', 'recoveryAdminEnabled', true);
6253
return true;
6354
}
6455

@@ -92,7 +83,7 @@ public function disableAdminRecovery($recoveryPassword) {
9283

9384
if ($keyManager->checkRecoveryPassword($recoveryPassword)) {
9485
// Set recoveryAdmin as disabled
95-
$this->config->setAppValue('encryption', 'recoveryAdminEnabled', '0');
86+
$this->appConfig->setValueBool('encryption', 'recoveryAdminEnabled', false);
9687
return true;
9788
}
9889
return false;
@@ -107,12 +98,7 @@ public function disableAdminRecovery($recoveryPassword) {
10798
*/
10899
public function isRecoveryEnabledForUser($user = '') {
109100
$uid = $user === '' ? $this->user->getUID() : $user;
110-
$recoveryMode = $this->config->getUserValue($uid,
111-
'encryption',
112-
'recoveryEnabled',
113-
0);
114-
115-
return ($recoveryMode === '1');
101+
return $this->userConfig->getValueBool($uid, 'encryption', 'recoveryEnabled');
116102
}
117103

118104
/**
@@ -121,23 +107,18 @@ public function isRecoveryEnabledForUser($user = '') {
121107
* @return bool
122108
*/
123109
public function isRecoveryKeyEnabled() {
124-
$enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', '0');
125-
126-
return ($enabled === '1');
110+
return $this->appConfig->getValueBool('encryption', 'recoveryAdminEnabled');
127111
}
128112

129113
/**
130-
* @param string $value
114+
* @param bool $value
131115
* @return bool
132116
*/
133-
public function setRecoveryForUser($value) {
117+
public function setRecoveryForUser(bool $value): bool {
134118
try {
135-
$this->config->setUserValue($this->user->getUID(),
136-
'encryption',
137-
'recoveryEnabled',
138-
$value);
119+
$this->userConfig->setValueBool($this->user->getUID(), 'encryption', 'recoveryEnabled', $value);
139120

140-
if ($value === '1') {
121+
if ($value) {
141122
$this->addRecoveryKeys('/' . $this->user->getUID() . '/files/');
142123
} else {
143124
$this->removeRecoveryKeys('/' . $this->user->getUID() . '/files/');

apps/encryption/lib/Settings/Admin.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCA\Encryption\Util;
1616
use OCP\AppFramework\Http\TemplateResponse;
1717
use OCP\AppFramework\Services\IInitialState;
18+
use OCP\Config\IUserConfig;
1819
use OCP\IAppConfig;
1920
use OCP\IConfig;
2021
use OCP\IL10N;
@@ -34,6 +35,7 @@ public function __construct(
3435
private ISession $session,
3536
private IInitialState $initialState,
3637
private IAppConfig $appConfig,
38+
private IUserConfig $userConfig,
3739
) {
3840
}
3941

@@ -52,8 +54,8 @@ public function getForm() {
5254
new View(),
5355
$crypt,
5456
$this->userSession,
55-
$this->config,
5657
$this->appConfig,
58+
$this->userConfig,
5759
$this->userManager);
5860

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

apps/encryption/lib/Util.php

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
use OC\Files\Storage\Storage;
1111
use OC\Files\View;
1212
use OCA\Encryption\Crypto\Crypt;
13+
use OCP\Config\IUserConfig;
1314
use OCP\Files\Storage\IStorage;
1415
use OCP\IAppConfig;
15-
use OCP\IConfig;
1616
use OCP\IUser;
1717
use OCP\IUserManager;
1818
use OCP\IUserSession;
@@ -25,8 +25,8 @@ public function __construct(
2525
private View $files,
2626
private Crypt $crypt,
2727
IUserSession $userSession,
28-
private IConfig $config,
2928
private IAppConfig $appConfig,
29+
private IUserConfig $userConfig,
3030
private IUserManager $userManager,
3131
) {
3232
$this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false;
@@ -39,12 +39,7 @@ public function __construct(
3939
* @return bool
4040
*/
4141
public function isRecoveryEnabledForUser($uid) {
42-
$recoveryMode = $this->config->getUserValue($uid,
43-
'encryption',
44-
'recoveryEnabled',
45-
'0');
46-
47-
return ($recoveryMode === '1');
42+
return $this->userConfig->getValueBool($uid, 'encryption', 'recoveryEnabled');
4843
}
4944

5045
/**
@@ -69,22 +64,16 @@ public function setEncryptHomeStorage(bool $encryptHomeStorage) {
6964
* check if master key is enabled
7065
*/
7166
public function isMasterKeyEnabled(): bool {
72-
$userMasterKey = $this->config->getAppValue('encryption', 'useMasterKey', '1');
73-
return ($userMasterKey === '1');
67+
return $this->appConfig->getValueBool('encryption', 'useMasterKey', true);
7468
}
7569

7670
/**
7771
* @param $enabled
7872
* @return bool
7973
*/
80-
public function setRecoveryForUser($enabled) {
81-
$value = $enabled ? '1' : '0';
82-
74+
public function setRecoveryForUser(bool $enabled): bool {
8375
try {
84-
$this->config->setUserValue($this->user->getUID(),
85-
'encryption',
86-
'recoveryEnabled',
87-
$value);
76+
$this->userConfig->setValueBool($this->user->getUID(), 'encryption', 'recoveryEnabled', $enabled);
8877
return true;
8978
} catch (PreConditionNotMetException $e) {
9079
return false;

apps/encryption/tests/Controller/RecoveryControllerTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ public static function userSetRecoveryProvider(): array {
114114
public function testUserSetRecovery($enableRecovery, $expectedMessage, $expectedStatus): void {
115115
$this->recoveryMock->expects($this->any())
116116
->method('setRecoveryForUser')
117-
->with($enableRecovery)
117+
->with($enableRecovery === '1')
118118
->willReturnMap([
119-
['1', true],
120-
['0', false]
119+
[true, true],
120+
[false, false]
121121
]);
122122

123123

apps/encryption/tests/RecoveryTest.php

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@
1414
use OCA\Encryption\Crypto\Crypt;
1515
use OCA\Encryption\KeyManager;
1616
use OCA\Encryption\Recovery;
17+
use OCP\Config\IUserConfig;
1718
use OCP\Encryption\IFile;
18-
use OCP\IConfig;
19+
use OCP\IAppConfig;
1920
use OCP\IUser;
2021
use OCP\IUserSession;
2122
use PHPUnit\Framework\MockObject\MockObject;
@@ -29,7 +30,8 @@ class RecoveryTest extends TestCase {
2930
private IUserSession&MockObject $userSessionMock;
3031
private IUser&MockObject $user;
3132
private KeyManager&MockObject $keyManagerMock;
32-
private IConfig&MockObject $configMock;
33+
private IAppConfig&MockObject $appConfigMock;
34+
private IUserConfig&MockObject $userConfigMock;
3335
private Crypt&MockObject $cryptMock;
3436

3537
private Recovery $instance;
@@ -56,7 +58,7 @@ public function testEnableAdminRecoverySuccessful(): void {
5658

5759
$this->assertTrue($this->instance->enableAdminRecovery('password'));
5860
$this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage);
59-
$this->assertEquals(1, self::$tempStorage['recoveryAdminEnabled']);
61+
$this->assertTrue(self::$tempStorage['recoveryAdminEnabled']);
6062

6163
$this->assertTrue($this->instance->enableAdminRecovery('password'));
6264
}
@@ -83,7 +85,7 @@ public function testEnableAdminRecoveryCouldNotCheckPassword(): void {
8385

8486
$this->assertTrue($this->instance->enableAdminRecovery('password'));
8587
$this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage);
86-
$this->assertEquals(1, self::$tempStorage['recoveryAdminEnabled']);
88+
$this->assertTrue(self::$tempStorage['recoveryAdminEnabled']);
8789

8890
$this->assertFalse($this->instance->enableAdminRecovery('password'));
8991
}
@@ -140,32 +142,32 @@ public function testDisableAdminRecovery(): void {
140142

141143
$this->assertArrayHasKey('recoveryAdminEnabled', self::$tempStorage);
142144
$this->assertTrue($this->instance->disableAdminRecovery('password'));
143-
$this->assertEquals(0, self::$tempStorage['recoveryAdminEnabled']);
145+
$this->assertFalse(self::$tempStorage['recoveryAdminEnabled']);
144146

145147
$this->assertFalse($this->instance->disableAdminRecovery('password'));
146148
}
147149

148150
public function testIsRecoveryEnabledForUser(): void {
149-
$this->configMock->expects($this->exactly(2))
150-
->method('getUserValue')
151-
->willReturnOnConsecutiveCalls('1', '0');
151+
$this->userConfigMock->expects($this->exactly(2))
152+
->method('getValueBool')
153+
->willReturnOnConsecutiveCalls(true, false);
152154

153155
$this->assertTrue($this->instance->isRecoveryEnabledForUser());
154156
$this->assertFalse($this->instance->isRecoveryEnabledForUser('admin'));
155157
}
156158

157159
public function testIsRecoveryKeyEnabled(): void {
158160
$this->assertFalse($this->instance->isRecoveryKeyEnabled());
159-
self::$tempStorage['recoveryAdminEnabled'] = '1';
161+
self::$tempStorage['recoveryAdminEnabled'] = true;
160162
$this->assertTrue($this->instance->isRecoveryKeyEnabled());
161163
}
162164

163165
public function testSetRecoveryFolderForUser(): void {
164166
$this->viewMock->expects($this->exactly(2))
165167
->method('getDirectoryContent')
166168
->willReturn([]);
167-
$this->assertTrue($this->instance->setRecoveryForUser(0));
168-
$this->assertTrue($this->instance->setRecoveryForUser('1'));
169+
$this->assertTrue($this->instance->setRecoveryForUser(false));
170+
$this->assertTrue($this->instance->setRecoveryForUser(true));
169171
}
170172

171173
public function testRecoverUserFiles(): void {
@@ -239,52 +241,41 @@ protected function setUp(): void {
239241

240242
$this->cryptMock = $this->getMockBuilder(Crypt::class)->disableOriginalConstructor()->getMock();
241243
$this->keyManagerMock = $this->getMockBuilder(KeyManager::class)->disableOriginalConstructor()->getMock();
242-
$this->configMock = $this->createMock(IConfig::class);
244+
$this->appConfigMock = $this->createMock(IAppConfig::class);
245+
$this->userConfigMock = $this->createMock(IUserConfig::class);
243246
$this->fileMock = $this->createMock(IFile::class);
244247
$this->viewMock = $this->createMock(View::class);
245248

246-
$this->configMock->expects($this->any())
247-
->method('setAppValue')
248-
->willReturnCallback([$this, 'setValueTester']);
249+
$this->appConfigMock->expects($this->any())
250+
->method('setValueBool')
251+
->willReturnCallback(function (string $app, string $key, bool $value): bool {
252+
self::$tempStorage[$key] = $value;
253+
return true;
254+
});
249255

250-
$this->configMock->expects($this->any())
251-
->method('getAppValue')
256+
$this->appConfigMock->expects($this->any())
257+
->method('getValueBool')
252258
->willReturnCallback([$this, 'getValueTester']);
253259

254260
$this->instance = new Recovery($this->userSessionMock,
255261
$this->cryptMock,
256262
$this->keyManagerMock,
257-
$this->configMock,
263+
$this->appConfigMock,
264+
$this->userConfigMock,
258265
$this->fileMock,
259266
$this->viewMock);
260267
}
261268

262269

263-
/**
264-
* @param $app
265-
* @param $key
266-
* @param $value
267-
*/
268-
public function setValueTester($app, $key, $value) {
270+
public function setValueTester(string $app, string $key, bool $value): void {
269271
self::$tempStorage[$key] = $value;
270272
}
271273

272-
/**
273-
* @param $key
274-
*/
275-
public function removeValueTester($key) {
274+
public function removeValueTester(string $key): void {
276275
unset(self::$tempStorage[$key]);
277276
}
278277

279-
/**
280-
* @param $app
281-
* @param $key
282-
* @return mixed
283-
*/
284-
public function getValueTester($app, $key) {
285-
if (!empty(self::$tempStorage[$key])) {
286-
return self::$tempStorage[$key];
287-
}
288-
return null;
278+
public function getValueTester(string $app, string $key, bool $default = false): bool {
279+
return self::$tempStorage[$key] ?? $default;
289280
}
290281
}

0 commit comments

Comments
 (0)