diff --git a/apps/encryption/lib/Controller/RecoveryController.php b/apps/encryption/lib/Controller/RecoveryController.php index 66e5d6308c862..f24490dfc5797 100644 --- a/apps/encryption/lib/Controller/RecoveryController.php +++ b/apps/encryption/lib/Controller/RecoveryController.php @@ -118,37 +118,31 @@ public function changeRecoveryPassword(string $newPassword, string $oldPassword, } } - /** - * @param string $userEnableRecovery - * @return DataResponse - */ #[NoAdminRequired] - public function userSetRecovery($userEnableRecovery) { - if ($userEnableRecovery === '0' || $userEnableRecovery === '1') { - $result = $this->recovery->setRecoveryForUser($userEnableRecovery); + public function userSetRecovery(bool $userEnableRecovery): DataResponse { + $result = $this->recovery->setRecoveryForUser($userEnableRecovery); - if ($result) { - if ($userEnableRecovery === '0') { - return new DataResponse( - [ - 'data' => [ - 'message' => $this->l->t('Recovery Key disabled')] - ] - ); - } + if ($result) { + if (!$userEnableRecovery) { return new DataResponse( [ 'data' => [ - 'message' => $this->l->t('Recovery Key enabled')] + 'message' => $this->l->t('Recovery Key disabled')] ] ); } + return new DataResponse( + [ + 'data' => [ + 'message' => $this->l->t('Recovery Key enabled')] + ] + ); } + return new DataResponse( [ 'data' => [ - 'message' => $this->l->t('Could not enable the recovery key, please try again or contact your administrator') - ] + 'message' => $this->l->t('Could not enable the recovery key, please try again or contact your administrator')] ], Http::STATUS_BAD_REQUEST); } } diff --git a/apps/encryption/lib/Recovery.php b/apps/encryption/lib/Recovery.php index 7dcd2ca330e23..74b0b6f888134 100644 --- a/apps/encryption/lib/Recovery.php +++ b/apps/encryption/lib/Recovery.php @@ -17,19 +17,8 @@ use OCP\PreConditionNotMetException; class Recovery { - /** - * @var null|IUser - */ - protected $user; + protected ?IUser $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, @@ -41,11 +30,7 @@ public function __construct( $this->user = ($userSession->isLoggedIn()) ? $userSession->getUser() : null; } - /** - * @param string $password - * @return bool - */ - public function enableAdminRecovery($password) { + public function enableAdminRecovery(string $password): bool { $appConfig = $this->config; $keyManager = $this->keyManager; @@ -84,11 +69,7 @@ public function changeRecoveryKeyPassword(string $newPassword, string $oldPasswo return false; } - /** - * @param string $recoveryPassword - * @return bool - */ - public function disableAdminRecovery($recoveryPassword) { + public function disableAdminRecovery(string $recoveryPassword): bool { $keyManager = $this->keyManager; if ($keyManager->checkRecoveryPassword($recoveryPassword)) { @@ -103,42 +84,34 @@ public function disableAdminRecovery($recoveryPassword) { * check if recovery is enabled for user * * @param string $user if no user is given we check the current logged-in user - * - * @return bool */ - public function isRecoveryEnabledForUser($user = '') { + public function isRecoveryEnabledForUser(string $user = ''): bool { $uid = $user === '' ? $this->user->getUID() : $user; $recoveryMode = $this->config->getUserValue($uid, 'encryption', 'recoveryEnabled', - 0); + '0'); return ($recoveryMode === '1'); } /** * check if recovery is key is enabled by the administrator - * - * @return bool */ - public function isRecoveryKeyEnabled() { + public function isRecoveryKeyEnabled(): bool { $enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', '0'); return ($enabled === '1'); } - /** - * @param string $value - * @return bool - */ - public function setRecoveryForUser($value) { + public function setRecoveryForUser(bool $value): bool { try { $this->config->setUserValue($this->user->getUID(), 'encryption', 'recoveryEnabled', - $value); + $value ? '1' : '0'); - if ($value === '1') { + if ($value) { $this->addRecoveryKeys('/' . $this->user->getUID() . '/files/'); } else { $this->removeRecoveryKeys('/' . $this->user->getUID() . '/files/'); diff --git a/apps/encryption/lib/Util.php b/apps/encryption/lib/Util.php index fbe98767f47ea..a5dd226f28398 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -83,11 +83,7 @@ public function isMasterKeyEnabled(): bool { return ($userMasterKey === '1'); } - /** - * @param $enabled - * @return bool - */ - public function setRecoveryForUser($enabled) { + public function setRecoveryForUser(bool $enabled): bool { $value = $enabled ? '1' : '0'; try { diff --git a/apps/encryption/tests/Controller/RecoveryControllerTest.php b/apps/encryption/tests/Controller/RecoveryControllerTest.php index 5bbe60ddce8c1..702ab5ea4675b 100644 --- a/apps/encryption/tests/Controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/Controller/RecoveryControllerTest.php @@ -100,24 +100,19 @@ public function testChangeRecoveryPassword($password, $confirmPassword, $oldPass public static function userSetRecoveryProvider(): array { return [ - ['1', 'Recovery Key enabled', Http::STATUS_OK], - ['0', 'Could not enable the recovery key, please try again or contact your administrator', Http::STATUS_BAD_REQUEST] + [true, 'Recovery Key enabled', Http::STATUS_OK], + [false, 'Could not enable the recovery key, please try again or contact your administrator', Http::STATUS_BAD_REQUEST] ]; } - /** - * @param $enableRecovery - * @param $expectedMessage - * @param $expectedStatus - */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'userSetRecoveryProvider')] - public function testUserSetRecovery($enableRecovery, $expectedMessage, $expectedStatus): void { + public function testUserSetRecovery(bool $enableRecovery, string $expectedMessage, int $expectedStatus): void { $this->recoveryMock->expects($this->any()) ->method('setRecoveryForUser') ->with($enableRecovery) ->willReturnMap([ - ['1', true], - ['0', false] + [true, true], + [false, false] ]); $response = $this->controller->userSetRecovery($enableRecovery); diff --git a/apps/encryption/tests/RecoveryTest.php b/apps/encryption/tests/RecoveryTest.php index 19d7a5ecd63a0..fcdedadd1748b 100644 --- a/apps/encryption/tests/RecoveryTest.php +++ b/apps/encryption/tests/RecoveryTest.php @@ -164,8 +164,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 { diff --git a/apps/encryption/tests/UtilTest.php b/apps/encryption/tests/UtilTest.php index 757a1b092b4f1..0129f2ace67ec 100644 --- a/apps/encryption/tests/UtilTest.php +++ b/apps/encryption/tests/UtilTest.php @@ -33,7 +33,7 @@ class UtilTest extends TestCase { protected IMountPoint&MockObject $mountMock; public function testSetRecoveryForUser(): void { - $this->instance->setRecoveryForUser('1'); + $this->instance->setRecoveryForUser(true); $this->assertArrayHasKey('recoveryEnabled', self::$tempStorage); } diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php index 375254470f007..3c1fcd0a9998a 100644 --- a/apps/settings/lib/Controller/ChangePasswordController.php +++ b/apps/settings/lib/Controller/ChangePasswordController.php @@ -173,20 +173,22 @@ public function changeUserPassword(?string $username = null, ?string $password = ], ]); } - if (!$result && $recoveryEnabledForUser) { - return new JSONResponse([ - 'status' => 'error', - 'data' => [ - 'message' => $this->l->t('Backend does not support password change, but the encryption of the account key was updated.'), - ] - ]); - } elseif (!$result && !$recoveryEnabledForUser) { - return new JSONResponse([ - 'status' => 'error', - 'data' => [ - 'message' => $this->l->t('Unable to change password'), - ] - ]); + if (!$result) { + if ($recoveryEnabledForUser) { + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $this->l->t('Backend does not support password change, but the encryption of the account key was updated.'), + ] + ]); + } else { + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $this->l->t('Unable to change password'), + ] + ]); + } } } } else {