From 828440eb0f9f7ab83ae6e7594626468ba60d234c Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 26 Aug 2025 10:48:56 +0200 Subject: [PATCH 01/54] Implement password policy with hook --- .../forms/Account/ChangePasswordForm.php | 34 ++++++--- .../General/PasswordPolicyConfigForm.php | 66 +++++++++++++++++ .../forms/Config/GeneralConfigForm.php | 3 + application/forms/Config/User/UserForm.php | 12 ++- .../Application/ApplicationBootstrap.php | 2 + .../Application/Hook/PasswordPolicyHook.php | 30 ++++++++ .../ProvidedHook/DefaultPasswordPolicy.php | 73 +++++++++++++++++++ .../Authentication/PasswordValidator.php | 41 +++++++++++ 8 files changed, 250 insertions(+), 11 deletions(-) create mode 100644 application/forms/Config/General/PasswordPolicyConfigForm.php create mode 100644 library/Icinga/Application/Hook/PasswordPolicyHook.php create mode 100644 library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php create mode 100644 library/Icinga/Authentication/PasswordValidator.php diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 5bca11cc86..10298dc194 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -3,11 +3,15 @@ namespace Icinga\Forms\Account; +use Icinga\Application\Config; +use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; use Icinga\User; use Icinga\Web\Form; use Icinga\Web\Notification; +use ipl\Html\Text; +use Icinga\Forms\Config\GeneralConfigForm; /** * Form for changing user passwords @@ -29,40 +33,50 @@ public function init() $this->setSubmitLabel($this->translate('Update Account')); } + /** * {@inheritdoc} + * @throws \Zend_Validate_Exception */ public function createElements(array $formData) { + $passwordPolicy = Config::app()->get('global', 'password_policy'); + if(isset($passwordPolicy) && class_exists($passwordPolicy)) { + $passwordPolicyObject = new $passwordPolicy(); + $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); + } + $this->addElement( 'password', 'old_password', array( - 'label' => $this->translate('Old Password'), - 'required' => true + 'label' => $this->translate('Old Password'), + 'required' => true ) ); $this->addElement( 'password', 'new_password', array( - 'label' => $this->translate('New Password'), - 'required' => true + 'label' => $this->translate('New Password'), + 'required' => true, + 'validators' => array(new PasswordValidator()) ) ); $this->addElement( 'password', 'new_password_confirmation', array( - 'label' => $this->translate('Confirm New Password'), - 'required' => true, - 'validators' => array( + 'label' => $this->translate('Confirm New Password'), + 'required' => true, + 'validators' => array( array('identical', false, array('new_password')) ) ) ); } + /** * {@inheritdoc} */ @@ -83,13 +97,13 @@ public function onSuccess() public function isValid($formData) { $valid = parent::isValid($formData); - if (! $valid) { + if (!$valid) { return false; } $oldPasswordEl = $this->getElement('old_password'); - if (! $this->backend->authenticate($this->Auth()->getUser(), $oldPasswordEl->getValue())) { + if (!$this->backend->authenticate($this->Auth()->getUser(), $oldPasswordEl->getValue())) { $oldPasswordEl->addError($this->translate('Old password is invalid')); $this->markAsError(); return false; @@ -111,7 +125,7 @@ public function getBackend() /** * Set the user backend * - * @param DbUserBackend $backend + * @param DbUserBackend $backend * * @return $this */ diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php new file mode 100644 index 0000000000..ba7960e6af --- /dev/null +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -0,0 +1,66 @@ +setName('form_config_general_password_policy'); + } + + /** + * {@inheritdoc} + * + * @return $this + */ + public function createElements(array $formData) + { + $this->addElement( + 'checkbox', + 'global_password_policy', + array( + 'label' => $this->translate('Password Policy'), + 'value' => true, + 'description' => $this->translate( + 'Enforce strong password requirements for new passwords' + ), + ) + ); + + $passwordPolicies = []; + + foreach (Hook::all('passwordpolicy') as $class => $policy) { + $passwordPolicies[$class] = $policy->getName(); + } + + asort($passwordPolicies); + $this->addElement( + 'select', + 'global_password_policy', + array( + 'description' => $this->translate('Enforce strong '. + 'password requirements for new passwords'), + 'label' => $this->translate('Password Policy'), + 'multiOptions' => array_merge( + ['' => sprintf(' - %s - ', + $this->translate('No Password Policy'))], + $passwordPolicies + ), + ) + ); + + return $this; + } +} diff --git a/application/forms/Config/GeneralConfigForm.php b/application/forms/Config/GeneralConfigForm.php index 5f15512a5a..9ec5ad552d 100644 --- a/application/forms/Config/GeneralConfigForm.php +++ b/application/forms/Config/GeneralConfigForm.php @@ -7,6 +7,7 @@ use Icinga\Forms\Config\General\DefaultAuthenticationDomainConfigForm; use Icinga\Forms\Config\General\LoggingConfigForm; use Icinga\Forms\Config\General\ThemingConfigForm; +use Icinga\Forms\Config\General\PasswordPolicyConfigForm; use Icinga\Forms\ConfigForm; /** @@ -32,9 +33,11 @@ public function createElements(array $formData) $loggingConfigForm = new LoggingConfigForm(); $themingConfigForm = new ThemingConfigForm(); $domainConfigForm = new DefaultAuthenticationDomainConfigForm(); + $passwordPolicyConfigForm = new PasswordPolicyConfigForm(); $this->addSubForm($appConfigForm->create($formData)); $this->addSubForm($loggingConfigForm->create($formData)); $this->addSubForm($themingConfigForm->create($formData)); $this->addSubForm($domainConfigForm->create($formData)); + $this->addSubForm($passwordPolicyConfigForm->create($formData)); } } diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index fb2ef4dc36..a2db3ab201 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -3,7 +3,9 @@ namespace Icinga\Forms\Config\User; +use Icinga\Application\Config; use Icinga\Application\Hook\ConfigFormEventsHook; +use Icinga\Authentication\PasswordValidator; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; use Icinga\Web\Notification; @@ -15,8 +17,15 @@ class UserForm extends RepositoryForm * * @param array $formData The data sent by the user */ + protected function createInsertElements(array $formData) { + $passwordPolicy = Config::app()->get('global', 'password_policy'); + if(isset($passwordPolicy) && class_exists($passwordPolicy)) { + $passwordPolicyObject = new $passwordPolicy(); + $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); + } + $this->addElement( 'checkbox', 'is_active', @@ -39,7 +48,8 @@ protected function createInsertElements(array $formData) 'password', array( 'required' => true, - 'label' => $this->translate('Password') + 'label' => $this->translate('Password'), + 'validators' => array(new PasswordValidator()) ) ); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 32abeaabb0..1537917df5 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -7,6 +7,7 @@ use ErrorException; use Exception; use Icinga\Application\ProvidedHook\DbMigration; +use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; @@ -740,6 +741,7 @@ public function hasLocales() protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); + Hook::register('passwordpolicy', DefaultPasswordPolicy::class, DefaultPasswordPolicy::class); return $this; } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php new file mode 100644 index 0000000000..86b7db081e --- /dev/null +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -0,0 +1,30 @@ +_messages = []; + $passwordPolicy = Config::app() + ->get('global', 'password_policy'); + + if (! isset($passwordPolicy) || ! class_exists($passwordPolicy)) { + return true; + } + + $passwordPolicyObject = new $passwordPolicy(); + $errorMessage = $passwordPolicyObject->validatePassword($value); + + if ($errorMessage != null) { + $this->_messages[] = $errorMessage; + return false; + } + + return true; + } +} From 3e7a2cf1de91686ad3e0e40b5c0c93c1a48be070 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 26 Aug 2025 11:37:50 +0200 Subject: [PATCH 02/54] fix code sniffer violations --- application/forms/Account/ChangePasswordForm.php | 2 +- .../forms/Config/General/PasswordPolicyConfigForm.php | 9 +++++---- application/forms/Config/User/UserForm.php | 2 +- library/Icinga/Application/Hook/PasswordPolicyHook.php | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 10298dc194..8c3f3ebde2 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -41,7 +41,7 @@ public function init() public function createElements(array $formData) { $passwordPolicy = Config::app()->get('global', 'password_policy'); - if(isset($passwordPolicy) && class_exists($passwordPolicy)) { + if (isset($passwordPolicy) && class_exists($passwordPolicy)) { $passwordPolicyObject = new $passwordPolicy(); $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); } diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index ba7960e6af..8406f1b995 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -50,14 +50,15 @@ public function createElements(array $formData) 'select', 'global_password_policy', array( - 'description' => $this->translate('Enforce strong '. - 'password requirements for new passwords'), + 'description' => $this->translate( + 'Enforce strong password requirements for new passwords'), 'label' => $this->translate('Password Policy'), 'multiOptions' => array_merge( ['' => sprintf(' - %s - ', - $this->translate('No Password Policy'))], + $this->translate('No Password Policy') + )], $passwordPolicies - ), + ), ) ); diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index a2db3ab201..9b490db642 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -21,7 +21,7 @@ class UserForm extends RepositoryForm protected function createInsertElements(array $formData) { $passwordPolicy = Config::app()->get('global', 'password_policy'); - if(isset($passwordPolicy) && class_exists($passwordPolicy)) { + if (isset($passwordPolicy) && class_exists($passwordPolicy)) { $passwordPolicyObject = new $passwordPolicy(); $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 86b7db081e..133214bbcf 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -26,5 +26,4 @@ public function displayPasswordPolicy(): string; * otherwise returns an error message describing the violations */ public function validatePassword(string $password): ?string; - } From ed7836581cb678f332980fcd5a7e323ffc334af0 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 26 Aug 2025 14:24:50 +0200 Subject: [PATCH 03/54] fix code sniffer violations --- .../forms/Config/General/PasswordPolicyConfigForm.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index 8406f1b995..be639d6fc4 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -51,11 +51,13 @@ public function createElements(array $formData) 'global_password_policy', array( 'description' => $this->translate( - 'Enforce strong password requirements for new passwords'), + 'Enforce strong password requirements for new passwords' + ), 'label' => $this->translate('Password Policy'), 'multiOptions' => array_merge( - ['' => sprintf(' - %s - ', - $this->translate('No Password Policy') + ['' => sprintf( + ' - %s - ', + $this->translate('No Password Policy') )], $passwordPolicies ), From bb96ba765dcf1b3ac91d21e5db2ed5e5b61d3039 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 26 Aug 2025 14:27:37 +0200 Subject: [PATCH 04/54] fix code sniffer violations --- application/forms/Config/General/PasswordPolicyConfigForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index be639d6fc4..0f04083e87 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -51,7 +51,7 @@ public function createElements(array $formData) 'global_password_policy', array( 'description' => $this->translate( - 'Enforce strong password requirements for new passwords' + 'Enforce strong password requirements for new passwords' ), 'label' => $this->translate('Password Policy'), 'multiOptions' => array_merge( From 174c59c2f43375954aabc3b7a052e8241410f368 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Tue, 26 Aug 2025 15:27:37 +0200 Subject: [PATCH 05/54] Update application/forms/Account/ChangePasswordForm.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Aleksandrovič Klimov --- application/forms/Account/ChangePasswordForm.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 8c3f3ebde2..29ba719703 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -10,8 +10,6 @@ use Icinga\User; use Icinga\Web\Form; use Icinga\Web\Notification; -use ipl\Html\Text; -use Icinga\Forms\Config\GeneralConfigForm; /** * Form for changing user passwords From 93c6479cfda36f446f83d1f8e5c019d6ec9a6db6 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Tue, 26 Aug 2025 15:27:51 +0200 Subject: [PATCH 06/54] Update application/forms/Account/ChangePasswordForm.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Aleksandrovič Klimov --- application/forms/Account/ChangePasswordForm.php | 1 - 1 file changed, 1 deletion(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 29ba719703..724502ab5d 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -31,7 +31,6 @@ public function init() $this->setSubmitLabel($this->translate('Update Account')); } - /** * {@inheritdoc} * @throws \Zend_Validate_Exception From 4ab7f2fa5f84bd34553ab36935d1e0300fc116a3 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 26 Aug 2025 17:09:51 +0200 Subject: [PATCH 07/54] add constructor for password policy objekt --- .../forms/Account/ChangePasswordForm.php | 60 +++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 724502ab5d..06e9dec860 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -4,6 +4,7 @@ namespace Icinga\Forms\Account; use Icinga\Application\Config; +use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -23,6 +24,23 @@ class ChangePasswordForm extends Form */ protected $backend; + /** + * The password policy object + * + * @var PasswordPolicyHook|null + */ + protected ?PasswordPolicyHook $passwordPolicyObject; + + /** + * Constructor + * + * @param PasswordPolicyHook|null $passwordPolicyObject + */ + public function __construct($passwordPolicyObject = null) + { + $this->passwordPolicyObject = $passwordPolicyObject; + parent::__construct(); + } /** * {@inheritdoc} */ @@ -33,47 +51,55 @@ public function init() /** * {@inheritdoc} - * @throws \Zend_Validate_Exception */ public function createElements(array $formData) { - $passwordPolicy = Config::app()->get('global', 'password_policy'); - if (isset($passwordPolicy) && class_exists($passwordPolicy)) { - $passwordPolicyObject = new $passwordPolicy(); - $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); + if ($this->passwordPolicyObject === null) { + $passwordPolicy = Config::app()->get( + 'global', + 'password_policy' + ); + if (isset($passwordPolicy) && class_exists($passwordPolicy)) { + $this->passwordPolicyObject = new $passwordPolicy(); + } + } + + if ($this->passwordPolicyObject) { + $this->addDescription( + $this->passwordPolicyObject->displayPasswordPolicy() + ); } $this->addElement( 'password', 'old_password', array( - 'label' => $this->translate('Old Password'), - 'required' => true + 'label' => $this->translate('Old Password'), + 'required' => true ) ); $this->addElement( 'password', 'new_password', array( - 'label' => $this->translate('New Password'), - 'required' => true, - 'validators' => array(new PasswordValidator()) + 'label' => $this->translate('New Password'), + 'required' => true, + 'validators' => [new PasswordValidator()] ) ); $this->addElement( 'password', 'new_password_confirmation', array( - 'label' => $this->translate('Confirm New Password'), - 'required' => true, - 'validators' => array( + 'label' => $this->translate('Confirm New Password'), + 'required' => true, + 'validators' => array( array('identical', false, array('new_password')) ) ) ); } - /** * {@inheritdoc} */ @@ -94,13 +120,13 @@ public function onSuccess() public function isValid($formData) { $valid = parent::isValid($formData); - if (!$valid) { + if (! $valid) { return false; } $oldPasswordEl = $this->getElement('old_password'); - if (!$this->backend->authenticate($this->Auth()->getUser(), $oldPasswordEl->getValue())) { + if (! $this->backend->authenticate($this->Auth()->getUser(), $oldPasswordEl->getValue())) { $oldPasswordEl->addError($this->translate('Old password is invalid')); $this->markAsError(); return false; @@ -122,7 +148,7 @@ public function getBackend() /** * Set the user backend * - * @param DbUserBackend $backend + * @param DbUserBackend $backend * * @return $this */ From 9175c4de94a61d25a968ccfc78f5c245f743b3d1 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Wed, 27 Aug 2025 09:48:05 +0200 Subject: [PATCH 08/54] add constructor to PasswordValidator and codereview changes --- .../forms/Account/ChangePasswordForm.php | 4 +-- .../General/PasswordPolicyConfigForm.php | 34 ++++--------------- .../Application/Hook/PasswordPolicyHook.php | 1 + .../ProvidedHook/DefaultPasswordPolicy.php | 1 + .../Authentication/PasswordValidator.php | 29 +++++++++++----- 5 files changed, 31 insertions(+), 38 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 06e9dec860..419731843d 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -36,7 +36,7 @@ class ChangePasswordForm extends Form * * @param PasswordPolicyHook|null $passwordPolicyObject */ - public function __construct($passwordPolicyObject = null) + public function __construct(?PasswordPolicyHook $passwordPolicyObject = null) { $this->passwordPolicyObject = $passwordPolicyObject; parent::__construct(); @@ -84,7 +84,7 @@ public function createElements(array $formData) array( 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => [new PasswordValidator()] + 'validators' => [new PasswordValidator($this->passwordPolicyObject)] ) ); $this->addElement( diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index 0f04083e87..c9cd03e4da 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -1,4 +1,5 @@ setName('form_config_general_password_policy'); } - /** - * {@inheritdoc} - * - * @return $this - */ - public function createElements(array $formData) + public function createElements(array $formData): self { - $this->addElement( - 'checkbox', - 'global_password_policy', - array( - 'label' => $this->translate('Password Policy'), - 'value' => true, - 'description' => $this->translate( - 'Enforce strong password requirements for new passwords' - ), - ) - ); - - $passwordPolicies = []; + $passwordPolicies = []; foreach (Hook::all('passwordpolicy') as $class => $policy) { $passwordPolicies[$class] = $policy->getName(); @@ -49,19 +30,16 @@ public function createElements(array $formData) $this->addElement( 'select', 'global_password_policy', - array( + [ 'description' => $this->translate( 'Enforce strong password requirements for new passwords' ), 'label' => $this->translate('Password Policy'), 'multiOptions' => array_merge( - ['' => sprintf( - ' - %s - ', - $this->translate('No Password Policy') - )], + ['' => $this->translate('No Password Policy')], $passwordPolicies ), - ) + ] ); return $this; diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 133214bbcf..25dbb215ea 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -1,4 +1,5 @@ passwordPolicyObject = $passwordPolicyObject; + } + /** * Checks if password matches with password policy * throws a message if not * - * If no password policy is configured, all passwords are considered valid + * If no password policy is set, all passwords are considered valid * * @param mixed $value The password to validate * @@ -21,17 +37,14 @@ class PasswordValidator extends Zend_Validate_Abstract public function isValid($value): bool { $this->_messages = []; - $passwordPolicy = Config::app() - ->get('global', 'password_policy'); - if (! isset($passwordPolicy) || ! class_exists($passwordPolicy)) { + if ($this->passwordPolicyObject === null) { return true; } - $passwordPolicyObject = new $passwordPolicy(); - $errorMessage = $passwordPolicyObject->validatePassword($value); + $errorMessage = $this->passwordPolicyObject->validatePassword($value); - if ($errorMessage != null) { + if ($errorMessage !== null) { $this->_messages[] = $errorMessage; return false; } From 1215b94130e45f783e91dd0be455d93471397e94 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Wed, 27 Aug 2025 09:51:28 +0200 Subject: [PATCH 09/54] fix codesniffer --- application/forms/Config/General/PasswordPolicyConfigForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index c9cd03e4da..f8edcf592a 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -20,7 +20,7 @@ public function init(): void public function createElements(array $formData): self { - $passwordPolicies = []; + $passwordPolicies = []; foreach (Hook::all('passwordpolicy') as $class => $policy) { $passwordPolicies[$class] = $policy->getName(); From 0a44791ecc25ec9ee315f038b6e477a4ba0d1c21 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 10:27:12 +0200 Subject: [PATCH 10/54] Implement 'None Password Policy'-class for consistency --- .../forms/Account/ChangePasswordForm.php | 42 ++++++--------- .../General/PasswordPolicyConfigForm.php | 8 +-- application/forms/Config/User/UserForm.php | 1 - .../Application/ApplicationBootstrap.php | 2 + .../Application/Hook/PasswordPolicyHook.php | 2 +- .../ProvidedHook/DefaultPasswordPolicy.php | 53 +++++++++++-------- .../ProvidedHook/NonePasswordPolicy.php | 28 ++++++++++ .../Authentication/PasswordValidator.php | 27 ++++------ 8 files changed, 90 insertions(+), 73 deletions(-) create mode 100644 library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 419731843d..42c5cbc72a 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -5,6 +5,7 @@ use Icinga\Application\Config; use Icinga\Application\Hook\PasswordPolicyHook; +use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -25,22 +26,12 @@ class ChangePasswordForm extends Form protected $backend; /** - * The password policy object + * The password policy object * - * @var PasswordPolicyHook|null + * @var PasswordPolicyHook */ - protected ?PasswordPolicyHook $passwordPolicyObject; + protected ?PasswordPolicyHook $passwordPolicyObject = null; - /** - * Constructor - * - * @param PasswordPolicyHook|null $passwordPolicyObject - */ - public function __construct(?PasswordPolicyHook $passwordPolicyObject = null) - { - $this->passwordPolicyObject = $passwordPolicyObject; - parent::__construct(); - } /** * {@inheritdoc} */ @@ -54,20 +45,15 @@ public function init() */ public function createElements(array $formData) { - if ($this->passwordPolicyObject === null) { - $passwordPolicy = Config::app()->get( - 'global', - 'password_policy' - ); - if (isset($passwordPolicy) && class_exists($passwordPolicy)) { - $this->passwordPolicyObject = new $passwordPolicy(); - } - } + $passwordPolicy = Config::app()->get( + 'global', + 'password_policy' + ); + $this->passwordPolicyObject = new $passwordPolicy(); + $passwordPolicyDescription = $this->passwordPolicyObject->displayPasswordPolicy(); - if ($this->passwordPolicyObject) { - $this->addDescription( - $this->passwordPolicyObject->displayPasswordPolicy() - ); + if ($passwordPolicyDescription != '') { + $this->addDescription($passwordPolicyDescription); } $this->addElement( @@ -84,7 +70,9 @@ public function createElements(array $formData) array( 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => [new PasswordValidator($this->passwordPolicyObject)] + 'validators' => + $this->passwordPolicyObject !== null ? + [new PasswordValidator($this->passwordPolicyObject)] : [], ) ); $this->addElement( diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index f8edcf592a..d87f08588c 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -4,6 +4,7 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Hook; +use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; use Icinga\Web\Form; /** @@ -35,13 +36,12 @@ public function createElements(array $formData): self 'Enforce strong password requirements for new passwords' ), 'label' => $this->translate('Password Policy'), - 'multiOptions' => array_merge( - ['' => $this->translate('No Password Policy')], - $passwordPolicies - ), + 'value' => DefaultPasswordPolicy::class, + 'multiOptions' =>$passwordPolicies ] ); + return $this; } } diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 9b490db642..be6ce1490f 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -17,7 +17,6 @@ class UserForm extends RepositoryForm * * @param array $formData The data sent by the user */ - protected function createInsertElements(array $formData) { $passwordPolicy = Config::app()->get('global', 'password_policy'); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 1537917df5..cbe04b09a4 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -8,6 +8,7 @@ use Exception; use Icinga\Application\ProvidedHook\DbMigration; use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; +use Icinga\Application\ProvidedHook\NonePasswordPolicy; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; @@ -742,6 +743,7 @@ protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); Hook::register('passwordpolicy', DefaultPasswordPolicy::class, DefaultPasswordPolicy::class); + Hook::register('passwordpolicy', NonePasswordPolicy::class, NonePasswordPolicy::class); return $this; } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 25dbb215ea..1854228110 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -26,5 +26,5 @@ public function displayPasswordPolicy(): string; * @return string|null Returns null if the password is valid, * otherwise returns an error message describing the violations */ - public function validatePassword(string $password): ?string; + public function validatePassword(string $password): ?array; } diff --git a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php index 7547f3d367..3b1ed3dc80 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php @@ -4,6 +4,7 @@ namespace Icinga\Application\ProvidedHook; use Icinga\Application\Hook\PasswordPolicyHook; +use ipl\I18n\Translation; /** * Default implementation of a password policy @@ -17,56 +18,64 @@ */ class DefaultPasswordPolicy implements PasswordPolicyHook { - /** - * @inheritdoc - */ + use Translation; + public function getName(): string { - return 'Default Password Policy'; + return 'Default'; } - /** - * @inheritdoc - */ public function displayPasswordPolicy(): string { - $message = ( - 'Password requirements: ' . 'minimum 12 characters, ' . - 'at least 1 number, ' . - '1 special character, ' . 'uppercase and lowercase letters' - ); + $message = + $this->translate( + 'Password requirements: minimum 12 characters, at least 1 number, ' . + '1 special character, uppercase and lowercase letters.' + ); return $message; } - /** - * @inheritdoc - */ - public function validatePassword(string $password): ?string + public function validatePassword(string $password): ?array { $violations = []; if (strlen($password) < 12) { - $violations[] = ('Password must be at least 12 characters long'); + $violations[] = + $this->translate( + 'Password must be at least 12 characters long' + ); } if (! preg_match('/[0-9]/', $password)) { - $violations[] = ('Password must contain at least one number'); + $violations[] = + $this->translate( + 'Password must contain at least one number' + ); } if (! preg_match('/[^a-zA-Z0-9]/', $password)) { - $violations[] = ('Password must contain at least one special character'); + $violations[] = + $this->translate( + 'Password must contain at least one special character' + ); } if (! preg_match('/[A-Z]/', $password)) { - $violations[] = ('Password must contain at least one uppercase letter'); + $violations[] = + $this->translate( + 'Password must contain at least one uppercase letter' + ); } if (! preg_match('/[a-z]/', $password)) { - $violations[] = ('Password must contain at least one lowercase letter'); + $violations[] = + $this->translate( + 'Password must contain at least one lowercase letter' + ); } if (! empty($violations)) { - return implode(", ", $violations); + return $violations; } return null; diff --git a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php new file mode 100644 index 0000000000..82f38a19f8 --- /dev/null +++ b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php @@ -0,0 +1,28 @@ +passwordPolicyObject = $passwordPolicyObject; } @@ -27,28 +29,17 @@ public function __construct(?PasswordPolicyHook $passwordPolicyObject = null) * Checks if password matches with password policy * throws a message if not * - * If no password policy is set, all passwords are considered valid - * * @param mixed $value The password to validate * * @return bool - * */ public function isValid($value): bool { - $this->_messages = []; - - if ($this->passwordPolicyObject === null) { + if ($this->passwordPolicyObject->validatePassword($value) === null) { return true; } - $errorMessage = $this->passwordPolicyObject->validatePassword($value); - - if ($errorMessage !== null) { - $this->_messages[] = $errorMessage; - return false; - } - - return true; + $this->_messages = $this->passwordPolicyObject->validatePassword($value); + return false; } } From 8f49073781239d921d2adb36ec8ab24a7d45a3b7 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 10:33:29 +0200 Subject: [PATCH 11/54] fix codesniffer --- library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php | 1 - 1 file changed, 1 deletion(-) diff --git a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php index 82f38a19f8..19281f0239 100644 --- a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php @@ -18,7 +18,6 @@ public function getName(): string public function displayPasswordPolicy(): string { return ''; - } public function validatePassword(string $password): ?array From 43d648313fcf9264f3a0e02e3a4682550aefa91b Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 12:58:40 +0200 Subject: [PATCH 12/54] fix if no password policy is set in config.ini --- .../forms/Account/ChangePasswordForm.php | 28 ++++++------ application/forms/Config/User/UserForm.php | 17 ++++++-- .../Application/Hook/PasswordPolicyHook.php | 8 ++-- .../ProvidedHook/DefaultPasswordPolicy.php | 43 ++++++++----------- .../ProvidedHook/NonePasswordPolicy.php | 6 +-- .../Authentication/PasswordValidator.php | 10 +++-- 6 files changed, 57 insertions(+), 55 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 42c5cbc72a..549cc8bb9c 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -25,13 +25,6 @@ class ChangePasswordForm extends Form */ protected $backend; - /** - * The password policy object - * - * @var PasswordPolicyHook - */ - protected ?PasswordPolicyHook $passwordPolicyObject = null; - /** * {@inheritdoc} */ @@ -45,15 +38,19 @@ public function init() */ public function createElements(array $formData) { + $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( - 'global', - 'password_policy' + 'global', + 'password_policy' ); - $this->passwordPolicyObject = new $passwordPolicy(); - $passwordPolicyDescription = $this->passwordPolicyObject->displayPasswordPolicy(); - if ($passwordPolicyDescription != '') { - $this->addDescription($passwordPolicyDescription); + if(isset($passwordPolicy)){ + $passwordPolicyObject = new $passwordPolicy(); + $passwordPolicyDescription = $passwordPolicyObject->getDescription(); + + if ($passwordPolicyDescription != '') { + $this->addDescription($passwordPolicyDescription); + } } $this->addElement( @@ -70,9 +67,8 @@ public function createElements(array $formData) array( 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => - $this->passwordPolicyObject !== null ? - [new PasswordValidator($this->passwordPolicyObject)] : [], + 'validators' => $passwordPolicyObject !== null ? + [new PasswordValidator($passwordPolicyObject)] : [], ) ); $this->addElement( diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index be6ce1490f..442876fd6b 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -19,10 +19,18 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $passwordPolicy = Config::app()->get('global', 'password_policy'); - if (isset($passwordPolicy) && class_exists($passwordPolicy)) { + $passwordPolicyObject = null; + $passwordPolicy = Config::app()->get( + 'global', + 'password_policy' + ); + if (isset($passwordPolicy)) { $passwordPolicyObject = new $passwordPolicy(); - $this->addDescription($passwordPolicyObject->displayPasswordPolicy()); + $passwordPolicyDescription = $passwordPolicyObject->getDescription(); + + if ($passwordPolicyDescription != '') { + $this->addDescription($passwordPolicyDescription); + } } $this->addElement( @@ -48,7 +56,8 @@ protected function createInsertElements(array $formData) array( 'required' => true, 'label' => $this->translate('Password'), - 'validators' => array(new PasswordValidator()) + 'validators' => $passwordPolicyObject !== null ? + [new PasswordValidator($passwordPolicyObject)] : [], ) ); diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 1854228110..f54b7e5766 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -12,19 +12,19 @@ interface PasswordPolicyHook */ public function getName(): string; -/** + /** * Displays the rules of the password policy for users * * @return string */ - public function displayPasswordPolicy(): string; + public function getDescription(): string; /** * Validate a given password against the defined policy * * @param string $password - * @return string|null Returns null if the password is valid, + * @return array Returns an empty array if the password is valid, * otherwise returns an error message describing the violations */ - public function validatePassword(string $password): ?array; + public function validatePassword(string $password): array; } diff --git a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php index 3b1ed3dc80..5e38a1e15b 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php @@ -25,7 +25,7 @@ public function getName(): string return 'Default'; } - public function displayPasswordPolicy(): string + public function getDescription(): string { $message = $this->translate( @@ -35,49 +35,44 @@ public function displayPasswordPolicy(): string return $message; } - public function validatePassword(string $password): ?array + public function validatePassword(string $password): array { $violations = []; - if (strlen($password) < 12) { - $violations[] = - $this->translate( - 'Password must be at least 12 characters long' - ); + if (mb_strlen($password) < 12) { + $violations[] = $this->translate( + 'Password must be at least 12 characters long' + ); } if (! preg_match('/[0-9]/', $password)) { - $violations[] = - $this->translate( - 'Password must contain at least one number' - ); + $violations[] = $this->translate( + 'Password must contain at least one number' + ); } if (! preg_match('/[^a-zA-Z0-9]/', $password)) { - $violations[] = - $this->translate( - 'Password must contain at least one special character' - ); + $violations[] = $this->translate( + 'Password must contain at least one special character' + ); } if (! preg_match('/[A-Z]/', $password)) { - $violations[] = - $this->translate( - 'Password must contain at least one uppercase letter' - ); + $violations[] = $this->translate( + 'Password must contain at least one uppercase letter' + ); } if (! preg_match('/[a-z]/', $password)) { - $violations[] = - $this->translate( - 'Password must contain at least one lowercase letter' - ); + $violations[] = $this->translate( + 'Password must contain at least one lowercase letter' + ); } if (! empty($violations)) { return $violations; } - return null; + return []; } } diff --git a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php index 19281f0239..c55407c88f 100644 --- a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php @@ -15,13 +15,13 @@ public function getName(): string return 'None'; } - public function displayPasswordPolicy(): string + public function getDescription(): string { return ''; } - public function validatePassword(string $password): ?array + public function validatePassword(string $password): array { - return null; + return []; } } diff --git a/library/Icinga/Authentication/PasswordValidator.php b/library/Icinga/Authentication/PasswordValidator.php index 4a7f837b96..8b9ecbd7c3 100644 --- a/library/Icinga/Authentication/PasswordValidator.php +++ b/library/Icinga/Authentication/PasswordValidator.php @@ -35,11 +35,13 @@ public function __construct(PasswordPolicyHook $passwordPolicyObject) */ public function isValid($value): bool { - if ($this->passwordPolicyObject->validatePassword($value) === null) { - return true; + $message = $this->passwordPolicyObject->validatePassword($value); + + if (!empty($message)) { + $this->_messages = $message; + return false; } - $this->_messages = $this->passwordPolicyObject->validatePassword($value); - return false; + return true; } } From 02c067ef0f0b6fa53add795b27cb769e36ebb4a7 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 13:03:18 +0200 Subject: [PATCH 13/54] fix codesniffer --- application/forms/Account/ChangePasswordForm.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 549cc8bb9c..d7deba7864 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -40,11 +40,11 @@ public function createElements(array $formData) { $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( - 'global', - 'password_policy' + 'global', + 'password_policy' ); - if(isset($passwordPolicy)){ + if (isset($passwordPolicy)) { $passwordPolicyObject = new $passwordPolicy(); $passwordPolicyDescription = $passwordPolicyObject->getDescription(); From 740837f9c11eda24b4d12a223dfcad88ec69847b Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 15:13:12 +0200 Subject: [PATCH 14/54] changes codereview --- .../forms/Account/ChangePasswordForm.php | 4 ++-- .../ProvidedHook/DefaultPasswordPolicy.php | 18 ++++++------------ .../ProvidedHook/NonePasswordPolicy.php | 4 +++- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index d7deba7864..73511e8fa2 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -4,7 +4,6 @@ namespace Icinga\Forms\Account; use Icinga\Application\Config; -use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; @@ -41,7 +40,8 @@ public function createElements(array $formData) $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( 'global', - 'password_policy' + 'password_policy', + DefaultPasswordPolicy::class ); if (isset($passwordPolicy)) { diff --git a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php index 5e38a1e15b..ad611cccdf 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php @@ -22,17 +22,15 @@ class DefaultPasswordPolicy implements PasswordPolicyHook public function getName(): string { - return 'Default'; + return $this->translate('Default'); } public function getDescription(): string { - $message = - $this->translate( - 'Password requirements: minimum 12 characters, at least 1 number, ' . - '1 special character, uppercase and lowercase letters.' - ); - return $message; + return $this->translate( + 'Password requirements: minimum 12 characters,' . + 'at least 1 number, 1 special character, uppercase and lowercase letters.' + ); } public function validatePassword(string $password): array @@ -69,10 +67,6 @@ public function validatePassword(string $password): array ); } - if (! empty($violations)) { - return $violations; - } - - return []; + return $violations; } } diff --git a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php index c55407c88f..60ac8674bf 100644 --- a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php @@ -4,15 +4,17 @@ namespace Icinga\Application\ProvidedHook; use Icinga\Application\Hook\PasswordPolicyHook; +use ipl\I18n\Translation; /** * None Password Policy to validate all passwords */ class NonePasswordPolicy implements PasswordPolicyHook { + use Translation; public function getName(): string { - return 'None'; + return $this->translate('None'); } public function getDescription(): string From 64d7036c4e67b1f3c4b5b3be519f3034cbafb6ca Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 15:51:04 +0200 Subject: [PATCH 15/54] codereview --- application/forms/Config/User/UserForm.php | 4 +++- library/Icinga/Application/ApplicationBootstrap.php | 4 ++-- .../Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php | 2 +- .../{NonePasswordPolicy.php => NoPasswordPolicy.php} | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) rename library/Icinga/Application/ProvidedHook/{NonePasswordPolicy.php => NoPasswordPolicy.php} (90%) diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 442876fd6b..8d8a591e7b 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -5,6 +5,7 @@ use Icinga\Application\Config; use Icinga\Application\Hook\ConfigFormEventsHook; +use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; @@ -22,7 +23,8 @@ protected function createInsertElements(array $formData) $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( 'global', - 'password_policy' + 'password_policy', + DefaultPasswordPolicy::class ); if (isset($passwordPolicy)) { $passwordPolicyObject = new $passwordPolicy(); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index cbe04b09a4..b290e463a3 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -8,7 +8,7 @@ use Exception; use Icinga\Application\ProvidedHook\DbMigration; use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; -use Icinga\Application\ProvidedHook\NonePasswordPolicy; +use Icinga\Application\ProvidedHook\NoPasswordPolicy; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; @@ -743,7 +743,7 @@ protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); Hook::register('passwordpolicy', DefaultPasswordPolicy::class, DefaultPasswordPolicy::class); - Hook::register('passwordpolicy', NonePasswordPolicy::class, NonePasswordPolicy::class); + Hook::register('passwordpolicy', NoPasswordPolicy::class, NoPasswordPolicy::class); return $this; } diff --git a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php index ad611cccdf..3feb2a99dc 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php @@ -28,7 +28,7 @@ public function getName(): string public function getDescription(): string { return $this->translate( - 'Password requirements: minimum 12 characters,' . + 'Password requirements: minimum 12 characters, ' . 'at least 1 number, 1 special character, uppercase and lowercase letters.' ); } diff --git a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php b/library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php similarity index 90% rename from library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php rename to library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php index 60ac8674bf..b4d109b5f6 100644 --- a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php @@ -9,9 +9,10 @@ /** * None Password Policy to validate all passwords */ -class NonePasswordPolicy implements PasswordPolicyHook +class NoPasswordPolicy implements PasswordPolicyHook { use Translation; + public function getName(): string { return $this->translate('None'); From b3a1a315f10caf298465b9c7be4bf2b341b4f533 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 16:47:17 +0200 Subject: [PATCH 16/54] codereview --- application/forms/Account/ChangePasswordForm.php | 12 ++++-------- application/forms/Config/User/UserForm.php | 11 ++++------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 73511e8fa2..a21f41adbf 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -37,20 +37,16 @@ public function init() */ public function createElements(array $formData) { - $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( 'global', 'password_policy', DefaultPasswordPolicy::class ); + $passwordPolicyObject = new $passwordPolicy(); + $passwordPolicyDescription = $passwordPolicyObject->getDescription(); - if (isset($passwordPolicy)) { - $passwordPolicyObject = new $passwordPolicy(); - $passwordPolicyDescription = $passwordPolicyObject->getDescription(); - - if ($passwordPolicyDescription != '') { - $this->addDescription($passwordPolicyDescription); - } + if ($passwordPolicyDescription != '') { + $this->addDescription($passwordPolicyDescription); } $this->addElement( diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 8d8a591e7b..19a778f70e 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -20,19 +20,16 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $passwordPolicyObject = null; $passwordPolicy = Config::app()->get( 'global', 'password_policy', DefaultPasswordPolicy::class ); - if (isset($passwordPolicy)) { - $passwordPolicyObject = new $passwordPolicy(); - $passwordPolicyDescription = $passwordPolicyObject->getDescription(); + $passwordPolicyObject = new $passwordPolicy(); + $passwordPolicyDescription = $passwordPolicyObject->getDescription(); - if ($passwordPolicyDescription != '') { - $this->addDescription($passwordPolicyDescription); - } + if ($passwordPolicyDescription != '') { + $this->addDescription($passwordPolicyDescription); } $this->addElement( From d2d91d2a870eb1fa75eaa87acc69124bf29de13d Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 17:22:38 +0200 Subject: [PATCH 17/54] codereview --- application/forms/Account/ChangePasswordForm.php | 3 +-- application/forms/Config/User/UserForm.php | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index a21f41adbf..3c4437ef5f 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -63,8 +63,7 @@ public function createElements(array $formData) array( 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => $passwordPolicyObject !== null ? - [new PasswordValidator($passwordPolicyObject)] : [], + 'validators' => [new PasswordValidator($passwordPolicyObject)] ) ); $this->addElement( diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 19a778f70e..5da7583f9c 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -55,8 +55,7 @@ protected function createInsertElements(array $formData) array( 'required' => true, 'label' => $this->translate('Password'), - 'validators' => $passwordPolicyObject !== null ? - [new PasswordValidator($passwordPolicyObject)] : [], + 'validators' => [new PasswordValidator($passwordPolicyObject)] ) ); From 2a106715e6f11c72627fa0801648527439a1fb53 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 1 Sep 2025 13:30:59 +0200 Subject: [PATCH 18/54] Change Class Default in CommonPasswordPolicy --- .gitignore | 3 +++ application/forms/Account/ChangePasswordForm.php | 5 +++-- .../forms/Config/General/PasswordPolicyConfigForm.php | 5 +++-- application/forms/Config/User/UserForm.php | 5 +++-- library/Icinga/Application/ApplicationBootstrap.php | 4 ++-- .../{DefaultPasswordPolicy.php => CommonPasswordPolicy.php} | 6 +++--- 6 files changed, 17 insertions(+), 11 deletions(-) rename library/Icinga/Application/ProvidedHook/{DefaultPasswordPolicy.php => CommonPasswordPolicy.php} (92%) diff --git a/.gitignore b/.gitignore index ff5bb165eb..5ca14cbc4a 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,6 @@ build/* # Exclude dompdf font cache library/vendor/dompdf/lib/fonts/*.php library/vendor/dompdf/lib/fonts/log.htm + +#locale änderung für Tests sollen nicht gepushed werden +composer.json diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 3c4437ef5f..15986cf6fa 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -4,7 +4,8 @@ namespace Icinga\Forms\Account; use Icinga\Application\Config; -use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; +use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -40,7 +41,7 @@ public function createElements(array $formData) $passwordPolicy = Config::app()->get( 'global', 'password_policy', - DefaultPasswordPolicy::class + NoPasswordPolicy::class ); $passwordPolicyObject = new $passwordPolicy(); $passwordPolicyDescription = $passwordPolicyObject->getDescription(); diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index d87f08588c..ef6ea61645 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -4,7 +4,8 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Hook; -use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; +use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Web\Form; /** @@ -36,7 +37,7 @@ public function createElements(array $formData): self 'Enforce strong password requirements for new passwords' ), 'label' => $this->translate('Password Policy'), - 'value' => DefaultPasswordPolicy::class, + 'value' => NoPasswordPolicy::class, 'multiOptions' =>$passwordPolicies ] ); diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 5da7583f9c..62707a9fc2 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -5,7 +5,8 @@ use Icinga\Application\Config; use Icinga\Application\Hook\ConfigFormEventsHook; -use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; +use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; @@ -23,7 +24,7 @@ protected function createInsertElements(array $formData) $passwordPolicy = Config::app()->get( 'global', 'password_policy', - DefaultPasswordPolicy::class + NoPasswordPolicy::class ); $passwordPolicyObject = new $passwordPolicy(); $passwordPolicyDescription = $passwordPolicyObject->getDescription(); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index b290e463a3..34668c9063 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -7,7 +7,7 @@ use ErrorException; use Exception; use Icinga\Application\ProvidedHook\DbMigration; -use Icinga\Application\ProvidedHook\DefaultPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\NoPasswordPolicy; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; @@ -742,7 +742,7 @@ public function hasLocales() protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); - Hook::register('passwordpolicy', DefaultPasswordPolicy::class, DefaultPasswordPolicy::class); + Hook::register('passwordpolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); Hook::register('passwordpolicy', NoPasswordPolicy::class, NoPasswordPolicy::class); return $this; diff --git a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php similarity index 92% rename from library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php rename to library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index 3feb2a99dc..a3efc158e0 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -7,7 +7,7 @@ use ipl\I18n\Translation; /** - * Default implementation of a password policy + * Common implementation of a password policy * * Enforces: * - Minimum length of 12 characters @@ -16,13 +16,13 @@ * - At least one uppercase letter * - At least one lowercase letter */ -class DefaultPasswordPolicy implements PasswordPolicyHook +class CommonPasswordPolicy implements PasswordPolicyHook { use Translation; public function getName(): string { - return $this->translate('Default'); + return $this->translate('Common'); } public function getDescription(): string From d7acd7caf4acae95b45ba18cb6f7ef94545a55d6 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 2 Sep 2025 13:42:31 +0200 Subject: [PATCH 19/54] Unittest for Classes Password Policy --- .../Application/CommonPasswordPolicyTest.php | 102 ++++++++++++++++++ .../Application/NoPasswordPolicyTest.php | 33 ++++++ 2 files changed, 135 insertions(+) create mode 100644 test/php/library/Icinga/Application/CommonPasswordPolicyTest.php create mode 100644 test/php/library/Icinga/Application/NoPasswordPolicyTest.php diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php new file mode 100644 index 0000000000..6221588965 --- /dev/null +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -0,0 +1,102 @@ +object = new CommonPasswordPolicy(); + } + + public function testClassIsInstanzOf() + { + $this->assertInstanceOf(PasswordPolicyHook::class, $this->object); + } + + public function testMethodGetName(): void + { + $this->assertSame('Common', $this->object->getName()); + } + + public function testValidatePasswordTooShort() + { + $res = $this->object->validatePassword('Icinga1#'); + $this->assertSame('Password must be at least 12 characters long', $res[0]); + $this->assertCount(1, $res); + } + + public function testValidatePasswordNoNumber() + { + $res = $this->object->validatePassword('Icingaadmin#'); + $this->assertSame('Password must contain at least one number', $res[0]); + var_dump($res); + $this->assertCount(1, $res); + } + + public function testValidatePasswordNoSpecialCharacter() + { + $res = $this->object->validatePassword('Icingaadmin1'); + $this->assertSame('Password must contain at least one special character', $res[0]); + $this->assertCount(1, $res); + } + + public function testValidatePasswordNoUpperCaseLetters() + { + $res = $this->object->validatePassword('icingaadmin1#'); + $this->assertSame('Password must contain at least one uppercase letter', $res[0]); + $this->assertCount(1, $res); + } + + public function testValidatePasswordNoLowerCaseLetters() + { + $res = $this->object->validatePassword('ICINGAADMIN1#'); + $this->assertSame('Password must contain at least one lowercase letter', $res[0]); + $this->assertCount(1, $res); + } + + public function testValidatePasswordValid() + { + $res = $this->object->validatePassword('Icingaadmin1#'); + $this->assertEmpty($res); + } + + public function testValidatePasswordOnlyLowerCaseLetters() + { + $res = $this->object->validatePassword('icingawebadmin'); + var_dump($res); + $this->assertCount(3, $res); + $this->assertSame('Password must contain at least one number', $res[0]); + $this->assertSame('Password must contain at least one special character', $res[1]); + $this->assertSame('Password must contain at least one uppercase letter', $res[2]); + } + + public function testValidatePasswordWithLengthAndUpperCaseLetters() + { + $res = $this->object->validatePassword('ICINGAADMIN'); + var_dump($res); + $this->assertCount(4, $res); + $this->assertSame('Password must be at least 12 characters long', $res[0]); + $this->assertSame('Password must contain at least one number', $res[1]); + $this->assertSame('Password must contain at least one special character', $res[2]); + $this->assertSame('Password must contain at least one lowercase letter', $res[3]); + } + + public function testValidatePasswordWithManyCharacters() + { + $longPassword = str_repeat('a', 1000); + var_dump($longPassword); + $res = $this->object->validatePassword($longPassword); + var_dump($res); + $this->assertCount(3, $res); + } + } + + + diff --git a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php new file mode 100644 index 0000000000..fd4d981ac4 --- /dev/null +++ b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php @@ -0,0 +1,33 @@ +object = new NoPasswordPolicy(); + } + + public function testClassIsInstanzOf() + { + $this->assertInstanceOf(PasswordPolicyHook::class, $this->object); + } + + public function testMethodGetName(): void + { + $this->assertSame('None', $this->object->getName()); + } + + public function testValidatePasswordValid() + { + $res = $this->object->validatePassword('icingaadmin'); + $this->assertEmpty($res); + } +} From 33bb7ba946813cddc7a75ce80d405f52eb828e56 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 2 Sep 2025 15:59:18 +0200 Subject: [PATCH 20/54] codereview: Improve type hints and return types --- modules/password-policy | 0 .../Application/CommonPasswordPolicyTest.php | 52 ++++++++----------- .../Application/NoPasswordPolicyTest.php | 15 ++---- 3 files changed, 26 insertions(+), 41 deletions(-) create mode 100644 modules/password-policy diff --git a/modules/password-policy b/modules/password-policy new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index 6221588965..c3a8e38f02 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -8,79 +8,71 @@ class CommonPasswordPolicyTest extends BaseTestCase { - private object $object; + private PasswordPolicyHook $instance; public function setUp(): void { - $this->object = new CommonPasswordPolicy(); - } - - public function testClassIsInstanzOf() - { - $this->assertInstanceOf(PasswordPolicyHook::class, $this->object); + $this->instance = new CommonPasswordPolicy(); } public function testMethodGetName(): void { - $this->assertSame('Common', $this->object->getName()); + $this->assertSame('Common', $this->instance->getName()); } - public function testValidatePasswordTooShort() + public function testValidatePasswordTooShort(): void { - $res = $this->object->validatePassword('Icinga1#'); + $res = $this->instance->validatePassword('Icinga1#'); $this->assertSame('Password must be at least 12 characters long', $res[0]); $this->assertCount(1, $res); } - public function testValidatePasswordNoNumber() + public function testValidatePasswordNoNumber(): void { - $res = $this->object->validatePassword('Icingaadmin#'); + $res = $this->instance->validatePassword('Icingaadmin#'); $this->assertSame('Password must contain at least one number', $res[0]); - var_dump($res); $this->assertCount(1, $res); } - public function testValidatePasswordNoSpecialCharacter() + public function testValidatePasswordNoSpecialCharacter(): void { - $res = $this->object->validatePassword('Icingaadmin1'); + $res = $this->instance->validatePassword('Icingaadmin1'); $this->assertSame('Password must contain at least one special character', $res[0]); $this->assertCount(1, $res); } - public function testValidatePasswordNoUpperCaseLetters() + public function testValidatePasswordNoUpperCaseLetters(): void { - $res = $this->object->validatePassword('icingaadmin1#'); + $res = $this->instance->validatePassword('icingaadmin1#'); $this->assertSame('Password must contain at least one uppercase letter', $res[0]); $this->assertCount(1, $res); } - public function testValidatePasswordNoLowerCaseLetters() + public function testValidatePasswordNoLowerCaseLetters(): void { - $res = $this->object->validatePassword('ICINGAADMIN1#'); + $res = $this->instance->validatePassword('ICINGAADMIN1#'); $this->assertSame('Password must contain at least one lowercase letter', $res[0]); $this->assertCount(1, $res); } - public function testValidatePasswordValid() + public function testValidatePasswordValid(): void { - $res = $this->object->validatePassword('Icingaadmin1#'); + $res = $this->instance->validatePassword('Icingaadmin1#'); $this->assertEmpty($res); } - public function testValidatePasswordOnlyLowerCaseLetters() + public function testValidatePasswordOnlyLowerCaseLetters(): void { - $res = $this->object->validatePassword('icingawebadmin'); - var_dump($res); + $res = $this->instance->validatePassword('icingawebadmin'); $this->assertCount(3, $res); $this->assertSame('Password must contain at least one number', $res[0]); $this->assertSame('Password must contain at least one special character', $res[1]); $this->assertSame('Password must contain at least one uppercase letter', $res[2]); } - public function testValidatePasswordWithLengthAndUpperCaseLetters() + public function testValidatePasswordWithLengthAndUpperCaseLetters(): void { - $res = $this->object->validatePassword('ICINGAADMIN'); - var_dump($res); + $res = $this->instance->validatePassword('ICINGAADMIN'); $this->assertCount(4, $res); $this->assertSame('Password must be at least 12 characters long', $res[0]); $this->assertSame('Password must contain at least one number', $res[1]); @@ -88,12 +80,10 @@ public function testValidatePasswordWithLengthAndUpperCaseLetters() $this->assertSame('Password must contain at least one lowercase letter', $res[3]); } - public function testValidatePasswordWithManyCharacters() + public function testValidatePasswordWithManyCharacters(): void { $longPassword = str_repeat('a', 1000); - var_dump($longPassword); - $res = $this->object->validatePassword($longPassword); - var_dump($res); + $res = $this->instance->validatePassword($longPassword); $this->assertCount(3, $res); } } diff --git a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php index fd4d981ac4..4eb4b06854 100644 --- a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php @@ -8,26 +8,21 @@ class NoPasswordPolicyTest extends BaseTestCase { - private object $object; + private PasswordPolicyHook $instance; public function setUp(): void { - $this->object = new NoPasswordPolicy(); - } - - public function testClassIsInstanzOf() - { - $this->assertInstanceOf(PasswordPolicyHook::class, $this->object); + $this->instance = new NoPasswordPolicy(); } public function testMethodGetName(): void { - $this->assertSame('None', $this->object->getName()); + $this->assertSame('None', $this->instance->getName()); } - public function testValidatePasswordValid() + public function testValidatePasswordValid(): void { - $res = $this->object->validatePassword('icingaadmin'); + $res = $this->instance->validatePassword('icingaadmin'); $this->assertEmpty($res); } } From 9d8cbcd08fac9652922130cd2f3828a572a52e85 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Wed, 10 Sep 2025 13:10:52 +0200 Subject: [PATCH 21/54] codereview --- .gitignore | 3 -- .../General/PasswordPolicyConfigForm.php | 3 +- .../Application/Hook/PasswordPolicyHook.php | 2 +- .../ProvidedHook/CommonPasswordPolicy.php | 20 ++------ .../Application/CommonPasswordPolicyTest.php | 48 ++++++++++--------- .../Application/NoPasswordPolicyTest.php | 4 +- 6 files changed, 35 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 5ca14cbc4a..ff5bb165eb 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,3 @@ build/* # Exclude dompdf font cache library/vendor/dompdf/lib/fonts/*.php library/vendor/dompdf/lib/fonts/log.htm - -#locale änderung für Tests sollen nicht gepushed werden -composer.json diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index ef6ea61645..4fb19f55a0 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -4,7 +4,6 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Hook; -use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Web\Form; @@ -20,7 +19,7 @@ public function init(): void $this->setName('form_config_general_password_policy'); } - public function createElements(array $formData): self + public function createElements(array $formData): static { $passwordPolicies = []; diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index f54b7e5766..a6013eec70 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -23,7 +23,7 @@ public function getDescription(): string; * Validate a given password against the defined policy * * @param string $password - * @return array Returns an empty array if the password is valid, + * @return string[] Returns an empty array if the password is valid, * otherwise returns an error message describing the violations */ public function validatePassword(string $password): array; diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index a3efc158e0..f49bd1d1e1 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -38,33 +38,23 @@ public function validatePassword(string $password): array $violations = []; if (mb_strlen($password) < 12) { - $violations[] = $this->translate( - 'Password must be at least 12 characters long' - ); + $violations[] = $this->translate('Password must be at least 12 characters long'); } if (! preg_match('/[0-9]/', $password)) { - $violations[] = $this->translate( - 'Password must contain at least one number' - ); + $violations[] = $this->translate('Password must contain at least one number'); } if (! preg_match('/[^a-zA-Z0-9]/', $password)) { - $violations[] = $this->translate( - 'Password must contain at least one special character' - ); + $violations[] = $this->translate('Password must contain at least one special character'); } if (! preg_match('/[A-Z]/', $password)) { - $violations[] = $this->translate( - 'Password must contain at least one uppercase letter' - ); + $violations[] = $this->translate('Password must contain at least one uppercase letter'); } if (! preg_match('/[a-z]/', $password)) { - $violations[] = $this->translate( - 'Password must contain at least one lowercase letter' - ); + $violations[] = $this->translate('Password must contain at least one lowercase letter'); } return $violations; diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index c3a8e38f02..dc2142d23a 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -4,9 +4,9 @@ use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\ProvidedHook\CommonPasswordPolicy; -use Icinga\Test\BaseTestCase; +use PHPUnit\Framework\TestCase; -class CommonPasswordPolicyTest extends BaseTestCase +class CommonPasswordPolicyTest extends TestCase { private PasswordPolicyHook $instance; @@ -22,40 +22,40 @@ public function testMethodGetName(): void public function testValidatePasswordTooShort(): void { + $expectedResults = ['Password must be at least 12 characters long']; $res = $this->instance->validatePassword('Icinga1#'); - $this->assertSame('Password must be at least 12 characters long', $res[0]); - $this->assertCount(1, $res); + $this->assertSame($expectedResults, $res); } public function testValidatePasswordNoNumber(): void { + $expectedResults = ['Password must contain at least one number']; $res = $this->instance->validatePassword('Icingaadmin#'); - $this->assertSame('Password must contain at least one number', $res[0]); - $this->assertCount(1, $res); + $this->assertSame($expectedResults, $res); } public function testValidatePasswordNoSpecialCharacter(): void { + $expectedResult = ['Password must contain at least one special character']; $res = $this->instance->validatePassword('Icingaadmin1'); - $this->assertSame('Password must contain at least one special character', $res[0]); - $this->assertCount(1, $res); + $this->assertSame($expectedResult, $res); } public function testValidatePasswordNoUpperCaseLetters(): void { + $expectedResult = ['Password must contain at least one uppercase letter']; $res = $this->instance->validatePassword('icingaadmin1#'); - $this->assertSame('Password must contain at least one uppercase letter', $res[0]); - $this->assertCount(1, $res); + $this->assertSame($expectedResult, $res); } public function testValidatePasswordNoLowerCaseLetters(): void { + $expectedResult = ['Password must contain at least one lowercase letter']; $res = $this->instance->validatePassword('ICINGAADMIN1#'); - $this->assertSame('Password must contain at least one lowercase letter', $res[0]); - $this->assertCount(1, $res); + $this->assertSame($expectedResult, $res); } - public function testValidatePasswordValid(): void + public function testMethodValidatePasswordAlwaysReturnAnEmptyArray(): void { $res = $this->instance->validatePassword('Icingaadmin1#'); $this->assertEmpty($res); @@ -63,21 +63,25 @@ public function testValidatePasswordValid(): void public function testValidatePasswordOnlyLowerCaseLetters(): void { + $expectedResult = [ + 'Password must contain at least one number', + 'Password must contain at least one special character', + 'Password must contain at least one uppercase letter' + ]; $res = $this->instance->validatePassword('icingawebadmin'); - $this->assertCount(3, $res); - $this->assertSame('Password must contain at least one number', $res[0]); - $this->assertSame('Password must contain at least one special character', $res[1]); - $this->assertSame('Password must contain at least one uppercase letter', $res[2]); + $this->assertSame($expectedResult, $res); } public function testValidatePasswordWithLengthAndUpperCaseLetters(): void { + $expectedResult = [ + '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', + ]; $res = $this->instance->validatePassword('ICINGAADMIN'); - $this->assertCount(4, $res); - $this->assertSame('Password must be at least 12 characters long', $res[0]); - $this->assertSame('Password must contain at least one number', $res[1]); - $this->assertSame('Password must contain at least one special character', $res[2]); - $this->assertSame('Password must contain at least one lowercase letter', $res[3]); + $this->assertSame($expectedResult, $res); } public function testValidatePasswordWithManyCharacters(): void diff --git a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php index 4eb4b06854..9d59332df6 100644 --- a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/NoPasswordPolicyTest.php @@ -3,10 +3,10 @@ namespace Tests\Icinga\Application; use Icinga\Application\Hook\PasswordPolicyHook; -use Icinga\Test\BaseTestCase; +use PHPUnit\Framework\TestCase; use Icinga\Application\ProvidedHook\NoPasswordPolicy; -class NoPasswordPolicyTest extends BaseTestCase +class NoPasswordPolicyTest extends TestCase { private PasswordPolicyHook $instance; From 2682e0ffe43701441de9b4c3db48fc66140fd2df Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Wed, 10 Sep 2025 17:37:57 +0200 Subject: [PATCH 22/54] add documantation for general and custom password policy --- doc/05-Authentication.md | 89 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 22772b5a1e..0469a96341 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -158,6 +158,95 @@ resource = icingaweb-mysql Please read [this chapter](20-Advanced-Topics.md#advanced-topics-authentication-tips-manual-user-database-auth) in order to manually create users directly inside the database. +### Password Policy +Icinga Web 2 supports password policies when using database authentication. +You can configure this under **Configuration > Application > General**. + +By default, no password policy is enforced ('None'). +Icinga Web 2 provides a built-in policy called 'Common' with the following requirements: + +* Minimum length of 12 characters +* At least one number +* At least one special character +* At least one uppercase letter +* At least one lowercase letter + +#### Custom Password Policy +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 +Name: My Password Policy +Version: 1.0.0 +Description: Custom password policy implementation +Author: Your Name +``` + +**Implement the Hook** + +Icinga Web 2 provides the `PasswordPolicyHook` interface with predefined methods +that simplify the implementation of custom password policies. + +Create `library/MyPasswordPolicy/ProvidedHook/PasswordPolicy.php`: + +```php +namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; + +use Icinga\Application\Hook\PasswordPolicyHook; + +class PasswordPolicy implements PasswordPolicyHook +{ + public function getName(): string + { + return 'My Custom Policy'; + } + + public function getDescription(): string + { + return 'Custom password requirements: 8+ chars, 1 number'; + } + + public function validatePassword(string $password): array + { + $violations = []; + + if (strlen($password) < 8) { + $violations[] = 'Password must be at least 8 characters'; + } + + if (!preg_match('/[0-9]/', $password)) { + $violations[] = 'Password must contain at least one number'; + } + + return $violations; + } +} +``` + +**Register the Hook** + +Create `run.php`: +```php +/** @var $this \Icinga\Application\Modules\Module */ + +$this->provideHook('passwordpolicy', 'PasswordPolicy'); +``` + + +Enable the module: +```bash +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 From 91f20c82247a6edc7126251efc70e6a2ffe65b8e Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 11 Sep 2025 13:28:29 +0200 Subject: [PATCH 23/54] codereview --- .../forms/Account/ChangePasswordForm.php | 23 +++++++++---------- .../General/PasswordPolicyConfigForm.php | 16 +++++-------- application/forms/Config/User/UserForm.php | 21 ++++++++--------- doc/05-Authentication.md | 2 +- .../Application/ApplicationBootstrap.php | 4 ++-- ...sswordPolicy.php => AnyPasswordPolicy.php} | 4 ++-- modules/password-policy | 0 ...licyTest.php => AnyPasswordPolicyTest.php} | 8 +++---- 8 files changed, 36 insertions(+), 42 deletions(-) rename library/Icinga/Application/ProvidedHook/{NoPasswordPolicy.php => AnyPasswordPolicy.php} (83%) delete mode 100644 modules/password-policy rename test/php/library/Icinga/Application/{NoPasswordPolicyTest.php => AnyPasswordPolicyTest.php} (67%) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 15986cf6fa..be7fd98ba9 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -4,8 +4,7 @@ namespace Icinga\Forms\Account; use Icinga\Application\Config; -use Icinga\Application\ProvidedHook\CommonPasswordPolicy; -use Icinga\Application\ProvidedHook\NoPasswordPolicy; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -38,15 +37,15 @@ public function init() */ public function createElements(array $formData) { - $passwordPolicy = Config::app()->get( + $passwordPolicyClass = Config::app()->get( 'global', 'password_policy', - NoPasswordPolicy::class + AnyPasswordPolicy::class ); - $passwordPolicyObject = new $passwordPolicy(); - $passwordPolicyDescription = $passwordPolicyObject->getDescription(); - if ($passwordPolicyDescription != '') { + $passwordPolicy = new $passwordPolicyClass(); + $passwordPolicyDescription = $passwordPolicy->getDescription(); + if ($passwordPolicyDescription !== '') { $this->addDescription($passwordPolicyDescription); } @@ -61,11 +60,11 @@ public function createElements(array $formData) $this->addElement( 'password', 'new_password', - array( - 'label' => $this->translate('New Password'), - 'required' => true, - 'validators' => [new PasswordValidator($passwordPolicyObject)] - ) + [ + 'label' => $this->translate('New Password'), + 'required' => true, + 'validators' => [new PasswordValidator($passwordPolicy)] + ] ); $this->addElement( 'password', diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index 4fb19f55a0..4c94a4bb2d 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -4,7 +4,7 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Hook; -use Icinga\Application\ProvidedHook\NoPasswordPolicy; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Web\Form; /** @@ -22,26 +22,22 @@ public function init(): void public function createElements(array $formData): static { $passwordPolicies = []; - foreach (Hook::all('passwordpolicy') as $class => $policy) { $passwordPolicies[$class] = $policy->getName(); } - asort($passwordPolicies); + $this->addElement( 'select', 'global_password_policy', [ - 'description' => $this->translate( - 'Enforce strong password requirements for new passwords' - ), - 'label' => $this->translate('Password Policy'), - 'value' => NoPasswordPolicy::class, - 'multiOptions' =>$passwordPolicies + 'description' => $this->translate('Enforce password requirements for new passwords'), + 'label' => $this->translate('Password Policy'), + 'value' => AnyPasswordPolicy::class, + 'multiOptions' => $passwordPolicies ] ); - return $this; } } diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 62707a9fc2..dafa9257b7 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -5,8 +5,7 @@ use Icinga\Application\Config; use Icinga\Application\Hook\ConfigFormEventsHook; -use Icinga\Application\ProvidedHook\CommonPasswordPolicy; -use Icinga\Application\ProvidedHook\NoPasswordPolicy; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; @@ -21,14 +20,14 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $passwordPolicy = Config::app()->get( + $passwordPolicyClass = Config::app()->get( 'global', 'password_policy', - NoPasswordPolicy::class + AnyPasswordPolicy::class ); - $passwordPolicyObject = new $passwordPolicy(); - $passwordPolicyDescription = $passwordPolicyObject->getDescription(); + $passwordPolicy = new $passwordPolicyClass(); + $passwordPolicyDescription = $passwordPolicy->getDescription(); if ($passwordPolicyDescription != '') { $this->addDescription($passwordPolicyDescription); } @@ -53,11 +52,11 @@ protected function createInsertElements(array $formData) $this->addElement( 'password', 'password', - array( - 'required' => true, - 'label' => $this->translate('Password'), - 'validators' => [new PasswordValidator($passwordPolicyObject)] - ) + [ + 'required' => true, + 'label' => $this->translate('Password'), + 'validators' => [new PasswordValidator($passwordPolicy)] + ] ); $this->setTitle($this->translate('Add a new user')); diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 0469a96341..312306483b 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -162,7 +162,7 @@ in order to manually create users directly inside the database. Icinga Web 2 supports password policies when using database authentication. You can configure this under **Configuration > Application > General**. -By default, no password policy is enforced ('None'). +By default, no password policy is enforced ('Any'). Icinga Web 2 provides a built-in policy called 'Common' with the following requirements: * Minimum length of 12 characters diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 34668c9063..b141917363 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -8,7 +8,7 @@ use Exception; use Icinga\Application\ProvidedHook\DbMigration; use Icinga\Application\ProvidedHook\CommonPasswordPolicy; -use Icinga\Application\ProvidedHook\NoPasswordPolicy; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; @@ -743,7 +743,7 @@ protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); Hook::register('passwordpolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); - Hook::register('passwordpolicy', NoPasswordPolicy::class, NoPasswordPolicy::class); + Hook::register('passwordpolicy', AnyPasswordPolicy::class, AnyPasswordPolicy::class); return $this; } diff --git a/library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php similarity index 83% rename from library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php rename to library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index b4d109b5f6..6ee285c2af 100644 --- a/library/Icinga/Application/ProvidedHook/NoPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -9,13 +9,13 @@ /** * None Password Policy to validate all passwords */ -class NoPasswordPolicy implements PasswordPolicyHook +class AnyPasswordPolicy implements PasswordPolicyHook { use Translation; public function getName(): string { - return $this->translate('None'); + return $this->translate('Any'); } public function getDescription(): string diff --git a/modules/password-policy b/modules/password-policy deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php similarity index 67% rename from test/php/library/Icinga/Application/NoPasswordPolicyTest.php rename to test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index 9d59332df6..fdf137e18c 100644 --- a/test/php/library/Icinga/Application/NoPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -4,20 +4,20 @@ use Icinga\Application\Hook\PasswordPolicyHook; use PHPUnit\Framework\TestCase; -use Icinga\Application\ProvidedHook\NoPasswordPolicy; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; -class NoPasswordPolicyTest extends TestCase +class AnyPasswordPolicyTest extends TestCase { private PasswordPolicyHook $instance; public function setUp(): void { - $this->instance = new NoPasswordPolicy(); + $this->instance = new AnyPasswordPolicy(); } public function testMethodGetName(): void { - $this->assertSame('None', $this->instance->getName()); + $this->assertSame('Any', $this->instance->getName()); } public function testValidatePasswordValid(): void From ba6972ded52d5c3e5fddbe05106c8cd24fc096c9 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 11 Sep 2025 13:52:51 +0200 Subject: [PATCH 24/54] codereview change description methods typ to return string or null --- application/forms/Account/ChangePasswordForm.php | 2 +- application/forms/Config/User/UserForm.php | 2 +- library/Icinga/Application/Hook/PasswordPolicyHook.php | 4 ++-- library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php | 4 ++-- .../Icinga/Application/ProvidedHook/CommonPasswordPolicy.php | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index be7fd98ba9..9830f13dd8 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -45,7 +45,7 @@ public function createElements(array $formData) $passwordPolicy = new $passwordPolicyClass(); $passwordPolicyDescription = $passwordPolicy->getDescription(); - if ($passwordPolicyDescription !== '') { + if ($passwordPolicyDescription !== null) { $this->addDescription($passwordPolicyDescription); } diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index dafa9257b7..6361aedbc1 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -28,7 +28,7 @@ protected function createInsertElements(array $formData) $passwordPolicy = new $passwordPolicyClass(); $passwordPolicyDescription = $passwordPolicy->getDescription(); - if ($passwordPolicyDescription != '') { + if ($passwordPolicyDescription !== null) { $this->addDescription($passwordPolicyDescription); } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index a6013eec70..96d1d65d50 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -15,9 +15,9 @@ public function getName(): string; /** * Displays the rules of the password policy for users * - * @return string + * @return string|null */ - public function getDescription(): string; + public function getDescription(): ?string; /** * Validate a given password against the defined policy diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 6ee285c2af..332f59ce76 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -18,9 +18,9 @@ public function getName(): string return $this->translate('Any'); } - public function getDescription(): string + public function getDescription(): ?string { - return ''; + return null; } public function validatePassword(string $password): array diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index f49bd1d1e1..b9b61ecf9b 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -25,7 +25,7 @@ public function getName(): string return $this->translate('Common'); } - public function getDescription(): string + public function getDescription(): ?string { return $this->translate( 'Password requirements: minimum 12 characters, ' . From 6b2ffd0babd7a2dd08256d6b5486911ae3951d8b Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 29 Sep 2025 09:39:06 +0200 Subject: [PATCH 25/54] implement Password Policy Helper Class to reduce code duplication --- .../forms/Account/ChangePasswordForm.php | 20 +++------- application/forms/Config/User/UserForm.php | 20 +++------- .../Application/ApplicationBootstrap.php | 14 +++---- .../ProvidedHook/AnyPasswordPolicy.php | 9 ++++- .../Authentication/PasswordPolicyHelper.php | 40 +++++++++++++++++++ .../Authentication/PasswordValidator.php | 3 +- .../Application/CommonPasswordPolicyTest.php | 3 -- 7 files changed, 66 insertions(+), 43 deletions(-) create mode 100644 library/Icinga/Authentication/PasswordPolicyHelper.php diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 9830f13dd8..36f04f795d 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -3,9 +3,7 @@ namespace Icinga\Forms\Account; -use Icinga\Application\Config; -use Icinga\Application\ProvidedHook\AnyPasswordPolicy; -use Icinga\Authentication\PasswordValidator; +use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; use Icinga\User; @@ -37,17 +35,9 @@ public function init() */ public function createElements(array $formData) { - $passwordPolicyClass = Config::app()->get( - 'global', - 'password_policy', - AnyPasswordPolicy::class - ); - - $passwordPolicy = new $passwordPolicyClass(); - $passwordPolicyDescription = $passwordPolicy->getDescription(); - if ($passwordPolicyDescription !== null) { - $this->addDescription($passwordPolicyDescription); - } + $helper = new PasswordPolicyHelper(); + $helper->addPasswordPolicyDescription($this); + $passwordValidator = $helper->getPasswordValidator(); $this->addElement( 'password', @@ -63,7 +53,7 @@ public function createElements(array $formData) [ 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => [new PasswordValidator($passwordPolicy)] + 'validators' => [$passwordValidator] ] ); $this->addElement( diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 6361aedbc1..01ab5461b7 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -3,10 +3,8 @@ namespace Icinga\Forms\Config\User; -use Icinga\Application\Config; use Icinga\Application\Hook\ConfigFormEventsHook; -use Icinga\Application\ProvidedHook\AnyPasswordPolicy; -use Icinga\Authentication\PasswordValidator; +use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; use Icinga\Web\Notification; @@ -20,17 +18,9 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $passwordPolicyClass = Config::app()->get( - 'global', - 'password_policy', - AnyPasswordPolicy::class - ); - - $passwordPolicy = new $passwordPolicyClass(); - $passwordPolicyDescription = $passwordPolicy->getDescription(); - if ($passwordPolicyDescription !== null) { - $this->addDescription($passwordPolicyDescription); - } + $helper = new PasswordPolicyHelper(); + $helper->addPasswordPolicyDescription($this); + $passwordValidator = $helper->getPasswordValidator(); $this->addElement( 'checkbox', @@ -55,7 +45,7 @@ protected function createInsertElements(array $formData) [ 'required' => true, 'label' => $this->translate('Password'), - 'validators' => [new PasswordValidator($passwordPolicy)] + 'validators' => [$passwordValidator] ] ); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index b141917363..9a916e4884 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -6,18 +6,18 @@ use DirectoryIterator; use ErrorException; use Exception; -use Icinga\Application\ProvidedHook\DbMigration; -use Icinga\Application\ProvidedHook\CommonPasswordPolicy; -use Icinga\Application\ProvidedHook\AnyPasswordPolicy; -use ipl\I18n\GettextTranslator; -use ipl\I18n\StaticTranslator; -use LogicException; use Icinga\Application\Modules\Manager as ModuleManager; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; +use Icinga\Application\ProvidedHook\DbMigration; use Icinga\Authentication\User\UserBackend; use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; -use Icinga\Exception\NotReadableError; use Icinga\Exception\IcingaException; +use Icinga\Exception\NotReadableError; +use ipl\I18n\GettextTranslator; +use ipl\I18n\StaticTranslator; +use LogicException; /** * This class bootstraps a thin Icinga application layer diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 332f59ce76..d9d7297f71 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -7,15 +7,20 @@ use ipl\I18n\Translation; /** - * None Password Policy to validate all passwords + * Policy to allow any password */ class AnyPasswordPolicy implements PasswordPolicyHook { use Translation; + /** + * Policy named 'none' to indicate that no password policy is enforced and any password is accepted + * + * @return string + */ public function getName(): string { - return $this->translate('Any'); + return $this->translate('None'); } public function getDescription(): ?string diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php new file mode 100644 index 0000000000..b23e6ab0d8 --- /dev/null +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -0,0 +1,40 @@ +get( + 'global', + 'password_policy', + self::DEFAULT_PASSWORD_POLICY + ); + + $this->passwordPolicy = new $passwordPolicyClass(); + } + + public function addPasswordPolicyDescription(Form $form): void + { + $description = $this->passwordPolicy->getDescription(); + if ($description !== null) { + $form->addDescription($description); + } + } + + public function getPasswordValidator(): PasswordValidator + { + return new PasswordValidator($this->passwordPolicy); + } +} diff --git a/library/Icinga/Authentication/PasswordValidator.php b/library/Icinga/Authentication/PasswordValidator.php index 8b9ecbd7c3..5ce5685d07 100644 --- a/library/Icinga/Authentication/PasswordValidator.php +++ b/library/Icinga/Authentication/PasswordValidator.php @@ -9,7 +9,7 @@ class PasswordValidator extends Zend_Validate_Abstract { /** - * The password policy object + * The password policy object * * @var PasswordPolicyHook */ @@ -39,6 +39,7 @@ public function isValid($value): bool if (!empty($message)) { $this->_messages = $message; + return false; } diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index dc2142d23a..b044e0ee82 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -91,6 +91,3 @@ public function testValidatePasswordWithManyCharacters(): void $this->assertCount(3, $res); } } - - - From 71090aec9b154a79a7ea7627ba67968f95b2effa Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 6 Oct 2025 13:44:42 +0200 Subject: [PATCH 26/54] fix failed unittest --- test/php/library/Icinga/Application/AnyPasswordPolicyTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index fdf137e18c..1ee728c6c3 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -17,7 +17,7 @@ public function setUp(): void public function testMethodGetName(): void { - $this->assertSame('Any', $this->instance->getName()); + $this->assertSame('None', $this->instance->getName()); } public function testValidatePasswordValid(): void From d7e36e372813524ad1e5e714fd5a10da66427b60 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 14 Oct 2025 11:17:45 +0200 Subject: [PATCH 27/54] Handle invalid password policy config --- .../forms/Account/ChangePasswordForm.php | 13 +++++++--- .../Authentication/PasswordPolicyHelper.php | 25 +++++++++++-------- .../Application/AnyPasswordPolicyTest.php | 5 ---- .../Application/CommonPasswordPolicyTest.php | 5 ---- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 36f04f795d..71f5f40c96 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -3,6 +3,7 @@ namespace Icinga\Forms\Account; +use Icinga\Application\Logger; use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -35,9 +36,15 @@ public function init() */ public function createElements(array $formData) { - $helper = new PasswordPolicyHelper(); - $helper->addPasswordPolicyDescription($this); - $passwordValidator = $helper->getPasswordValidator(); + try { + $helper = new PasswordPolicyHelper(); + $helper->addPasswordPolicyDescription($this); + $passwordValidator = $helper->getPasswordValidator(); + } catch (\Throwable $e) { + $passwordValidator = []; + Logger::error($e); + Notification::error("The configured password policy could not be found."); + } $this->addElement( 'password', diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index b23e6ab0d8..ff1746375b 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -3,6 +3,7 @@ namespace Icinga\Authentication; use Icinga\Application\Config; +use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Authentication\PasswordValidator; use Icinga\Web\Form; @@ -16,21 +17,25 @@ class PasswordPolicyHelper public function __construct() { - $passwordPolicyClass = Config::app()->get( - 'global', - 'password_policy', - self::DEFAULT_PASSWORD_POLICY - ); + $passwordPolicyClass = Config::app()->get( + 'global', + 'password_policy', + self::DEFAULT_PASSWORD_POLICY + ); - $this->passwordPolicy = new $passwordPolicyClass(); + $instance = new $passwordPolicyClass; + + if(class_exists($passwordPolicyClass) && ($instance instanceof PasswordPolicyHook)) { + $this->passwordPolicy = new $passwordPolicyClass(); + } } public function addPasswordPolicyDescription(Form $form): void { - $description = $this->passwordPolicy->getDescription(); - if ($description !== null) { - $form->addDescription($description); - } + $description = $this->passwordPolicy->getDescription(); + if ($description !== null) { + $form->addDescription($description); + } } public function getPasswordValidator(): PasswordValidator diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index 1ee728c6c3..59873d79a9 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -15,11 +15,6 @@ public function setUp(): void $this->instance = new AnyPasswordPolicy(); } - public function testMethodGetName(): void - { - $this->assertSame('None', $this->instance->getName()); - } - public function testValidatePasswordValid(): void { $res = $this->instance->validatePassword('icingaadmin'); diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index b044e0ee82..2af97f7523 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -15,11 +15,6 @@ public function setUp(): void $this->instance = new CommonPasswordPolicy(); } - public function testMethodGetName(): void - { - $this->assertSame('Common', $this->instance->getName()); - } - public function testValidatePasswordTooShort(): void { $expectedResults = ['Password must be at least 12 characters long']; From 76bacb67ca8af123550085f1a9d0da9fdfec2113 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 14 Oct 2025 11:35:37 +0200 Subject: [PATCH 28/54] Fix code sniffer violation --- .../Authentication/PasswordPolicyHelper.php | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index ff1746375b..964562a49e 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -17,25 +17,26 @@ class PasswordPolicyHelper public function __construct() { - $passwordPolicyClass = Config::app()->get( - 'global', - 'password_policy', - self::DEFAULT_PASSWORD_POLICY - ); + $passwordPolicyClass = Config::app()->get( + 'global', + 'password_policy', + self::DEFAULT_PASSWORD_POLICY + ); - $instance = new $passwordPolicyClass; + $instance = new $passwordPolicyClass; - if(class_exists($passwordPolicyClass) && ($instance instanceof PasswordPolicyHook)) { - $this->passwordPolicy = new $passwordPolicyClass(); - } + if (class_exists($passwordPolicyClass) && ($instance instanceof PasswordPolicyHook)) { + $this->passwordPolicy = new $passwordPolicyClass(); + } } public function addPasswordPolicyDescription(Form $form): void { - $description = $this->passwordPolicy->getDescription(); - if ($description !== null) { - $form->addDescription($description); - } + $description = $this->passwordPolicy->getDescription(); + + if ($description !== null) { + $form->addDescription($description); + } } public function getPasswordValidator(): PasswordValidator From 29b74f1232661c055bf3ac9f27eb8df4d569fc33 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 6 Nov 2025 15:45:22 +0100 Subject: [PATCH 29/54] Apply code review suggestions - Fix validator handeling - Adjust hook registration name --- application/forms/Account/ChangePasswordForm.php | 9 +++++---- .../Config/General/PasswordPolicyConfigForm.php | 6 +++--- application/forms/Config/User/UserForm.php | 16 ++++++++++++---- .../Icinga/Application/ApplicationBootstrap.php | 4 ++-- .../Application/Hook/PasswordPolicyHook.php | 2 +- .../Authentication/PasswordPolicyHelper.php | 11 ++++++++--- 6 files changed, 31 insertions(+), 17 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 71f5f40c96..5384975dd1 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -36,14 +36,15 @@ public function init() */ public function createElements(array $formData) { + $validators = []; + try { $helper = new PasswordPolicyHelper(); + $validators[] = $helper->getPasswordValidator(); $helper->addPasswordPolicyDescription($this); - $passwordValidator = $helper->getPasswordValidator(); } catch (\Throwable $e) { - $passwordValidator = []; Logger::error($e); - Notification::error("The configured password policy could not be found."); + Notification::error("The configured password policy could not be loaded."); } $this->addElement( @@ -60,7 +61,7 @@ public function createElements(array $formData) [ 'label' => $this->translate('New Password'), 'required' => true, - 'validators' => [$passwordValidator] + 'validators' => $validators ] ); $this->addElement( diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index 4c94a4bb2d..de48798b42 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -4,7 +4,7 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Hook; -use Icinga\Application\ProvidedHook\AnyPasswordPolicy; +use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Web\Form; /** @@ -22,7 +22,7 @@ public function init(): void public function createElements(array $formData): static { $passwordPolicies = []; - foreach (Hook::all('passwordpolicy') as $class => $policy) { + foreach (Hook::all('PasswordPolicy') as $class => $policy) { $passwordPolicies[$class] = $policy->getName(); } asort($passwordPolicies); @@ -33,7 +33,7 @@ public function createElements(array $formData): static [ 'description' => $this->translate('Enforce password requirements for new passwords'), 'label' => $this->translate('Password Policy'), - 'value' => AnyPasswordPolicy::class, + 'value' => PasswordPolicyHelper::DEFAULT_PASSWORD_POLICY, 'multiOptions' => $passwordPolicies ] ); diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 01ab5461b7..e610ef5394 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -4,6 +4,7 @@ namespace Icinga\Forms\Config\User; use Icinga\Application\Hook\ConfigFormEventsHook; +use Icinga\Application\Logger; use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; @@ -18,9 +19,16 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $helper = new PasswordPolicyHelper(); - $helper->addPasswordPolicyDescription($this); - $passwordValidator = $helper->getPasswordValidator(); + $validators = []; + + try { + $helper = new PasswordPolicyHelper(); + $validators[] = $helper->getPasswordValidator(); + $helper->addPasswordPolicyDescription($this); + } catch (\Throwable $e) { + Logger::error($e); + Notification::error("The configured password policy could not be loaded."); + } $this->addElement( 'checkbox', @@ -45,7 +53,7 @@ protected function createInsertElements(array $formData) [ 'required' => true, 'label' => $this->translate('Password'), - 'validators' => [$passwordValidator] + 'validators' => $validators ] ); diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 9a916e4884..3c66881e80 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -742,8 +742,8 @@ public function hasLocales() protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); - Hook::register('passwordpolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); - Hook::register('passwordpolicy', AnyPasswordPolicy::class, AnyPasswordPolicy::class); + Hook::register('PasswordPolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); + Hook::register('PasswordPolicy', AnyPasswordPolicy::class, AnyPasswordPolicy::class); return $this; } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 96d1d65d50..9398d8fc0c 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -24,7 +24,7 @@ public function getDescription(): ?string; * * @param string $password * @return string[] Returns an empty array if the password is valid, - * otherwise returns an error message describing the violations + * otherwise returns error messages describing the violations */ public function validatePassword(string $password): array; } diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 964562a49e..70f1d440de 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -6,6 +6,7 @@ use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Authentication\PasswordValidator; + use Icinga\Web\Form; class PasswordPolicyHelper @@ -23,10 +24,14 @@ public function __construct() self::DEFAULT_PASSWORD_POLICY ); - $instance = new $passwordPolicyClass; + if (class_exists($passwordPolicyClass)) { + $instance = new $passwordPolicyClass(); - if (class_exists($passwordPolicyClass) && ($instance instanceof PasswordPolicyHook)) { - $this->passwordPolicy = new $passwordPolicyClass(); + if ($instance instanceof PasswordPolicyHook) { + $this->passwordPolicy = $instance; + } + } else { + $this->passwordPolicy = self::DEFAULT_PASSWORD_POLICY; } } From 47acabf2ac67a800afca2103b967142c3a73a412 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Fri, 21 Nov 2025 13:26:58 +0100 Subject: [PATCH 30/54] Rearrange helper, split hook and interface --- .../forms/Account/ChangePasswordForm.php | 18 +--- .../General/PasswordPolicyConfigForm.php | 12 +-- application/forms/Config/User/UserForm.php | 18 +--- doc/05-Authentication.md | 16 ++-- .../Application/ApplicationBootstrap.php | 7 +- .../Application/Hook/PasswordPolicyHook.php | 48 ++++++---- .../ProvidedHook/AnyPasswordPolicy.php | 4 +- .../ProvidedHook/CommonPasswordPolicy.php | 5 +- .../Icinga/Authentication/PasswordPolicy.php | 30 ++++++ .../Authentication/PasswordPolicyHelper.php | 96 ++++++++++++++----- ...idator.php => PasswordPolicyValidator.php} | 17 ++-- library/Icinga/Web/StyleSheet.php | 1 + public/css/icinga/password-policy-helper.less | 13 +++ .../Application/AnyPasswordPolicyTest.php | 7 +- .../Application/CommonPasswordPolicyTest.php | 60 +++++++----- 15 files changed, 219 insertions(+), 133 deletions(-) create mode 100644 library/Icinga/Authentication/PasswordPolicy.php rename library/Icinga/Authentication/{PasswordValidator.php => PasswordPolicyValidator.php} (54%) create mode 100644 public/css/icinga/password-policy-helper.less diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 5384975dd1..12b3dffa92 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -3,7 +3,6 @@ namespace Icinga\Forms\Account; -use Icinga\Application\Logger; use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; @@ -36,17 +35,6 @@ public function init() */ public function createElements(array $formData) { - $validators = []; - - try { - $helper = new PasswordPolicyHelper(); - $validators[] = $helper->getPasswordValidator(); - $helper->addPasswordPolicyDescription($this); - } catch (\Throwable $e) { - Logger::error($e); - Notification::error("The configured password policy could not be loaded."); - } - $this->addElement( 'password', 'old_password', @@ -60,10 +48,12 @@ public function createElements(array $formData) 'new_password', [ 'label' => $this->translate('New Password'), - 'required' => true, - 'validators' => $validators + 'required' => true ] ); + + PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password'); + $this->addElement( 'password', 'new_password_confirmation', diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index de48798b42..63e5e6e262 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -3,7 +3,7 @@ namespace Icinga\Forms\Config\General; -use Icinga\Application\Hook; +use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Web\Form; @@ -21,20 +21,14 @@ public function init(): void public function createElements(array $formData): static { - $passwordPolicies = []; - foreach (Hook::all('PasswordPolicy') as $class => $policy) { - $passwordPolicies[$class] = $policy->getName(); - } - asort($passwordPolicies); - $this->addElement( 'select', - 'global_password_policy', + sprintf('%s_%s', PasswordPolicyHelper::CONFIG_SECTION, PasswordPolicyHelper::CONFIG_KEY), [ 'description' => $this->translate('Enforce password requirements for new passwords'), 'label' => $this->translate('Password Policy'), 'value' => PasswordPolicyHelper::DEFAULT_PASSWORD_POLICY, - 'multiOptions' => $passwordPolicies + 'multiOptions' => PasswordPolicyHook::all() ] ); diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index e610ef5394..5974b754ae 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -8,7 +8,9 @@ use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Data\Filter\Filter; use Icinga\Forms\RepositoryForm; +use Icinga\Web\Form\Element\Note; use Icinga\Web\Notification; +use Throwable; class UserForm extends RepositoryForm { @@ -19,17 +21,6 @@ class UserForm extends RepositoryForm */ protected function createInsertElements(array $formData) { - $validators = []; - - try { - $helper = new PasswordPolicyHelper(); - $validators[] = $helper->getPasswordValidator(); - $helper->addPasswordPolicyDescription($this); - } catch (\Throwable $e) { - Logger::error($e); - Notification::error("The configured password policy could not be loaded."); - } - $this->addElement( 'checkbox', 'is_active', @@ -52,11 +43,12 @@ protected function createInsertElements(array $formData) 'password', [ 'required' => true, - 'label' => $this->translate('Password'), - 'validators' => $validators + 'label' => $this->translate('Password') ] ); + PasswordPolicyHelper::applyPasswordPolicy($this, 'password'); + $this->setTitle($this->translate('Add a new user')); $this->setSubmitLabel($this->translate('Add')); } diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 312306483b..59157c5ffc 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -159,11 +159,12 @@ Please read [this chapter](20-Advanced-Topics.md#advanced-topics-authentication- in order to manually create users directly inside the database. ### Password Policy -Icinga Web 2 supports password policies when using database authentication. + +Icinga Web supports password policies when using database authentication. You can configure this under **Configuration > Application > General**. By default, no password policy is enforced ('Any'). -Icinga Web 2 provides a built-in policy called 'Common' with the following requirements: +Icinga Web provides a built-in policy called 'Common' with the following requirements: * Minimum length of 12 characters * At least one number @@ -172,6 +173,7 @@ Icinga Web 2 provides a built-in policy called 'Common' with the following requi * At least one lowercase letter #### Custom Password Policy + You can create custom password policies by developing a module with a provided hook. **Create Module Structure** @@ -190,8 +192,8 @@ Author: Your Name **Implement the Hook** -Icinga Web 2 provides the `PasswordPolicyHook` interface with predefined methods -that simplify the implementation of custom password policies. +Icinga Web provides the `PasswordPolicyHook` with predefined methods +that simplify the extension of custom password policies. Create `library/MyPasswordPolicy/ProvidedHook/PasswordPolicy.php`: @@ -200,7 +202,7 @@ namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; use Icinga\Application\Hook\PasswordPolicyHook; -class PasswordPolicy implements PasswordPolicyHook +class PasswordPolicy extends PasswordPolicyHook { public function getName(): string { @@ -212,7 +214,7 @@ class PasswordPolicy implements PasswordPolicyHook return 'Custom password requirements: 8+ chars, 1 number'; } - public function validatePassword(string $password): array + public function validate(string $password): array { $violations = []; @@ -235,7 +237,7 @@ Create `run.php`: ```php /** @var $this \Icinga\Application\Modules\Module */ -$this->provideHook('passwordpolicy', 'PasswordPolicy'); +MyPasswordPolicy::register(); ``` diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 3c66881e80..5338565e26 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -6,10 +6,12 @@ use DirectoryIterator; use ErrorException; use Exception; +use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\Modules\Manager as ModuleManager; use Icinga\Application\ProvidedHook\AnyPasswordPolicy; use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\DbMigration; +use Icinga\Authentication\PasswordPolicy; use Icinga\Authentication\User\UserBackend; use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; @@ -742,8 +744,9 @@ public function hasLocales() protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); - Hook::register('PasswordPolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); - Hook::register('PasswordPolicy', AnyPasswordPolicy::class, AnyPasswordPolicy::class); + CommonPasswordPolicy::register(); + AnyPasswordPolicy::register(); + return $this; } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 9398d8fc0c..d28a55535c 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -3,28 +3,38 @@ namespace Icinga\Application\Hook; -interface PasswordPolicyHook +use Icinga\Application\Hook; +use Icinga\Authentication\PasswordPolicy; + +/** + * This class connects with the PasswordPolicy interface to use it as hook + */ +abstract class PasswordPolicyHook implements PasswordPolicy { /** - * Get the name of the password policy - * - * @return string - */ - public function getName(): string; + * Registers Hook as Password Policy + * + * @return void + */ + public static function register(): void + { + Hook::register('PasswordPolicy', static::class, static::class); + } /** - * Displays the rules of the password policy for users - * - * @return string|null - */ - public function getDescription(): ?string; + * Returns all registered password policies sorted by + * + * @return array + */ + public static function all(): array + { + $passwordPolicies = []; - /** - * Validate a given password against the defined policy - * - * @param string $password - * @return string[] Returns an empty array if the password is valid, - * otherwise returns error messages describing the violations - */ - public function validatePassword(string $password): array; + foreach (Hook::all('PasswordPolicy') as $class => $policy) { + $passwordPolicies[$class] = $policy->getName(); + } + + asort($passwordPolicies); + return $passwordPolicies; + } } diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index d9d7297f71..8684a5abb3 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -9,7 +9,7 @@ /** * Policy to allow any password */ -class AnyPasswordPolicy implements PasswordPolicyHook +class AnyPasswordPolicy extends PasswordPolicyHook { use Translation; @@ -28,7 +28,7 @@ public function getDescription(): ?string return null; } - public function validatePassword(string $password): array + public function validate(string $password): array { return []; } diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index b9b61ecf9b..38606c2ae3 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -16,10 +16,11 @@ * - At least one uppercase letter * - At least one lowercase letter */ -class CommonPasswordPolicy implements PasswordPolicyHook +class CommonPasswordPolicy extends PasswordPolicyHook { use Translation; + public function getName(): string { return $this->translate('Common'); @@ -33,7 +34,7 @@ public function getDescription(): ?string ); } - public function validatePassword(string $password): array + public function validate(string $password): array { $violations = []; diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php new file mode 100644 index 0000000000..401f9fd68d --- /dev/null +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -0,0 +1,30 @@ +get( - 'global', - 'password_policy', - self::DEFAULT_PASSWORD_POLICY - ); - - if (class_exists($passwordPolicyClass)) { - $instance = new $passwordPolicyClass(); - - if ($instance instanceof PasswordPolicyHook) { - $this->passwordPolicy = $instance; - } - } else { - $this->passwordPolicy = self::DEFAULT_PASSWORD_POLICY; + try { + $passwordPolicyClass = Config::app()->get( + 'global', + 'password_policy', + static::DEFAULT_PASSWORD_POLICY + ); + + $passwordPolicy = static::createPolicy($passwordPolicyClass); + $form->getElement($element)->addValidator(new PasswordPolicyValidator($passwordPolicy)); + static::addPasswordPolicyDescription($form, $passwordPolicy); + } catch (ConfigurationError $e) { + Logger::error($e); + + $form->addElement( + 'note', + 'error message', + [ + 'escape' => false, + 'decorators' => ['ViewHelper'], + 'value' => sprintf( + '
%s +
%s
+
', + $form->getView()->icon('warning-empty'), + t('There was a problem loading the configured password policy.' . + 'Please contact your administrator.') + ), + ] + ); } } - public function addPasswordPolicyDescription(Form $form): void + public static function createPolicy(string $passwordPolicyClass): PasswordPolicy { - $description = $this->passwordPolicy->getDescription(); + if ($passwordPolicyClass === static::DEFAULT_PASSWORD_POLICY) { + return new $passwordPolicyClass; + } - if ($description !== null) { - $form->addDescription($description); + if (! class_exists($passwordPolicyClass)) { + throw new ConfigurationError(t('Password policy class %s does not exist'), $passwordPolicyClass); } + + $passwordPolicy = new $passwordPolicyClass(); + + if (! $passwordPolicy instanceof PasswordPolicy) { + throw new ConfigurationError(t('Password policy is not an instance of %s'), PasswordPolicy::class); + } + + return $passwordPolicy; } - public function getPasswordValidator(): PasswordValidator + public static function addPasswordPolicyDescription(Form $form, PasswordPolicy $passwordPolicy): void { - return new PasswordValidator($this->passwordPolicy); + $description = $passwordPolicy->getDescription(); + + if ($description !== null) { + $form->addDescription($description); + } } } diff --git a/library/Icinga/Authentication/PasswordValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php similarity index 54% rename from library/Icinga/Authentication/PasswordValidator.php rename to library/Icinga/Authentication/PasswordPolicyValidator.php index 5ce5685d07..366ff33153 100644 --- a/library/Icinga/Authentication/PasswordValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -3,26 +3,25 @@ namespace Icinga\Authentication; -use Icinga\Application\Hook\PasswordPolicyHook; use Zend_Validate_Abstract; -class PasswordValidator extends Zend_Validate_Abstract +class PasswordPolicyValidator extends Zend_Validate_Abstract { /** * The password policy object * - * @var PasswordPolicyHook + * @var PasswordPolicy */ - private PasswordPolicyHook $passwordPolicyObject; + protected PasswordPolicy $passwordPolicy; /** * Constructor * - * @param PasswordPolicyHook $passwordPolicyObject + * @param PasswordPolicy $passwordPolicy */ - public function __construct(PasswordPolicyHook $passwordPolicyObject) + public function __construct(PasswordPolicy $passwordPolicy) { - $this->passwordPolicyObject = $passwordPolicyObject; + $this->passwordPolicy = $passwordPolicy; } /** @@ -35,9 +34,9 @@ public function __construct(PasswordPolicyHook $passwordPolicyObject) */ public function isValid($value): bool { - $message = $this->passwordPolicyObject->validatePassword($value); + $message = $this->passwordPolicy->validate($value); - if (!empty($message)) { + if (! empty($message)) { $this->_messages = $message; return false; diff --git a/library/Icinga/Web/StyleSheet.php b/library/Icinga/Web/StyleSheet.php index 86b357835b..cdcfdf16c8 100644 --- a/library/Icinga/Web/StyleSheet.php +++ b/library/Icinga/Web/StyleSheet.php @@ -82,6 +82,7 @@ class StyleSheet 'css/icinga/health.less', 'css/icinga/php-diff.less', 'css/icinga/pending-migration.less', + 'css/icinga/password-policy-helper.less', ]; /** diff --git a/public/css/icinga/password-policy-helper.less b/public/css/icinga/password-policy-helper.less new file mode 100644 index 0000000000..56f6c2453d --- /dev/null +++ b/public/css/icinga/password-policy-helper.less @@ -0,0 +1,13 @@ +/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +.error-message-password-policy { + display:flex; + gap:8px; + align-items:center; + background:#FF5566FF; + color:#282e39; + border:1px solid; + padding:10px 12px; + border-radius:.25em; + margin-left: 14em; +} diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index 59873d79a9..e4cfb28fe4 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -2,13 +2,13 @@ namespace Tests\Icinga\Application; -use Icinga\Application\Hook\PasswordPolicyHook; +use Icinga\Authentication\PasswordPolicy; use PHPUnit\Framework\TestCase; use Icinga\Application\ProvidedHook\AnyPasswordPolicy; class AnyPasswordPolicyTest extends TestCase { - private PasswordPolicyHook $instance; + private PasswordPolicy $instance; public function setUp(): void { @@ -17,7 +17,6 @@ public function setUp(): void public function testValidatePasswordValid(): void { - $res = $this->instance->validatePassword('icingaadmin'); - $this->assertEmpty($res); + $this->assertEmpty($this->instance->validate('icingaadmin')); } } diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index 2af97f7523..6294b408eb 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -2,13 +2,13 @@ namespace Tests\Icinga\Application; -use Icinga\Application\Hook\PasswordPolicyHook; +use Icinga\Authentication\PasswordPolicy; use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use PHPUnit\Framework\TestCase; class CommonPasswordPolicyTest extends TestCase { - private PasswordPolicyHook $instance; + protected PasswordPolicy $instance; public function setUp(): void { @@ -17,54 +17,60 @@ public function setUp(): void public function testValidatePasswordTooShort(): void { - $expectedResults = ['Password must be at least 12 characters long']; - $res = $this->instance->validatePassword('Icinga1#'); - $this->assertSame($expectedResults, $res); + $this->assertSame( + ['Password must be at least 12 characters long'], + $this->instance->validate('Icinga1#') + ); } public function testValidatePasswordNoNumber(): void { - $expectedResults = ['Password must contain at least one number']; - $res = $this->instance->validatePassword('Icingaadmin#'); - $this->assertSame($expectedResults, $res); + $this->assertSame( + ['Password must contain at least one number'], + $this->instance->validate('Icingaadmin#') + ); } public function testValidatePasswordNoSpecialCharacter(): void { - $expectedResult = ['Password must contain at least one special character']; - $res = $this->instance->validatePassword('Icingaadmin1'); - $this->assertSame($expectedResult, $res); + $this->assertSame( + ['Password must contain at least one special character'], + $this->instance->validate('Icingaadmin1') + ); } public function testValidatePasswordNoUpperCaseLetters(): void { - $expectedResult = ['Password must contain at least one uppercase letter']; - $res = $this->instance->validatePassword('icingaadmin1#'); - $this->assertSame($expectedResult, $res); + $this->assertSame( + ['Password must contain at least one uppercase letter'], + $this->instance->validate('icingaadmin1#') + ); } public function testValidatePasswordNoLowerCaseLetters(): void { - $expectedResult = ['Password must contain at least one lowercase letter']; - $res = $this->instance->validatePassword('ICINGAADMIN1#'); - $this->assertSame($expectedResult, $res); + $this->assertSame( + ['Password must contain at least one lowercase letter'], + $this->instance->validate('ICINGAADMIN1#') + ); } public function testMethodValidatePasswordAlwaysReturnAnEmptyArray(): void { - $res = $this->instance->validatePassword('Icingaadmin1#'); - $this->assertEmpty($res); + $this->assertEmpty($this->instance->validate('Icingaadmin1#')); } public function testValidatePasswordOnlyLowerCaseLetters(): void { - $expectedResult = [ + $expected = [ 'Password must contain at least one number', 'Password must contain at least one special character', 'Password must contain at least one uppercase letter' ]; - $res = $this->instance->validatePassword('icingawebadmin'); - $this->assertSame($expectedResult, $res); + $this->assertSame( + $expected, + $this->instance->validate('icingawebadmin') + ); } public function testValidatePasswordWithLengthAndUpperCaseLetters(): void @@ -75,14 +81,16 @@ public function testValidatePasswordWithLengthAndUpperCaseLetters(): void 'Password must contain at least one special character', 'Password must contain at least one lowercase letter', ]; - $res = $this->instance->validatePassword('ICINGAADMIN'); - $this->assertSame($expectedResult, $res); + $res = + $this->assertSame( + $expectedResult, + $this->instance->validate('ICINGAADMIN') + ); } public function testValidatePasswordWithManyCharacters(): void { $longPassword = str_repeat('a', 1000); - $res = $this->instance->validatePassword($longPassword); - $this->assertCount(3, $res); + $this->assertCount(3, $this->instance->validate($longPassword)); } } From 409137e2c71707723ec5197eb8c296b0a77a6376 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 26 Jan 2026 16:21:51 +0100 Subject: [PATCH 31/54] Adjust validation to accept oldPassword --- application/forms/Account/ChangePasswordForm.php | 2 +- application/forms/Config/User/UserForm.php | 2 +- doc/05-Authentication.md | 10 +++++++--- .../ProvidedHook/AnyPasswordPolicy.php | 2 +- .../ProvidedHook/CommonPasswordPolicy.php | 12 ++++++------ library/Icinga/Authentication/PasswordPolicy.php | 5 +++-- .../Authentication/PasswordPolicyHelper.php | 9 +++++++-- .../Authentication/PasswordPolicyValidator.php | 15 ++++++++++++--- .../Icinga/Application/AnyPasswordPolicyTest.php | 2 +- 9 files changed, 39 insertions(+), 20 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 12b3dffa92..284a954499 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -52,7 +52,7 @@ public function createElements(array $formData) ] ); - PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password'); + PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password', 'old_password'); $this->addElement( 'password', diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 5974b754ae..421d3be194 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -47,7 +47,7 @@ protected function createInsertElements(array $formData) ] ); - PasswordPolicyHelper::applyPasswordPolicy($this, 'password'); + PasswordPolicyHelper::applyPasswordPolicy($this, 'password', null); $this->setTitle($this->translate('Add a new user')); $this->setSubmitLabel($this->translate('Add')); diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 59157c5ffc..8df17efd50 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -214,17 +214,21 @@ class PasswordPolicy extends PasswordPolicyHook return 'Custom password requirements: 8+ chars, 1 number'; } - public function validate(string $password): array + public function validate(string $newPassword, ?string $oldPassword): array; { $violations = []; - if (strlen($password) < 8) { + if (strlen($newPassword) < 8) { $violations[] = 'Password must be at least 8 characters'; } - if (!preg_match('/[0-9]/', $password)) { + if (!preg_match('/[0-9]/', $newPassword)) { $violations[] = 'Password must contain at least one number'; } + + if ($oldPassword !== null && hash_equals($oldPassword, $newPassword)) { + $violations[] = 'New password must be different from the old password'; + } return $violations; } diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 8684a5abb3..7749dc1753 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -28,7 +28,7 @@ public function getDescription(): ?string return null; } - public function validate(string $password): array + public function validate(string $newPassword, ?string $oldPassword): array { return []; } diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index 38606c2ae3..fd161fc644 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -34,27 +34,27 @@ public function getDescription(): ?string ); } - public function validate(string $password): array + public function validate(string $newPassword, ?string $oldPassword): array { $violations = []; - if (mb_strlen($password) < 12) { + if (mb_strlen($newPassword) < 12) { $violations[] = $this->translate('Password must be at least 12 characters long'); } - if (! preg_match('/[0-9]/', $password)) { + if (! preg_match('/[0-9]/', $newPassword)) { $violations[] = $this->translate('Password must contain at least one number'); } - if (! preg_match('/[^a-zA-Z0-9]/', $password)) { + if (! preg_match('/[^a-zA-Z0-9]/', $newPassword)) { $violations[] = $this->translate('Password must contain at least one special character'); } - if (! preg_match('/[A-Z]/', $password)) { + if (! preg_match('/[A-Z]/', $newPassword)) { $violations[] = $this->translate('Password must contain at least one uppercase letter'); } - if (! preg_match('/[a-z]/', $password)) { + if (! preg_match('/[a-z]/', $newPassword)) { $violations[] = $this->translate('Password must contain at least one lowercase letter'); } diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php index 401f9fd68d..f55b509a53 100644 --- a/library/Icinga/Authentication/PasswordPolicy.php +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -22,9 +22,10 @@ public function getDescription(): ?string; /** * Validate a given password against the defined policy * - * @param string $password + * @param string $newPassword + * @param string|null $oldPassword * @return string[] Returns an empty array if the password is valid, * otherwise returns error messages describing the violations */ - public function validate(string $password): array; + public function validate(string $newPassword, ?string $oldPassword): array; } diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 54d37ab045..8d805ee3d8 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -31,7 +31,7 @@ class PasswordPolicyHelper */ const CONFIG_KEY = 'password_policy'; - public static function applyPasswordPolicy(Form $form, string $element) + public static function applyPasswordPolicy(Form $form, string $newPassword, ?string $oldPassword) { try { $passwordPolicyClass = Config::app()->get( @@ -41,7 +41,12 @@ public static function applyPasswordPolicy(Form $form, string $element) ); $passwordPolicy = static::createPolicy($passwordPolicyClass); - $form->getElement($element)->addValidator(new PasswordPolicyValidator($passwordPolicy)); + +// if($oldPassword !== null) { + $form->getElement($oldPassword); +// } + + $form->getElement($newPassword)->addValidator(new PasswordPolicyValidator($passwordPolicy)); static::addPasswordPolicyDescription($form, $passwordPolicy); } catch (ConfigurationError $e) { Logger::error($e); diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 366ff33153..080eb3c6c2 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -29,12 +29,21 @@ public function __construct(PasswordPolicy $passwordPolicy) * throws a message if not * * @param mixed $value The password to validate - * + * @param mixed|null $context The data of the form * @return bool */ - public function isValid($value): bool + public function isValid($value, mixed $context = null): bool { - $message = $this->passwordPolicy->validate($value); + $oldPassword = null; + + if (is_array($context)) { + $oldPasswordValue = $context['old_password'] ?? null; + if ($oldPasswordValue !== null && $oldPasswordValue !== '') { + $oldPassword = $oldPasswordValue; + } + } + + $message = $this->passwordPolicy->validate($value, $oldPassword); if (! empty($message)) { $this->_messages = $message; diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index e4cfb28fe4..796b729913 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -17,6 +17,6 @@ public function setUp(): void public function testValidatePasswordValid(): void { - $this->assertEmpty($this->instance->validate('icingaadmin')); + $this->assertEmpty($this->instance->validate('icingaadmin', 'null')); } } From 81e6859b94587ce69174537959a76b0cc7a99693 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 26 Jan 2026 16:55:39 +0100 Subject: [PATCH 32/54] Adjust Test with second value --- .../Application/CommonPasswordPolicyTest.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index 6294b408eb..5a638384fd 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -19,7 +19,7 @@ public function testValidatePasswordTooShort(): void { $this->assertSame( ['Password must be at least 12 characters long'], - $this->instance->validate('Icinga1#') + $this->instance->validate('Icinga1#', 'null') ); } @@ -27,7 +27,7 @@ public function testValidatePasswordNoNumber(): void { $this->assertSame( ['Password must contain at least one number'], - $this->instance->validate('Icingaadmin#') + $this->instance->validate('Icingaadmin#', 'null') ); } @@ -35,7 +35,7 @@ public function testValidatePasswordNoSpecialCharacter(): void { $this->assertSame( ['Password must contain at least one special character'], - $this->instance->validate('Icingaadmin1') + $this->instance->validate('Icingaadmin1', 'null') ); } @@ -43,7 +43,7 @@ public function testValidatePasswordNoUpperCaseLetters(): void { $this->assertSame( ['Password must contain at least one uppercase letter'], - $this->instance->validate('icingaadmin1#') + $this->instance->validate('icingaadmin1#', 'null') ); } @@ -51,13 +51,13 @@ public function testValidatePasswordNoLowerCaseLetters(): void { $this->assertSame( ['Password must contain at least one lowercase letter'], - $this->instance->validate('ICINGAADMIN1#') + $this->instance->validate('ICINGAADMIN1#', 'null') ); } public function testMethodValidatePasswordAlwaysReturnAnEmptyArray(): void { - $this->assertEmpty($this->instance->validate('Icingaadmin1#')); + $this->assertEmpty($this->instance->validate('Icingaadmin1#', 'null')); } public function testValidatePasswordOnlyLowerCaseLetters(): void @@ -69,7 +69,7 @@ public function testValidatePasswordOnlyLowerCaseLetters(): void ]; $this->assertSame( $expected, - $this->instance->validate('icingawebadmin') + $this->instance->validate('icingawebadmin', 'null') ); } @@ -84,13 +84,13 @@ public function testValidatePasswordWithLengthAndUpperCaseLetters(): void $res = $this->assertSame( $expectedResult, - $this->instance->validate('ICINGAADMIN') + $this->instance->validate('ICINGAADMIN', 'null') ); } public function testValidatePasswordWithManyCharacters(): void { $longPassword = str_repeat('a', 1000); - $this->assertCount(3, $this->instance->validate($longPassword)); + $this->assertCount(3, $this->instance->validate($longPassword, 'null')); } } From 7193bab7918a3bc738e4198257ffdd32b6ad1b62 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Fri, 30 Jan 2026 09:41:42 +0100 Subject: [PATCH 33/54] code-review --- .../forms/Account/ChangePasswordForm.php | 5 +- application/forms/Config/User/UserForm.php | 7 +- doc/05-Authentication.md | 32 ++++---- .../Application/ApplicationBootstrap.php | 7 +- .../Application/Hook/PasswordPolicyHook.php | 28 ++++--- .../ProvidedHook/AnyPasswordPolicy.php | 8 +- .../ProvidedHook/CommonPasswordPolicy.php | 3 +- .../Icinga/Authentication/PasswordPolicy.php | 13 +++- .../Authentication/PasswordPolicyHelper.php | 75 ++++++++++++------- .../PasswordPolicyValidator.php | 27 ++++--- library/Icinga/Web/StyleSheet.php | 1 - public/css/icinga/password-policy-helper.less | 13 ---- .../Application/AnyPasswordPolicyTest.php | 2 +- .../Application/CommonPasswordPolicyTest.php | 27 +++---- 14 files changed, 129 insertions(+), 119 deletions(-) delete mode 100644 public/css/icinga/password-policy-helper.less diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 284a954499..568192ca93 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -46,12 +46,11 @@ public function createElements(array $formData) $this->addElement( 'password', 'new_password', - [ + array( 'label' => $this->translate('New Password'), 'required' => true - ] + ) ); - PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password', 'old_password'); $this->addElement( diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 421d3be194..3bccc86c27 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -41,13 +41,12 @@ protected function createInsertElements(array $formData) $this->addElement( 'password', 'password', - [ + array( 'required' => true, 'label' => $this->translate('Password') - ] + ) ); - - PasswordPolicyHelper::applyPasswordPolicy($this, 'password', null); + PasswordPolicyHelper::applyPasswordPolicy($this, 'password'); $this->setTitle($this->translate('Add a new user')); $this->setSubmitLabel($this->translate('Add')); diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 8df17efd50..bfc7488af9 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -163,8 +163,8 @@ in order to manually create users directly inside the database. Icinga Web supports password policies when using database authentication. You can configure this under **Configuration > Application > General**. -By default, no password policy is enforced ('Any'). -Icinga Web provides a built-in policy called 'Common' with the following requirements: +By default, no password policy is enforced (`Any`). +Icinga Web provides a built-in policy called `Common` with the following requirements: * Minimum length of 12 characters * At least one number @@ -178,16 +178,16 @@ You can create custom password policies by developing a module with a provided h **Create Module Structure** ```bash -mkdir -p /usr/share/icingaweb2/modules/mypasswordpolicy/library/MyPasswordPolicy/ProvidedHook +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 Version: 1.0.0 Description: Custom password policy implementation -Author: Your Name ``` **Implement the Hook** @@ -195,15 +195,19 @@ Author: Your Name Icinga Web provides the `PasswordPolicyHook` with predefined methods that simplify the extension of custom password policies. -Create `library/MyPasswordPolicy/ProvidedHook/PasswordPolicy.php`: +Create `library/Mypasswordpolicy/ProvidedHook/PasswordPolicy.php`: ```php -namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; + ``` **Register the Hook** Create `run.php`: -```php -/** @var $this \Icinga\Application\Modules\Module */ +```php + ``` - Enable the module: ```bash icingacli module enable mypasswordpolicy diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 5338565e26..b804f130e2 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -6,12 +6,8 @@ use DirectoryIterator; use ErrorException; use Exception; -use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\Modules\Manager as ModuleManager; -use Icinga\Application\ProvidedHook\AnyPasswordPolicy; -use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\DbMigration; -use Icinga\Authentication\PasswordPolicy; use Icinga\Authentication\User\UserBackend; use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; @@ -20,6 +16,8 @@ use ipl\I18n\GettextTranslator; use ipl\I18n\StaticTranslator; use LogicException; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; /** * This class bootstraps a thin Icinga application layer @@ -747,7 +745,6 @@ protected function registerApplicationHooks(): self CommonPasswordPolicy::register(); AnyPasswordPolicy::register(); - return $this; } } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index d28a55535c..35e17dc31f 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -7,34 +7,38 @@ use Icinga\Authentication\PasswordPolicy; /** - * This class connects with the PasswordPolicy interface to use it as hook + * Base class for hookable password policies */ abstract class PasswordPolicyHook implements PasswordPolicy { /** - * Registers Hook as Password Policy - * - * @return void - */ + * Hook name + */ + protected const HOOK_NAME = 'PasswordPolicy'; + + /** + * Register password policy + * + * @return void + */ public static function register(): void { - Hook::register('PasswordPolicy', static::class, static::class); + Hook::register(self::HOOK_NAME, static::class, static::class); } /** - * Returns all registered password policies sorted by - * - * @return array - */ + * Return all registered password policies sorted by name + * + * @return array + */ public static function all(): array { $passwordPolicies = []; - foreach (Hook::all('PasswordPolicy') as $class => $policy) { $passwordPolicies[$class] = $policy->getName(); } - asort($passwordPolicies); + return $passwordPolicies; } } diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 7749dc1753..9915d4bec9 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -13,13 +13,9 @@ class AnyPasswordPolicy extends PasswordPolicyHook { use Translation; - /** - * Policy named 'none' to indicate that no password policy is enforced and any password is accepted - * - * @return string - */ public function getName(): string { + // Policy is named 'None' to indicate that no password policy is enforced and any password is accepted return $this->translate('None'); } @@ -28,7 +24,7 @@ public function getDescription(): ?string return null; } - public function validate(string $newPassword, ?string $oldPassword): array + public function validate(string $newPassword, ?string $oldPassword = null): array { return []; } diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index fd161fc644..d930b70f51 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -20,7 +20,6 @@ class CommonPasswordPolicy extends PasswordPolicyHook { use Translation; - public function getName(): string { return $this->translate('Common'); @@ -34,7 +33,7 @@ public function getDescription(): ?string ); } - public function validate(string $newPassword, ?string $oldPassword): array + public function validate(string $newPassword, ?string $oldPassword = null): array { $violations = []; diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php index f55b509a53..b16039c454 100644 --- a/library/Icinga/Authentication/PasswordPolicy.php +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -8,14 +8,19 @@ interface PasswordPolicy /** * Get the name of the password policy * + * Displayed when configuring a password policy. + * * @return string */ public function getName(): string; /** - * Displays the rules of the password policy for users - * - * @return string|null + * * Get the description of the password policy + * * + * * Displayed when creating or changing passwords while the policy is active. + * * Should contain the rules of the policy. + * * + * * @return ?string */ public function getDescription(): ?string; @@ -27,5 +32,5 @@ public function getDescription(): ?string; * @return string[] Returns an empty array if the password is valid, * otherwise returns error messages describing the violations */ - public function validate(string $newPassword, ?string $oldPassword): array; + public function validate(string $newPassword, ?string $oldPassword = null): array; } diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 8d805ee3d8..836ac598e6 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -1,5 +1,5 @@ getElement($oldPasswordElementName) === null) { + throw new LogicException(); + } try { $passwordPolicyClass = Config::app()->get( 'global', @@ -41,35 +48,43 @@ public static function applyPasswordPolicy(Form $form, string $newPassword, ?str ); $passwordPolicy = static::createPolicy($passwordPolicyClass); - -// if($oldPassword !== null) { - $form->getElement($oldPassword); -// } - - $form->getElement($newPassword)->addValidator(new PasswordPolicyValidator($passwordPolicy)); + // getElement() may return null if the element does not exist, causing this call to fail. + $form->getElement($newPasswordElementName)->addValidator( + new PasswordPolicyValidator($passwordPolicy, $oldPasswordElementName) + ); static::addPasswordPolicyDescription($form, $passwordPolicy); } catch (ConfigurationError $e) { Logger::error($e); $form->addElement( 'note', - 'error message', + 'bogus', [ - 'escape' => false, - 'decorators' => ['ViewHelper'], - 'value' => sprintf( - '
%s -
%s
-
', - $form->getView()->icon('warning-empty'), - t('There was a problem loading the configured password policy.' . - 'Please contact your administrator.') + 'decorators' => [ + 'ViewHelper', + [['HtmlTag#text' => 'HtmlTag'], ['tag' => 'div']], + [ + ['HtmlTag#i' => 'HtmlTag'], + [ + 'tag' => 'i', + 'class' => 'form-notification-icon icon fa fa-circle-exclamation', + 'placement' => 'prepend', + ], + ], + [['HtmlTag#div' => 'HtmlTag'], ['tag' => 'div', 'class' => 'form-notifications error']], + ], + 'value' => t( + 'There was a problem loading the configured password policy. ' + . 'Please contact your administrator.' ), ] ); } } - +/** + * Create and validate a password policy instance from the given class name and ensure it implements the + * password policy interface. + */ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy { if ($passwordPolicyClass === static::DEFAULT_PASSWORD_POLICY) { @@ -83,7 +98,11 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy $passwordPolicy = new $passwordPolicyClass(); if (! $passwordPolicy instanceof PasswordPolicy) { - throw new ConfigurationError(t('Password policy is not an instance of %s'), PasswordPolicy::class); + throw new ConfigurationError( + t('Password policy %s is not an instance of %s'), + $passwordPolicyClass, + PasswordPolicy::class + ); } return $passwordPolicy; diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 080eb3c6c2..7c984d384a 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -5,6 +5,12 @@ use Zend_Validate_Abstract; +/** + * Use the provided password policy to validate the new password. + * Optionally, retrieve the old password from the form context using the configured form element name + * and pass it to the policy for comparative validation. + * Delegate all validation logic to the policy instance and expose any returned violation messages. + */ class PasswordPolicyValidator extends Zend_Validate_Abstract { /** @@ -14,30 +20,31 @@ class PasswordPolicyValidator extends Zend_Validate_Abstract */ protected PasswordPolicy $passwordPolicy; + /** + * Name of the old password element + * + * @var string|null + */ + protected ?string $oldPasswordElementName; + /** * Constructor * * @param PasswordPolicy $passwordPolicy + * @param string|null $oldPasswordElementName */ - public function __construct(PasswordPolicy $passwordPolicy) + public function __construct(PasswordPolicy $passwordPolicy, ?string $oldPasswordElementName = null) { $this->passwordPolicy = $passwordPolicy; + $this->oldPasswordElementName = $oldPasswordElementName; } - /** - * Checks if password matches with password policy - * throws a message if not - * - * @param mixed $value The password to validate - * @param mixed|null $context The data of the form - * @return bool - */ public function isValid($value, mixed $context = null): bool { $oldPassword = null; if (is_array($context)) { - $oldPasswordValue = $context['old_password'] ?? null; + $oldPasswordValue = $context[$this->oldPasswordElementName] ?? null; if ($oldPasswordValue !== null && $oldPasswordValue !== '') { $oldPassword = $oldPasswordValue; } diff --git a/library/Icinga/Web/StyleSheet.php b/library/Icinga/Web/StyleSheet.php index cdcfdf16c8..86b357835b 100644 --- a/library/Icinga/Web/StyleSheet.php +++ b/library/Icinga/Web/StyleSheet.php @@ -82,7 +82,6 @@ class StyleSheet 'css/icinga/health.less', 'css/icinga/php-diff.less', 'css/icinga/pending-migration.less', - 'css/icinga/password-policy-helper.less', ]; /** diff --git a/public/css/icinga/password-policy-helper.less b/public/css/icinga/password-policy-helper.less deleted file mode 100644 index 56f6c2453d..0000000000 --- a/public/css/icinga/password-policy-helper.less +++ /dev/null @@ -1,13 +0,0 @@ -/* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ - -.error-message-password-policy { - display:flex; - gap:8px; - align-items:center; - background:#FF5566FF; - color:#282e39; - border:1px solid; - padding:10px 12px; - border-radius:.25em; - margin-left: 14em; -} diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index 796b729913..887f2f9dc9 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -8,7 +8,7 @@ class AnyPasswordPolicyTest extends TestCase { - private PasswordPolicy $instance; + private passwordPolicy $instance; public function setUp(): void { diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index 5a638384fd..03cc957a13 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -8,7 +8,7 @@ class CommonPasswordPolicyTest extends TestCase { - protected PasswordPolicy $instance; + protected passwordPolicy $instance; public function setUp(): void { @@ -19,7 +19,7 @@ public function testValidatePasswordTooShort(): void { $this->assertSame( ['Password must be at least 12 characters long'], - $this->instance->validate('Icinga1#', 'null') + $this->instance->validate('Icinga1#') ); } @@ -27,7 +27,7 @@ public function testValidatePasswordNoNumber(): void { $this->assertSame( ['Password must contain at least one number'], - $this->instance->validate('Icingaadmin#', 'null') + $this->instance->validate('Icingaadmin#') ); } @@ -35,7 +35,7 @@ public function testValidatePasswordNoSpecialCharacter(): void { $this->assertSame( ['Password must contain at least one special character'], - $this->instance->validate('Icingaadmin1', 'null') + $this->instance->validate('Icingaadmin1') ); } @@ -43,7 +43,7 @@ public function testValidatePasswordNoUpperCaseLetters(): void { $this->assertSame( ['Password must contain at least one uppercase letter'], - $this->instance->validate('icingaadmin1#', 'null') + $this->instance->validate('icingaadmin1#') ); } @@ -51,13 +51,13 @@ public function testValidatePasswordNoLowerCaseLetters(): void { $this->assertSame( ['Password must contain at least one lowercase letter'], - $this->instance->validate('ICINGAADMIN1#', 'null') + $this->instance->validate('ICINGAADMIN1#') ); } - public function testMethodValidatePasswordAlwaysReturnAnEmptyArray(): void + public function testValidatePasswordValid(): void { - $this->assertEmpty($this->instance->validate('Icingaadmin1#', 'null')); + $this->assertEmpty($this->instance->validate('Icingaadmin1#')); } public function testValidatePasswordOnlyLowerCaseLetters(): void @@ -69,7 +69,7 @@ public function testValidatePasswordOnlyLowerCaseLetters(): void ]; $this->assertSame( $expected, - $this->instance->validate('icingawebadmin', 'null') + $this->instance->validate('icingawebadmin') ); } @@ -81,16 +81,9 @@ public function testValidatePasswordWithLengthAndUpperCaseLetters(): void 'Password must contain at least one special character', 'Password must contain at least one lowercase letter', ]; - $res = $this->assertSame( $expectedResult, - $this->instance->validate('ICINGAADMIN', 'null') + $this->instance->validate('ICINGAADMIN') ); } - - public function testValidatePasswordWithManyCharacters(): void - { - $longPassword = str_repeat('a', 1000); - $this->assertCount(3, $this->instance->validate($longPassword, 'null')); - } } From 4dabe410cb1ed9b27d539a89d7bf3bd28159f0b7 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:42:11 +0100 Subject: [PATCH 34/54] Update application/forms/Config/General/PasswordPolicyConfigForm.php Co-authored-by: Eric Lippmann --- application/forms/Config/General/PasswordPolicyConfigForm.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php index 63e5e6e262..b8b719df2e 100644 --- a/application/forms/Config/General/PasswordPolicyConfigForm.php +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: GPL-3.0-or-later namespace Icinga\Forms\Config\General; From c29a90adb348f992d13fa3fe44dc037491fc6ec4 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:43:17 +0100 Subject: [PATCH 35/54] Update doc/05-Authentication.md Co-authored-by: Eric Lippmann --- doc/05-Authentication.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index bfc7488af9..ca5397e0fd 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -202,7 +202,8 @@ Create `library/Mypasswordpolicy/ProvidedHook/PasswordPolicy.php`: namespace Icinga\Module\Mypasswordpolicy\ProvidedHook; -use Icinga\Application\Hook\PasswordPolicyHook;use ipl\I18n\Translation; +use Icinga\Application\Hook\PasswordPolicyHook; +use ipl\I18n\Translation; class PasswordPolicy extends PasswordPolicyHook { From 74e549d3ed3e89d05d5088bd973d6791b7c62d0a Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:43:44 +0100 Subject: [PATCH 36/54] Update test/php/library/Icinga/Application/AnyPasswordPolicyTest.php Co-authored-by: Eric Lippmann --- test/php/library/Icinga/Application/AnyPasswordPolicyTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php index 887f2f9dc9..2e03d546d4 100644 --- a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -8,7 +8,7 @@ class AnyPasswordPolicyTest extends TestCase { - private passwordPolicy $instance; + protected passwordPolicy $instance; public function setUp(): void { From ca5f843c0782501a250e141abd6844e71e140c13 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:44:06 +0100 Subject: [PATCH 37/54] Update library/Icinga/Authentication/PasswordPolicyValidator.php Co-authored-by: Eric Lippmann --- library/Icinga/Authentication/PasswordPolicyValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 7c984d384a..f575f6ca30 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -28,7 +28,7 @@ class PasswordPolicyValidator extends Zend_Validate_Abstract protected ?string $oldPasswordElementName; /** - * Constructor + * Create a new PasswordPolicyValidator * * @param PasswordPolicy $passwordPolicy * @param string|null $oldPasswordElementName From 812646ee11db4246778bd671e5001f28a061c635 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:44:27 +0100 Subject: [PATCH 38/54] Update library/Icinga/Authentication/PasswordPolicyValidator.php Co-authored-by: Eric Lippmann --- library/Icinga/Authentication/PasswordPolicyValidator.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index f575f6ca30..76b5ab7642 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -13,11 +13,7 @@ */ class PasswordPolicyValidator extends Zend_Validate_Abstract { - /** - * The password policy object - * - * @var PasswordPolicy - */ + /** @var PasswordPolicy Policy to use for validation */ protected PasswordPolicy $passwordPolicy; /** From 315570fb7ab3d73822461c71d02daa3d727e5646 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:44:45 +0100 Subject: [PATCH 39/54] Update library/Icinga/Authentication/PasswordPolicyValidator.php Co-authored-by: Eric Lippmann --- library/Icinga/Authentication/PasswordPolicyValidator.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 76b5ab7642..1dbf37ebf6 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -16,11 +16,7 @@ class PasswordPolicyValidator extends Zend_Validate_Abstract /** @var PasswordPolicy Policy to use for validation */ protected PasswordPolicy $passwordPolicy; - /** - * Name of the old password element - * - * @var string|null - */ + /** @var ?string Name of the old password form element */ protected ?string $oldPasswordElementName; /** From b022b1835d96d860e380d22a22b59a6ccaf6589f Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:45:39 +0100 Subject: [PATCH 40/54] Update doc/05-Authentication.md Co-authored-by: Eric Lippmann --- doc/05-Authentication.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index ca5397e0fd..73a3bb8f4e 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -252,6 +252,7 @@ Mypasswordpolicy::register(); ?> ``` + Enable the module: ```bash icingacli module enable mypasswordpolicy From 408f1e5615ddc5f9f83a1151c6ca22f0e47f355a Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:46:27 +0100 Subject: [PATCH 41/54] Update doc/05-Authentication.md Co-authored-by: Eric Lippmann --- doc/05-Authentication.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index 73a3bb8f4e..ac57635420 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -244,6 +244,7 @@ class PasswordPolicy extends PasswordPolicyHook **Register the Hook** Create `run.php`: + ```php Date: Fri, 20 Mar 2026 14:47:27 +0100 Subject: [PATCH 42/54] Update doc/05-Authentication.md Co-authored-by: Eric Lippmann --- doc/05-Authentication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index ac57635420..bc7ff288d8 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -255,7 +255,7 @@ Mypasswordpolicy::register(); Enable the module: -```bash +```shell icingacli module enable mypasswordpolicy ``` From 37e7ed69a8eb1a0abd9d159bf6a1f60ebd7e867f Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:48:11 +0100 Subject: [PATCH 43/54] Update library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php Co-authored-by: Eric Lippmann --- library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 9915d4bec9..5b69133372 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -15,7 +15,7 @@ class AnyPasswordPolicy extends PasswordPolicyHook public function getName(): string { - // Policy is named 'None' to indicate that no password policy is enforced and any password is accepted + // Policy is named 'None' to indicate that no password policy is enforced and any password is accepted. return $this->translate('None'); } From 5f229e1298227642c0a38e173b332012979692ce Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:48:47 +0100 Subject: [PATCH 44/54] Update library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php Co-authored-by: Eric Lippmann --- .../Icinga/Application/ProvidedHook/CommonPasswordPolicy.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php index d930b70f51..fe0cd8ef01 100644 --- a/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: GPL-3.0-or-later namespace Icinga\Application\ProvidedHook; From a71c4d88728d159fd985530709aa912e999b2086 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:49:30 +0100 Subject: [PATCH 45/54] Update library/Icinga/Authentication/PasswordPolicy.php Co-authored-by: Eric Lippmann --- library/Icinga/Authentication/PasswordPolicy.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php index b16039c454..bc30d40bf8 100644 --- a/library/Icinga/Authentication/PasswordPolicy.php +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -15,12 +15,12 @@ interface PasswordPolicy public function getName(): string; /** - * * Get the description of the password policy - * * - * * Displayed when creating or changing passwords while the policy is active. - * * Should contain the rules of the policy. - * * - * * @return ?string + * Get the description of the password policy + * + * Displayed when creating or changing passwords while the policy is active. + * Should contain the rules of the policy. + * + * @return ?string */ public function getDescription(): ?string; From 2c197ebabe283b38bab4bf37a12d8d589e64b7cd Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:50:09 +0100 Subject: [PATCH 46/54] Update library/Icinga/Authentication/PasswordPolicyHelper.php Co-authored-by: Eric Lippmann --- library/Icinga/Authentication/PasswordPolicyHelper.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 836ac598e6..2116415d8c 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: GPL-3.0-or-later namespace Icinga\Authentication; From abef7e6466581b3b34a42720df731a7db65b2e2f Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:10:23 +0100 Subject: [PATCH 47/54] Apply suggestions from code review Co-authored-by: Eric Lippmann --- doc/05-Authentication.md | 4 +-- .../Application/Hook/PasswordPolicyHook.php | 6 ++--- .../ProvidedHook/AnyPasswordPolicy.php | 4 ++- .../Icinga/Authentication/PasswordPolicy.php | 9 ++++--- .../Authentication/PasswordPolicyHelper.php | 27 ++++++++++--------- .../PasswordPolicyValidator.php | 11 +++++--- .../Application/CommonPasswordPolicyTest.php | 6 ++--- 7 files changed, 37 insertions(+), 30 deletions(-) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index bc7ff288d8..ab69fb2aa4 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -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 diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php index 35e17dc31f..73cce02d64 100644 --- a/library/Icinga/Application/Hook/PasswordPolicyHook.php +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -11,9 +11,7 @@ */ abstract class PasswordPolicyHook implements PasswordPolicy { - /** - * Hook name - */ + /** @var string Hook name *( protected const HOOK_NAME = 'PasswordPolicy'; /** @@ -29,7 +27,7 @@ public static function register(): void /** * Return all registered password policies sorted by name * - * @return array + * @return array */ public static function all(): array { diff --git a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php index 5b69133372..9a11360e56 100644 --- a/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: GPL-3.0-or-later namespace Icinga\Application\ProvidedHook; diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php index bc30d40bf8..6f62e5e863 100644 --- a/library/Icinga/Authentication/PasswordPolicy.php +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: GPL-3.0-or-later namespace Icinga\Authentication; @@ -28,9 +30,10 @@ public function getDescription(): ?string; * 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; } diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 2116415d8c..c64148f8c0 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -15,19 +15,13 @@ class PasswordPolicyHelper { - /* - * Default class for password policy - */ + /** @var class-string 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 @@ public static function applyPasswordPolicy( if ($oldPasswordElementName !== null && $form->getElement($oldPasswordElementName) === null) { throw new LogicException(); } + try { $passwordPolicyClass = Config::app()->get( 'global', @@ -83,10 +78,16 @@ public static function applyPasswordPolicy( ); } } -/** - * 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 $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) { diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 1dbf37ebf6..7513ef42bf 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -1,14 +1,17 @@ +// 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) { diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php index 03cc957a13..dd364fe656 100644 --- a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -75,14 +75,14 @@ public function testValidatePasswordOnlyLowerCaseLetters(): void 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') ); } From d063739e3ae375cf2c9dd419c6b5341c9de723e7 Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:24:52 +0100 Subject: [PATCH 48/54] Update ChangePasswordForm.php --- application/forms/Account/ChangePasswordForm.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 568192ca93..6d9cede581 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -47,8 +47,8 @@ public function createElements(array $formData) 'password', 'new_password', array( - 'label' => $this->translate('New Password'), - 'required' => true + 'label' => $this->translate('New Password'), + 'required' => true ) ); PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password', 'old_password'); From 5ed97f180abc53a4fbefc3b6a42338e021088f8d Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:28:27 +0100 Subject: [PATCH 49/54] Update UserForm.php --- application/forms/Config/User/UserForm.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/forms/Config/User/UserForm.php b/application/forms/Config/User/UserForm.php index 3bccc86c27..aa3e327f44 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -42,8 +42,8 @@ protected function createInsertElements(array $formData) 'password', 'password', array( - 'required' => true, - 'label' => $this->translate('Password') + 'required' => true, + 'label' => $this->translate('Password') ) ); PasswordPolicyHelper::applyPasswordPolicy($this, 'password'); From 42abffc142f50d50d95fc3f1d2555a6c565778ef Mon Sep 17 00:00:00 2001 From: Jolien Trog <124633724+JolienTrog@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:30:59 +0100 Subject: [PATCH 50/54] Update 05-Authentication.md --- doc/05-Authentication.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/05-Authentication.md b/doc/05-Authentication.md index ab69fb2aa4..803dd2e46f 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -205,11 +205,9 @@ Create `library/Mypasswordpolicy/ProvidedHook/PasswordPolicy.php`: namespace Icinga\Module\Mypasswordpolicy\ProvidedHook; use Icinga\Application\Hook\PasswordPolicyHook; -use ipl\I18n\Translation; class PasswordPolicy extends PasswordPolicyHook { - use Translation; public function getName(): string { From b8a236489e857f424b47b99be4ca121fec8daaaf Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 23 Mar 2026 13:57:35 +0100 Subject: [PATCH 51/54] code-review changes --- .../Icinga/Authentication/PasswordPolicy.php | 4 +-- .../Authentication/PasswordPolicyHelper.php | 31 ++++++++++++++++--- .../PasswordPolicyValidator.php | 2 +- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php index 6f62e5e863..a7596d16a6 100644 --- a/library/Icinga/Authentication/PasswordPolicy.php +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -31,9 +31,9 @@ public function getDescription(): ?string; * * @param string $newPassword * @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; } diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index c64148f8c0..455431fe91 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -15,6 +15,8 @@ class PasswordPolicyHelper { + use Translation; + /** @var class-string Default password policy class */ const DEFAULT_PASSWORD_POLICY = AnyPasswordPolicy::class; @@ -27,14 +29,26 @@ class PasswordPolicyHelper /** * Load the configured password policy, fall back to a warning if the policy configuration is invalid. * On success, attaches the policy validator to the given new-password form element. + * + * @param Form $form + * @param string $newPasswordElementName + * @param string|null $oldPasswordElementName Optional name of the old password form element for comparison + * + * @return void + * + * @throws LogicException If the old password element is specified but does not exist in the form + * @throws ConfigurationError If the password policy class is misconfigured */ + public static function applyPasswordPolicy( Form $form, string $newPasswordElementName, ?string $oldPasswordElementName = null ) { if ($oldPasswordElementName !== null && $form->getElement($oldPasswordElementName) === null) { - throw new LogicException(); + throw new LogicException(sprintf( + $this->translate('Form element "%s" was specified but does not exist in the form'), + $oldPasswordElementName)); } try { @@ -70,7 +84,7 @@ public static function applyPasswordPolicy( ], [['HtmlTag#div' => 'HtmlTag'], ['tag' => 'div', 'class' => 'form-notifications error']], ], - 'value' => t( + 'value' => $this->translate( 'There was a problem loading the configured password policy. ' . 'Please contact your administrator.' ), @@ -80,7 +94,7 @@ public static function applyPasswordPolicy( } /** - * Create a {@link PasswordPolicy} instance from the given class name + * Create a {@link PasswordPolicy} instance from the given class name. * * @param class-string $passwordPolicyClass * @@ -95,14 +109,14 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy } if (! class_exists($passwordPolicyClass)) { - throw new ConfigurationError(t('Password policy class %s does not exist'), $passwordPolicyClass); + throw new ConfigurationError($this->translate('Password policy class %s does not exist'), $passwordPolicyClass); } $passwordPolicy = new $passwordPolicyClass(); if (! $passwordPolicy instanceof PasswordPolicy) { throw new ConfigurationError( - t('Password policy %s is not an instance of %s'), + $this->translate('Password policy %s is not an instance of %s'), $passwordPolicyClass, PasswordPolicy::class ); @@ -111,6 +125,13 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy return $passwordPolicy; } + /** + * Retrieve the description from the given password policy and add it to the form for display to the user. + * + * @param Form $form + * @param PasswordPolicy $passwordPolicy + * @return void + */ public static function addPasswordPolicyDescription(Form $form, PasswordPolicy $passwordPolicy): void { $description = $passwordPolicy->getDescription(); diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php index 7513ef42bf..4e7e10419d 100644 --- a/library/Icinga/Authentication/PasswordPolicyValidator.php +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -9,7 +9,7 @@ /** * 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 validation. * Delegate all validation logic to the policy instance and expose any returned violation messages. From 69d2d04f41080606f8f9c2ad0b702cfde167beeb Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 23 Mar 2026 14:22:18 +0100 Subject: [PATCH 52/54] code sniffer changes --- .../Icinga/Authentication/PasswordPolicyHelper.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 455431fe91..92d6c83423 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -29,11 +29,11 @@ class PasswordPolicyHelper /** * Load the configured password policy, fall back to a warning if the policy configuration is invalid. * On success, attaches the policy validator to the given new-password form element. - * + * * @param Form $form * @param string $newPasswordElementName * @param string|null $oldPasswordElementName Optional name of the old password form element for comparison - * + * * @return void * * @throws LogicException If the old password element is specified but does not exist in the form @@ -48,7 +48,8 @@ public static function applyPasswordPolicy( if ($oldPasswordElementName !== null && $form->getElement($oldPasswordElementName) === null) { throw new LogicException(sprintf( $this->translate('Form element "%s" was specified but does not exist in the form'), - $oldPasswordElementName)); + $oldPasswordElementName + )); } try { @@ -109,7 +110,10 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy } if (! class_exists($passwordPolicyClass)) { - throw new ConfigurationError($this->translate('Password policy class %s does not exist'), $passwordPolicyClass); + throw new ConfigurationError( + $this->translate('Password policy class %s does not exist'), + $passwordPolicyClass + ); } $passwordPolicy = new $passwordPolicyClass(); @@ -127,7 +131,7 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy /** * Retrieve the description from the given password policy and add it to the form for display to the user. - * + * * @param Form $form * @param PasswordPolicy $passwordPolicy * @return void From 1ec7a3817169559917b8ac9510d1d5fc90a82a46 Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Mon, 23 Mar 2026 14:24:20 +0100 Subject: [PATCH 53/54] code sniffer changes --- library/Icinga/Authentication/PasswordPolicyHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php index 92d6c83423..0dba02de49 100644 --- a/library/Icinga/Authentication/PasswordPolicyHelper.php +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -111,7 +111,7 @@ public static function createPolicy(string $passwordPolicyClass): PasswordPolicy if (! class_exists($passwordPolicyClass)) { throw new ConfigurationError( - $this->translate('Password policy class %s does not exist'), + $this->translate('Password policy class %s does not exist'), $passwordPolicyClass ); } From a3bc5d98794fc2ce664a7c36d8b02a6c0eff5f9c Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Tue, 24 Mar 2026 11:41:39 +0100 Subject: [PATCH 54/54] Implement Password Policy in setup Guide --- modules/setup/application/forms/AdminAccountPage.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/setup/application/forms/AdminAccountPage.php b/modules/setup/application/forms/AdminAccountPage.php index b33749efc9..da75c64b2f 100644 --- a/modules/setup/application/forms/AdminAccountPage.php +++ b/modules/setup/application/forms/AdminAccountPage.php @@ -5,6 +5,7 @@ use Exception; use Icinga\Application\Config; +use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Authentication\User\ExternalBackend; use Icinga\Authentication\User\UserBackend; use Icinga\Authentication\User\DbUserBackend; @@ -228,6 +229,8 @@ public function createElements(array $formData) 'autocomplete' => 'new-password' ) ); + PasswordPolicyHelper::applyPasswordPolicy($this, 'new_user_password'); + $this->addElement( 'password', 'new_user_2ndpass',