Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Migrate deprecated app config access to IAppConfig #650

Merged
merged 1 commit into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/AppInfo/Application.php
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
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
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]
];
}
}
Loading