-
Notifications
You must be signed in to change notification settings - Fork 281
Implement password policy with hook #5419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 30 commits
828440e
3e7a2cf
ed78365
bb96ba7
174c59c
93c6479
4ab7f2f
9175c4d
1215b94
0a44791
8f49073
43d6483
02c067e
740837f
64d7036
b3a1a31
d2d91d2
2a10671
d7acd7c
33bb7ba
9d8cbcd
2682e0f
91f20c8
ba6972d
6b2ffd0
71090ae
d7e36e3
76bacb6
29b74f1
47acabf
409137e
81e6859
7193bab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -45,11 +46,14 @@ public function createElements(array $formData) | |
| $this->addElement( | ||
| 'password', | ||
| 'new_password', | ||
| array( | ||
| 'label' => $this->translate('New Password'), | ||
| 'required' => true | ||
| ) | ||
| [ | ||
| 'label' => $this->translate('New Password'), | ||
| 'required' => true | ||
| ] | ||
|
||
| ); | ||
|
|
||
|
||
| PasswordPolicyHelper::applyPasswordPolicy($this, 'new_password'); | ||
|
|
||
| $this->addElement( | ||
| 'password', | ||
| 'new_password_confirmation', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| <?php | ||
| /* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ | ||
|
|
||
lippserd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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), | ||
| [ | ||
Al2Klimov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 'description' => $this->translate('Enforce password requirements for new passwords'), | ||
| 'label' => $this->translate('Password Policy'), | ||
| 'value' => PasswordPolicyHelper::DEFAULT_PASSWORD_POLICY, | ||
| 'multiOptions' => PasswordPolicyHook::all() | ||
| ] | ||
| ); | ||
|
|
||
lippserd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return $this; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| { | ||
|
|
@@ -37,12 +41,14 @@ protected function createInsertElements(array $formData) | |
| $this->addElement( | ||
| 'password', | ||
| 'password', | ||
| array( | ||
| 'required' => true, | ||
| 'label' => $this->translate('Password') | ||
| ) | ||
| [ | ||
| 'required' => true, | ||
| 'label' => $this->translate('Password') | ||
| ] | ||
|
||
| ); | ||
|
|
||
| PasswordPolicyHelper::applyPasswordPolicy($this, 'password'); | ||
|
|
||
| $this->setTitle($this->translate('Add a new user')); | ||
| $this->setSubmitLabel($this->translate('Add')); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -158,6 +158,97 @@ 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 <a id="authentication-password-policy"></a> | ||||||||||
lippserd marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
|
||||||||||
| 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 <a id="authentication-custom-password-policy"></a> | ||||||||||
|
|
||||||||||
| 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 | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||||||||||
| Version: 1.0.0 | ||||||||||
| Description: Custom password policy implementation | ||||||||||
| Author: Your Name | ||||||||||
|
||||||||||
| ``` | ||||||||||
|
|
||||||||||
| **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 | ||||||||||
| namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; | ||||||||||
|
||||||||||
| namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; | |
| <?php | |
| namespace Icinga\Module\MyPasswordPolicy\ProvidedHook; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example does not use Translation, but it would be nice if this were included as well.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** @var $this \Icinga\Application\Modules\Module */ | |
| <?php | |
| use Icinga\Module\Mypasswordpolicy\ProvidedHook\MyPasswordPolicy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -6,16 +6,20 @@ | |||
| use DirectoryIterator; | ||||
| use ErrorException; | ||||
| use Exception; | ||||
| use Icinga\Application\ProvidedHook\DbMigration; | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't reorder the whole imports. Two new lines for |
||||
| use ipl\I18n\GettextTranslator; | ||||
| use ipl\I18n\StaticTranslator; | ||||
| use LogicException; | ||||
| 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; | ||||
| 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 | ||||
|
|
@@ -740,6 +744,9 @@ public function hasLocales() | |||
| protected function registerApplicationHooks(): self | ||||
| { | ||||
| Hook::register('DbMigration', DbMigration::class, DbMigration::class); | ||||
| CommonPasswordPolicy::register(); | ||||
| AnyPasswordPolicy::register(); | ||||
|
|
||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
|
||||
| return $this; | ||||
| } | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||||
| /* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| namespace Icinga\Application\Hook; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| use Icinga\Application\Hook; | ||||||||||||||||||||||||||||||||
| use Icinga\Authentication\PasswordPolicy; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
Al2Klimov marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
| * This class connects with the PasswordPolicy interface to use it as hook | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| * This class connects with the PasswordPolicy interface to use it as hook | |
| * Base class for hookable password policies |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Registers Hook as Password Policy | |
| * Register password policy |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation does not look correct.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PasswordPolicy could be a protected const, e.g., HOOK_NAME.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Wait until Introduce
\Icinga\Application\Hook\Essentials#5474 is merged - Use that trait
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Returns all registered password policies sorted by | |
| * Return all registered password policies sorted by name |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation does not look correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $passwordPolicies = []; | |
| foreach (Hook::all('PasswordPolicy') as $class => $policy) { | |
| $passwordPolicies[$class] = $policy->getName(); | |
| } | |
| asort($passwordPolicies); | |
| return $passwordPolicies; | |
| $passwordPolicies = []; | |
| foreach (Hook::all('PasswordPolicy') as $class => $policy) { | |
| $passwordPolicies[$class] = $policy->getName(); | |
| } | |
| asort($passwordPolicies); | |
| return $passwordPolicies; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||
| /* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| namespace Icinga\Application\ProvidedHook; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| use Icinga\Application\Hook\PasswordPolicyHook; | ||||||||||||||||||||||||||||||
| use ipl\I18n\Translation; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Policy to allow any password | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| return $this->translate('None'); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| /** | |
| * 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('None'); | |
| } | |
| 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'); | |
| } |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||
| <?php | ||||
| /* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ | ||||
|
|
||||
| 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; | ||||
|
|
||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
|
||||
| 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 $password): array | ||||
| { | ||||
| $violations = []; | ||||
|
|
||||
| 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'); | ||||
| } | ||||
|
|
||||
| if (! preg_match('/[^a-zA-Z0-9]/', $password)) { | ||||
| $violations[] = $this->translate('Password must contain at least one special character'); | ||||
| } | ||||
|
|
||||
| if (! preg_match('/[A-Z]/', $password)) { | ||||
sukhwinder33445 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||
| $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'); | ||||
| } | ||||
|
|
||||
| return $violations; | ||||
| } | ||||
| } | ||||
Uh oh!
There was an error while loading. Please reload this page.