Skip to content

Commit

Permalink
chore: Migrate deprecated app config access to IAppConfig
Browse files Browse the repository at this point in the history 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
@susnux
susnux committed Sep 1, 2024
1 parent c71d039 commit aadc112
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 93 deletions.
4 changes: 3 additions & 1 deletion lib/AppInfo/Application.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
110 changes: 36 additions & 74 deletions lib/PasswordPolicyConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -20,176 +21,137 @@
*/
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);
if (!$hasInternetConnection) {
return false;
}

return $this->config->getAppValue(
'password_policy',
return $this->appConfig->getValueBool(
Application::APP_ID,
'enforceHaveIBeenPwned',
'1'
) === '1';
true,
);
}

/**
* Enforce checking against haveibeenpwned.com
*
* @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
);
}

/**
* @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
);
}
}
57 changes: 39 additions & 18 deletions tests/lib/PasswordPolicyConfigTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -122,16 +141,18 @@ 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);
}

public static function configTestData() {
return [
['1', true],
['0', false]
[true, true],
[false, false]
];
}
}

0 comments on commit aadc112

Please sign in to comment.