Apply suggestions from code review

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>
This commit is contained in:
Jolien Trog 2026-03-20 15:10:23 +01:00 committed by GitHub
parent 2c197ebabe
commit abef7e6466
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 37 additions and 30 deletions

View file

@ -177,12 +177,14 @@ Icinga Web provides a built-in policy called `Common` with the following require
You can create custom password policies by developing a module with a provided hook.
**Create Module Structure**
```bash
mkdir -p /usr/share/icingaweb2/modules/mypasswordpolicy/library/Mypasswordpolicy/ProvidedHook
cd /usr/share/icingaweb2/modules/mypasswordpolicy
```
Create `module.info`:
```ini
Module: mypasswordpolicy
Name: My Password Policy
@ -259,8 +261,6 @@ Enable the module:
icingacli module enable mypasswordpolicy
```
You can choose in the settings the preferred password policy.
The custom policy will now appear in **Configuration > Application > General** under Password Policy.
## Groups <a id="authentication-configuration-groups"></a>

View file

@ -11,9 +11,7 @@ use Icinga\Authentication\PasswordPolicy;
*/
abstract class PasswordPolicyHook implements PasswordPolicy
{
/**
* Hook name
*/
/** @var string Hook name *(
protected const HOOK_NAME = 'PasswordPolicy';
/**
@ -29,7 +27,7 @@ abstract class PasswordPolicyHook implements PasswordPolicy
/**
* Return all registered password policies sorted by name
*
* @return array
* @return array<string, string>
*/
public static function all(): array
{

View file

@ -1,5 +1,7 @@
<?php
/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */
// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later
namespace Icinga\Application\ProvidedHook;

View file

@ -1,5 +1,7 @@
<?php
/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */
// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later
namespace Icinga\Authentication;
@ -28,9 +30,10 @@ interface PasswordPolicy
* Validate a given password against the defined policy
*
* @param string $newPassword
* @param string|null $oldPassword
* @param ?string $oldPassword
*
* @return string[] Returns an empty array if the password is valid,
* otherwise returns error messages describing the violations
* otherwise returns error messages describing the violations
*/
public function validate(string $newPassword, ?string $oldPassword = null): array;
}

View file

@ -15,19 +15,13 @@ use LogicException;
class PasswordPolicyHelper
{
/*
* Default class for password policy
*/
/** @var class-string<PasswordPolicy> Default password policy class */
const DEFAULT_PASSWORD_POLICY = AnyPasswordPolicy::class;
/**
* Configuration section for password policy in the configuration file
*/
/** @var string INI configuration section for password policy */
const CONFIG_SECTION = 'global';
/**
* Configuration key for password policy in the configuration file
*/
/** @var string INI configuration key for password policy */
const CONFIG_KEY = 'password_policy';
/**
@ -42,6 +36,7 @@ class PasswordPolicyHelper
if ($oldPasswordElementName !== null && $form->getElement($oldPasswordElementName) === null) {
throw new LogicException();
}
try {
$passwordPolicyClass = Config::app()->get(
'global',
@ -83,10 +78,16 @@ class PasswordPolicyHelper
);
}
}
/**
* Create and validate a password policy instance from the given class name and ensure it implements the
* password policy interface.
*/
/**
* Create a {@link PasswordPolicy} instance from the given class name
*
* @param class-string<PasswordPolicy> $passwordPolicyClass
*
* @return PasswordPolicy
*
* @throws ConfigurationError If class does not exist or does not implement {@link PasswordPolicy}
*/
public static function createPolicy(string $passwordPolicyClass): PasswordPolicy
{
if ($passwordPolicyClass === static::DEFAULT_PASSWORD_POLICY) {

View file

@ -1,14 +1,17 @@
<?php
/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */
// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later
namespace Icinga\Authentication;
use Zend_Validate_Abstract;
/**
* Use the provided password policy to validate the new password.
* Validate passwords against a configured policy
*
* Optionally, retrieve the old password from the form context using the configured form element name
* and pass it to the policy for comparative validation.
* and pass it to the policy for validation.
* Delegate all validation logic to the policy instance and expose any returned violation messages.
*/
class PasswordPolicyValidator extends Zend_Validate_Abstract
@ -23,7 +26,7 @@ class PasswordPolicyValidator extends Zend_Validate_Abstract
* Create a new PasswordPolicyValidator
*
* @param PasswordPolicy $passwordPolicy
* @param string|null $oldPasswordElementName
* @param ?string $oldPasswordElementName
*/
public function __construct(PasswordPolicy $passwordPolicy, ?string $oldPasswordElementName = null)
{

View file

@ -75,14 +75,14 @@ class CommonPasswordPolicyTest extends TestCase
public function testValidatePasswordWithLengthAndUpperCaseLetters(): void
{
$expectedResult = [
$expected = [
'Password must be at least 12 characters long',
'Password must contain at least one number',
'Password must contain at least one special character',
'Password must contain at least one lowercase letter',
'Password must contain at least one lowercase letter'
];
$this->assertSame(
$expectedResult,
$expected,
$this->instance->validate('ICINGAADMIN')
);
}