From 0a8716830f34bae0442e01508f3158ac84cf550a Mon Sep 17 00:00:00 2001 From: Jolien Trog Date: Thu, 28 Aug 2025 12:58:40 +0200 Subject: [PATCH] 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 42c5cbc72..549cc8bb9 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 @@ class ChangePasswordForm extends Form */ 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 @@ class ChangePasswordForm extends Form 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 be6ce1490..442876fd6 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 @@ class UserForm extends RepositoryForm 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 185422811..f54b7e576 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 3b1ed3dc8..5e38a1e15 100644 --- a/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php @@ -25,7 +25,7 @@ class DefaultPasswordPolicy implements PasswordPolicyHook return 'Default'; } - public function displayPasswordPolicy(): string + public function getDescription(): string { $message = $this->translate( @@ -35,49 +35,44 @@ class DefaultPasswordPolicy implements PasswordPolicyHook 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 19281f023..c55407c88 100644 --- a/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php +++ b/library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php @@ -15,13 +15,13 @@ class NonePasswordPolicy implements PasswordPolicyHook 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 4a7f837b9..8b9ecbd7c 100644 --- a/library/Icinga/Authentication/PasswordValidator.php +++ b/library/Icinga/Authentication/PasswordValidator.php @@ -35,11 +35,13 @@ class PasswordValidator extends Zend_Validate_Abstract */ 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; } }