From 8601d6352208f4e4a3b06be9242080e21b79825e Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sun, 1 Sep 2024 17:52:51 +0200 Subject: [PATCH] chore: Migrate deprecated app config access to `IAppConfig` Signed-off-by: Ferdinand Thiessen --- lib/AppInfo/Application.php | 4 +- lib/PasswordPolicyConfig.php | 110 ++++++++----------------- tests/lib/PasswordPolicyConfigTest.php | 57 +++++++++---- 3 files changed, 78 insertions(+), 93 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 91529827..c813f1b5 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -29,8 +29,10 @@ use OCP\User\Events\UserLoggedInEvent; class Application extends App implements IBootstrap { + public const APP_ID = 'password_policy'; + public function __construct() { - parent::__construct('password_policy'); + parent::__construct(self::APP_ID); } public function register(IRegistrationContext $context): void { diff --git a/lib/PasswordPolicyConfig.php b/lib/PasswordPolicyConfig.php index 6c0bf676..c00fce06 100644 --- a/lib/PasswordPolicyConfig.php +++ b/lib/PasswordPolicyConfig.php @@ -6,9 +6,10 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ - namespace OCA\Password_Policy; +use OCA\Password_Policy\AppInfo\Application; +use OCP\IAppConfig; use OCP\IConfig; /** @@ -20,129 +21,93 @@ */ class PasswordPolicyConfig { - /** @var IConfig */ - private $config; - /** * Config constructor. - * - * @param IConfig $config */ - public function __construct(IConfig $config) { - $this->config = $config; + public function __construct( + private IConfig $config, + private IAppConfig $appConfig, + ) { } /** * get the enforced minimum length of passwords - * - * @return int */ public function getMinLength(): int { - return (int)$this->config->getAppValue('password_policy', 'minLength', '10'); + return $this->appConfig->getValueInt(Application::APP_ID, 'minLength', 10); } /** * Whether non-common passwords should be enforced - * - * @return bool */ public function getEnforceNonCommonPassword(): bool { - $enforceNonCommonPasswords = $this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueBool( + Application::APP_ID, 'enforceNonCommonPassword', - '1' + true ); - return $enforceNonCommonPasswords === '1'; } /** * does the password need to contain upper and lower case characters - * - * @return bool */ public function getEnforceUpperLowerCase(): bool { - $enforceUpperLowerCase = $this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueBool( + Application::APP_ID, 'enforceUpperLowerCase', - '0' ); - - return $enforceUpperLowerCase === '1'; } /** * does the password need to contain numeric characters - * - * @return bool */ public function getEnforceNumericCharacters(): bool { - $enforceNumericCharacters = $this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueBool( + Application::APP_ID, 'enforceNumericCharacters', - '0' ); - - return $enforceNumericCharacters === '1'; } /** * does the password need to contain special characters - * - * @return bool */ public function getEnforceSpecialCharacters(): bool { - $enforceSpecialCharacters = $this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueBool( + Application::APP_ID, 'enforceSpecialCharacters', - '0' ); - - return $enforceSpecialCharacters === '1'; } /** * set minimal length of passwords - * - * @param int $minLength */ - public function setMinLength(int $minLength) { - $this->config->setAppValue('password_policy', 'minLength', $minLength); + public function setMinLength(int $minLength): void { + $this->appConfig->setValueInt(Application::APP_ID, 'minLength', $minLength); } /** * enforce upper and lower case characters - * - * @param bool $enforceUpperLowerCase */ - public function setEnforceUpperLowerCase(bool $enforceUpperLowerCase) { - $value = $enforceUpperLowerCase === true ? '1' : '0'; - $this->config->setAppValue('password_policy', 'enforceUpperLowerCase', $value); + public function setEnforceUpperLowerCase(bool $enforceUpperLowerCase): void { + $this->appConfig->setValueBool(Application::APP_ID, 'enforceUpperLowerCase', $enforceUpperLowerCase); } /** * enforce numeric characters - * - * @param bool $enforceNumericCharacters */ - public function setEnforceNumericCharacters(bool $enforceNumericCharacters) { - $value = $enforceNumericCharacters === true ? '1' : '0'; - $this->config->setAppValue('password_policy', 'enforceNumericCharacters', $value); + public function setEnforceNumericCharacters(bool $enforceNumericCharacters): void { + $this->appConfig->setValueBool(Application::APP_ID, 'enforceNumericCharacters', $enforceNumericCharacters); } /** * enforce special characters - * - * @param bool $enforceSpecialCharacters */ - public function setEnforceSpecialCharacters(bool $enforceSpecialCharacters) { - $value = $enforceSpecialCharacters === true ? '1' : '0'; - $this->config->setAppValue('password_policy', 'enforceSpecialCharacters', $value); + public function setEnforceSpecialCharacters(bool $enforceSpecialCharacters): void { + $this->appConfig->setValueBool(Application::APP_ID, 'enforceSpecialCharacters', $enforceSpecialCharacters); } /** * Do we check against the HaveIBeenPwned passwords - * - * @return bool */ public function getEnforceHaveIBeenPwned(): bool { $hasInternetConnection = $this->config->getSystemValue('has_internet_connection', true); @@ -150,11 +115,11 @@ public function getEnforceHaveIBeenPwned(): bool { return false; } - return $this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueBool( + Application::APP_ID, 'enforceHaveIBeenPwned', - '1' - ) === '1'; + true, + ); } /** @@ -162,23 +127,21 @@ public function getEnforceHaveIBeenPwned(): bool { * * @param bool $enforceHaveIBeenPwned */ - public function setEnforceHaveIBeenPwned(bool $enforceHaveIBeenPwned) { - $this->config->setAppValue('password_policy', 'enforceHaveIBeenPwned', $enforceHaveIBeenPwned ? '1' : '0'); + public function setEnforceHaveIBeenPwned(bool $enforceHaveIBeenPwned): void { + $this->appConfig->setValueBool(Application::APP_ID, 'enforceHaveIBeenPwned', $enforceHaveIBeenPwned); } public function getHistorySize(): int { - return (int)$this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueInt( + Application::APP_ID, 'historySize', - 0 ); } public function getExpiryInDays(): int { - return (int)$this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueInt( + Application::APP_ID, 'expiration', - 0 ); } @@ -186,10 +149,9 @@ public function getExpiryInDays(): int { * @return int if 0 then there is no limit */ public function getMaximumLoginAttempts(): int { - return (int)$this->config->getAppValue( - 'password_policy', + return $this->appConfig->getValueInt( + Application::APP_ID, 'maximumLoginAttempts', - 0 ); } } diff --git a/tests/lib/PasswordPolicyConfigTest.php b/tests/lib/PasswordPolicyConfigTest.php index 38a94420..4d928c66 100644 --- a/tests/lib/PasswordPolicyConfigTest.php +++ b/tests/lib/PasswordPolicyConfigTest.php @@ -8,27 +8,32 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Password_Policy\PasswordPolicyConfig; +use OCP\IAppConfig; use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; class PasswordPolicyConfigTest extends TestCase { private IConfig&MockObject $config; + private IAppConfig&MockObject $appConfig; private PasswordPolicyConfig $instance; protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); - $this->instance = new PasswordPolicyConfig($this->config); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->instance = new PasswordPolicyConfig($this->config, $this->appConfig); } public function testGetMinLength() { - $appConfigValue = '42'; + $appConfigValue = 42; $expected = 42; - $this->config->expects($this->once())->method('getAppValue') - ->with('password_policy', 'minLength', '10') + $this->appConfig + ->expects(self::once()) + ->method('getValueInt') + ->with('password_policy', 'minLength', 10) ->willReturn($appConfigValue); $this->assertSame($expected, @@ -39,8 +44,10 @@ public function testGetMinLength() { * @dataProvider configTestData */ public function testGetEnforceNonCommonPassword($appConfigValue, $expected) { - $this->config->expects($this->once())->method('getAppValue') - ->with('password_policy', 'enforceNonCommonPassword', '1') + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('password_policy', 'enforceNonCommonPassword', true) ->willReturn($appConfigValue); $this->assertSame($expected, @@ -52,8 +59,10 @@ public function testGetEnforceNonCommonPassword($appConfigValue, $expected) { * @dataProvider configTestData */ public function testGetEnforceUpperLowerCase($appConfigValue, $expected) { - $this->config->expects($this->once())->method('getAppValue') - ->with('password_policy', 'enforceUpperLowerCase', '0') + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('password_policy', 'enforceUpperLowerCase', false) ->willReturn($appConfigValue); $this->assertSame($expected, @@ -65,8 +74,10 @@ public function testGetEnforceUpperLowerCase($appConfigValue, $expected) { * @dataProvider configTestData */ public function testGetEnforceNumericCharacters($appConfigValue, $expected) { - $this->config->expects($this->once())->method('getAppValue') - ->with('password_policy', 'enforceNumericCharacters', '0') + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('password_policy', 'enforceNumericCharacters', false) ->willReturn($appConfigValue); $this->assertSame($expected, @@ -79,8 +90,10 @@ public function testGetEnforceNumericCharacters($appConfigValue, $expected) { * @dataProvider configTestData */ public function testGetEnforceSpecialCharacters($appConfigValue, $expected) { - $this->config->expects($this->once())->method('getAppValue') - ->with('password_policy', 'enforceSpecialCharacters', '0') + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('password_policy', 'enforceSpecialCharacters', false) ->willReturn($appConfigValue); $this->assertSame($expected, @@ -91,7 +104,9 @@ public function testGetEnforceSpecialCharacters($appConfigValue, $expected) { public function testSetMinLength() { $expected = 42; - $this->config->expects($this->once())->method('setAppValue') + $this->appConfig + ->expects(self::once()) + ->method('setValueInt') ->with('password_policy', 'minLength', $expected); $this->instance->setMinLength($expected); @@ -101,7 +116,9 @@ public function testSetMinLength() { * @dataProvider configTestData */ public function testSetEnforceUpperLowerCase($expected, $setValue) { - $this->config->expects($this->once())->method('setAppValue') + $this->appConfig + ->expects(self::once()) + ->method('setValueBool') ->with('password_policy', 'enforceUpperLowerCase', $expected); $this->instance->setEnforceUpperLowerCase($setValue); @@ -111,7 +128,9 @@ public function testSetEnforceUpperLowerCase($expected, $setValue) { * @dataProvider configTestData */ public function testSetEnforceNumericCharacters($expected, $setValue) { - $this->config->expects($this->once())->method('setAppValue') + $this->appConfig + ->expects(self::once()) + ->method('setValueBool') ->with('password_policy', 'enforceNumericCharacters', $expected); $this->instance->setEnforceNumericCharacters($setValue); @@ -122,7 +141,9 @@ public function testSetEnforceNumericCharacters($expected, $setValue) { * @dataProvider configTestData */ public function testSetEnforceSpecialCharacters($expected, $setValue) { - $this->config->expects($this->once())->method('setAppValue') + $this->appConfig + ->expects(self::once()) + ->method('setValueBool') ->with('password_policy', 'enforceSpecialCharacters', $expected); $this->instance->setEnforceSpecialCharacters($setValue); @@ -130,8 +151,8 @@ public function testSetEnforceSpecialCharacters($expected, $setValue) { public static function configTestData() { return [ - ['1', true], - ['0', false] + [true, true], + [false, false] ]; } }