diff --git a/application/forms/Account/ChangePasswordForm.php b/application/forms/Account/ChangePasswordForm.php index 5bca11cc86..6d9cede581 100644 --- a/application/forms/Account/ChangePasswordForm.php +++ b/application/forms/Account/ChangePasswordForm.php @@ -3,6 +3,7 @@ namespace Icinga\Forms\Account; +use Icinga\Authentication\PasswordPolicyHelper; use Icinga\Authentication\User\DbUserBackend; use Icinga\Data\Filter\Filter; use Icinga\User; @@ -46,10 +47,12 @@ 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'); + $this->addElement( 'password', 'new_password_confirmation', diff --git a/application/forms/Config/General/PasswordPolicyConfigForm.php b/application/forms/Config/General/PasswordPolicyConfigForm.php new file mode 100644 index 0000000000..b8b719df2e --- /dev/null +++ b/application/forms/Config/General/PasswordPolicyConfigForm.php @@ -0,0 +1,39 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Forms\Config\General; + +use Icinga\Application\Hook\PasswordPolicyHook; +use Icinga\Authentication\PasswordPolicyHelper; +use Icinga\Web\Form; + +/** + * Configuration form for password policy selection + * + * This form is not used directly but as subform for the {@link GeneralConfigForm}. + */ +class PasswordPolicyConfigForm extends Form +{ + public function init(): void + { + $this->setName('form_config_general_password_policy'); + } + + public function createElements(array $formData): static + { + $this->addElement( + 'select', + 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' => PasswordPolicyHook::all() + ] + ); + + 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..aa3e327f44 100644 --- a/application/forms/Config/User/UserForm.php +++ b/application/forms/Config/User/UserForm.php @@ -4,9 +4,13 @@ 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; +use Icinga\Web\Form\Element\Note; use Icinga\Web\Notification; +use Throwable; class UserForm extends RepositoryForm { @@ -42,6 +46,7 @@ protected function createInsertElements(array $formData) '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 22772b5a1e..803dd2e46f 100644 --- a/doc/05-Authentication.md +++ b/doc/05-Authentication.md @@ -158,6 +158,108 @@ 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 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: + +* 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 +Module: mypasswordpolicy +Name: My Password Policy +Version: 1.0.0 +Description: Custom password policy implementation +``` + +**Implement the Hook** + +Icinga Web provides the `PasswordPolicyHook` with predefined methods +that simplify the extension of custom password policies. + +Create `library/Mypasswordpolicy/ProvidedHook/PasswordPolicy.php`: + +```php + +``` + +**Register the Hook** + +Create `run.php`: + +```php + +``` + + +Enable the module: +```shell +icingacli module enable mypasswordpolicy +``` + +The custom policy will now appear in **Configuration > Application > General** under Password Policy. ## Groups diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index 32abeaabb0..b804f130e2 100644 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -6,16 +6,18 @@ use DirectoryIterator; use ErrorException; use Exception; -use Icinga\Application\ProvidedHook\DbMigration; -use ipl\I18n\GettextTranslator; -use ipl\I18n\StaticTranslator; -use LogicException; use Icinga\Application\Modules\Manager as ModuleManager; +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; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; +use Icinga\Application\ProvidedHook\CommonPasswordPolicy; /** * This class bootstraps a thin Icinga application layer @@ -740,6 +742,8 @@ public function hasLocales() protected function registerApplicationHooks(): self { Hook::register('DbMigration', DbMigration::class, DbMigration::class); + CommonPasswordPolicy::register(); + AnyPasswordPolicy::register(); return $this; } diff --git a/library/Icinga/Application/Hook/PasswordPolicyHook.php b/library/Icinga/Application/Hook/PasswordPolicyHook.php new file mode 100644 index 0000000000..73cce02d64 --- /dev/null +++ b/library/Icinga/Application/Hook/PasswordPolicyHook.php @@ -0,0 +1,42 @@ + + */ + 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 new file mode 100644 index 0000000000..9a11360e56 --- /dev/null +++ b/library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php @@ -0,0 +1,33 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Application\ProvidedHook; + +use Icinga\Application\Hook\PasswordPolicyHook; +use ipl\I18n\Translation; + +/** + * Policy to allow any password + */ +class AnyPasswordPolicy extends PasswordPolicyHook +{ + use Translation; + + 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'); + } + + public function getDescription(): ?string + { + return null; + } + + 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 new file mode 100644 index 0000000000..fe0cd8ef01 --- /dev/null +++ b/library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php @@ -0,0 +1,64 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Application\ProvidedHook; + +use Icinga\Application\Hook\PasswordPolicyHook; +use ipl\I18n\Translation; + +/** + * Common implementation of a password policy + * + * Enforces: + * - Minimum length of 12 characters + * - At least one number + * - At least one special character + * - At least one uppercase letter + * - At least one lowercase letter + */ +class CommonPasswordPolicy extends PasswordPolicyHook +{ + use Translation; + + public function getName(): string + { + return $this->translate('Common'); + } + + public function getDescription(): ?string + { + return $this->translate( + 'Password requirements: minimum 12 characters, ' . + 'at least 1 number, 1 special character, uppercase and lowercase letters.' + ); + } + + public function validate(string $newPassword, ?string $oldPassword = null): array + { + $violations = []; + + if (mb_strlen($newPassword) < 12) { + $violations[] = $this->translate('Password must be at least 12 characters long'); + } + + if (! preg_match('/[0-9]/', $newPassword)) { + $violations[] = $this->translate('Password must contain at least one number'); + } + + if (! preg_match('/[^a-zA-Z0-9]/', $newPassword)) { + $violations[] = $this->translate('Password must contain at least one special character'); + } + + if (! preg_match('/[A-Z]/', $newPassword)) { + $violations[] = $this->translate('Password must contain at least one uppercase letter'); + } + + if (! preg_match('/[a-z]/', $newPassword)) { + $violations[] = $this->translate('Password must contain at least one lowercase letter'); + } + + return $violations; + } +} diff --git a/library/Icinga/Authentication/PasswordPolicy.php b/library/Icinga/Authentication/PasswordPolicy.php new file mode 100644 index 0000000000..a7596d16a6 --- /dev/null +++ b/library/Icinga/Authentication/PasswordPolicy.php @@ -0,0 +1,39 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Authentication; + +interface PasswordPolicy +{ + /** + * Get the name of the password policy + * + * Displayed when configuring a password policy. + * + * @return string + */ + 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 + */ + public function getDescription(): ?string; + + /** + * Validate a given password against the defined policy + * + * @param string $newPassword + * @param ?string $oldPassword + * + * @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 = null): array; +} diff --git a/library/Icinga/Authentication/PasswordPolicyHelper.php b/library/Icinga/Authentication/PasswordPolicyHelper.php new file mode 100644 index 0000000000..0dba02de49 --- /dev/null +++ b/library/Icinga/Authentication/PasswordPolicyHelper.php @@ -0,0 +1,147 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Authentication; + +use Icinga\Application\Config; +use Icinga\Application\Logger; +use Icinga\Application\ProvidedHook\AnyPasswordPolicy; +use Icinga\Exception\ConfigurationError; +use Icinga\Web\Form; +use ipl\I18n\Translation; +use LogicException; + +class PasswordPolicyHelper +{ + use Translation; + + /** @var class-string Default password policy class */ + const DEFAULT_PASSWORD_POLICY = AnyPasswordPolicy::class; + + /** @var string INI configuration section for password policy */ + const CONFIG_SECTION = 'global'; + + /** @var string INI configuration key for password policy */ + const CONFIG_KEY = 'password_policy'; + + /** + * 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(sprintf( + $this->translate('Form element "%s" was specified but does not exist in the form'), + $oldPasswordElementName + )); + } + + try { + $passwordPolicyClass = Config::app()->get( + 'global', + 'password_policy', + static::DEFAULT_PASSWORD_POLICY + ); + + $passwordPolicy = static::createPolicy($passwordPolicyClass); + // 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', + 'bogus', + [ + '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' => $this->translate( + 'There was a problem loading the configured password policy. ' + . 'Please contact your administrator.' + ), + ] + ); + } + } + + /** + * 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) { + return new $passwordPolicyClass; + } + + if (! class_exists($passwordPolicyClass)) { + throw new ConfigurationError( + $this->translate('Password policy class %s does not exist'), + $passwordPolicyClass + ); + } + + $passwordPolicy = new $passwordPolicyClass(); + + if (! $passwordPolicy instanceof PasswordPolicy) { + throw new ConfigurationError( + $this->translate('Password policy %s is not an instance of %s'), + $passwordPolicyClass, + PasswordPolicy::class + ); + } + + 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(); + + if ($description !== null) { + $form->addDescription($description); + } + } +} diff --git a/library/Icinga/Authentication/PasswordPolicyValidator.php b/library/Icinga/Authentication/PasswordPolicyValidator.php new file mode 100644 index 0000000000..4e7e10419d --- /dev/null +++ b/library/Icinga/Authentication/PasswordPolicyValidator.php @@ -0,0 +1,58 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Authentication; + +use Zend_Validate_Abstract; + +/** + * 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. + */ +class PasswordPolicyValidator extends Zend_Validate_Abstract +{ + /** @var PasswordPolicy Policy to use for validation */ + protected PasswordPolicy $passwordPolicy; + + /** @var ?string Name of the old password form element */ + protected ?string $oldPasswordElementName; + + /** + * Create a new PasswordPolicyValidator + * + * @param PasswordPolicy $passwordPolicy + * @param ?string $oldPasswordElementName + */ + public function __construct(PasswordPolicy $passwordPolicy, ?string $oldPasswordElementName = null) + { + $this->passwordPolicy = $passwordPolicy; + $this->oldPasswordElementName = $oldPasswordElementName; + } + + public function isValid($value, mixed $context = null): bool + { + $oldPassword = null; + + if (is_array($context)) { + $oldPasswordValue = $context[$this->oldPasswordElementName] ?? null; + if ($oldPasswordValue !== null && $oldPasswordValue !== '') { + $oldPassword = $oldPasswordValue; + } + } + + $message = $this->passwordPolicy->validate($value, $oldPassword); + + if (! empty($message)) { + $this->_messages = $message; + + return false; + } + + return true; + } +} 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', diff --git a/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php new file mode 100644 index 0000000000..2e03d546d4 --- /dev/null +++ b/test/php/library/Icinga/Application/AnyPasswordPolicyTest.php @@ -0,0 +1,22 @@ +instance = new AnyPasswordPolicy(); + } + + public function testValidatePasswordValid(): void + { + $this->assertEmpty($this->instance->validate('icingaadmin', 'null')); + } +} diff --git a/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php new file mode 100644 index 0000000000..dd364fe656 --- /dev/null +++ b/test/php/library/Icinga/Application/CommonPasswordPolicyTest.php @@ -0,0 +1,89 @@ +instance = new CommonPasswordPolicy(); + } + + public function testValidatePasswordTooShort(): void + { + $this->assertSame( + ['Password must be at least 12 characters long'], + $this->instance->validate('Icinga1#') + ); + } + + public function testValidatePasswordNoNumber(): void + { + $this->assertSame( + ['Password must contain at least one number'], + $this->instance->validate('Icingaadmin#') + ); + } + + public function testValidatePasswordNoSpecialCharacter(): void + { + $this->assertSame( + ['Password must contain at least one special character'], + $this->instance->validate('Icingaadmin1') + ); + } + + public function testValidatePasswordNoUpperCaseLetters(): void + { + $this->assertSame( + ['Password must contain at least one uppercase letter'], + $this->instance->validate('icingaadmin1#') + ); + } + + public function testValidatePasswordNoLowerCaseLetters(): void + { + $this->assertSame( + ['Password must contain at least one lowercase letter'], + $this->instance->validate('ICINGAADMIN1#') + ); + } + + public function testValidatePasswordValid(): void + { + $this->assertEmpty($this->instance->validate('Icingaadmin1#')); + } + + public function testValidatePasswordOnlyLowerCaseLetters(): void + { + $expected = [ + 'Password must contain at least one number', + 'Password must contain at least one special character', + 'Password must contain at least one uppercase letter' + ]; + $this->assertSame( + $expected, + $this->instance->validate('icingawebadmin') + ); + } + + public function testValidatePasswordWithLengthAndUpperCaseLetters(): void + { + $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' + ]; + $this->assertSame( + $expected, + $this->instance->validate('ICINGAADMIN') + ); + } + }