-
Notifications
You must be signed in to change notification settings - Fork 283
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 all 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
4dabe41
c29a90a
74e549d
ca5f843
812646e
315570f
b022b18
408f1e5
1380632
37e7ed6
5f229e1
a71c4d8
2c197eb
abef7e6
d063739
5ed97f1
42abffc
b8a2364
69d2d04
1ec7a38
a3bc5d9
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 |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| <?php | ||
|
|
||
| // SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com> | ||
| // SPDX-License-Identifier: GPL-3.0-or-later | ||
|
|
||
|
lippserd marked this conversation as 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}. | ||
|
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.
|
||
| */ | ||
| class PasswordPolicyConfigForm extends Form | ||
|
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. There already was some discussion with @lippserd to move this form into a new Security section in IW2. |
||
| { | ||
| public function init(): void | ||
| { | ||
| $this->setName('form_config_general_password_policy'); | ||
| } | ||
|
|
||
| public function createElements(array $formData): static | ||
| { | ||
| $this->addElement( | ||
|
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. I'd like to see the description of the password policy somewhere in this form. |
||
| 'select', | ||
| sprintf('%s_%s', PasswordPolicyHelper::CONFIG_SECTION, PasswordPolicyHelper::CONFIG_KEY), | ||
| [ | ||
|
Al2Klimov marked this conversation as resolved.
|
||
| 'description' => $this->translate('Enforce password requirements for new passwords'), | ||
| 'label' => $this->translate('Password Policy'), | ||
| 'value' => PasswordPolicyHelper::DEFAULT_PASSWORD_POLICY, | ||
|
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. Having an invalid value set in the config (class not found or not an instance of PasswordPolicy) causes IW2 to display |
||
| 'multiOptions' => PasswordPolicyHook::all() | ||
| ] | ||
| ); | ||
|
|
||
|
lippserd marked this conversation as resolved.
|
||
| return $this; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,42 @@ | ||||||
| <?php | ||||||
| /* Icinga Web 2 | (c) 2025 Icinga GmbH | GPLv2+ */ | ||||||
|
JolienTrog marked this conversation as resolved.
|
||||||
|
|
||||||
| namespace Icinga\Application\Hook; | ||||||
|
|
||||||
| use Icinga\Application\Hook; | ||||||
| use Icinga\Authentication\PasswordPolicy; | ||||||
|
|
||||||
| /** | ||||||
| * Base class for hookable password policies | ||||||
| */ | ||||||
| abstract class PasswordPolicyHook implements PasswordPolicy | ||||||
| { | ||||||
| /** @var string Hook name *( | ||||||
|
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
This will not parse correctly! |
||||||
| protected const HOOK_NAME = 'PasswordPolicy'; | ||||||
|
|
||||||
| /** | ||||||
| * Register password policy | ||||||
| * | ||||||
| * @return void | ||||||
| */ | ||||||
| public static function register(): void | ||||||
| { | ||||||
| Hook::register(self::HOOK_NAME, static::class, static::class); | ||||||
| } | ||||||
|
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.
|
||||||
|
|
||||||
| /** | ||||||
| * Return all registered password policies sorted by name | ||||||
| * | ||||||
| * @return array<string, string> | ||||||
| */ | ||||||
| public static function all(): array | ||||||
| { | ||||||
| $passwordPolicies = []; | ||||||
| foreach (Hook::all('PasswordPolicy') as $class => $policy) { | ||||||
|
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
We have the constant, why not use it? |
||||||
| $passwordPolicies[$class] = $policy->getName(); | ||||||
| } | ||||||
| asort($passwordPolicies); | ||||||
|
|
||||||
| return $passwordPolicies; | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <?php | ||
|
|
||
| // SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com> | ||
| // 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 []; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| <?php | ||
|
|
||
| // SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com> | ||
| // 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; | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.