Implement password policy with hook by JolienTrog · Pull Request #5419 · Icinga/icingaweb2

JolienTrog · 2025-08-26T09:22:14Z

Ref #4401

application/forms/Config/General/PasswordPolicyConfigForm.php

library/Icinga/Authentication/PasswordValidator.php

application/forms/Account/ChangePasswordForm.php

application/forms/Config/GeneralConfigForm.php

application/forms/Account/ChangePasswordForm.php

application/forms/Config/General/PasswordPolicyConfigForm.php

application/forms/Config/User/UserForm.php

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php

library/Icinga/Authentication/PasswordValidator.php

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php

library/Icinga/Authentication/PasswordValidator.php

application/forms/Account/ChangePasswordForm.php

library/Icinga/Application/Hook/PasswordPolicyHook.php

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php

application/forms/Account/ChangePasswordForm.php

application/forms/Config/User/UserForm.php

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php

library/Icinga/Authentication/PasswordValidator.php

application/forms/Account/ChangePasswordForm.php

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php

library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php

application/forms/Account/ChangePasswordForm.php

application/forms/Config/User/UserForm.php

Al2Klimov

During setup, the wizard prompts me for a username and password "to configure your first administrative account". This should enforce the chosen password policy.

test/php/library/Icinga/Application/CommonPasswordPolicyTest.php

library/Icinga/Application/Hook/PasswordPolicyHook.php

library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php

application/forms/Config/General/PasswordPolicyConfigForm.php

test/php/library/Icinga/Application/NoPasswordPolicyTest.php

test/php/library/Icinga/Application/CommonPasswordPolicyTest.php

application/forms/Config/General/PasswordPolicyConfigForm.php

doc/05-Authentication.md

lippserd · 2026-03-20T09:15:39Z

doc/05-Authentication.md

+
+class PasswordPolicy extends PasswordPolicyHook
+{
+    use Translation;


You use the Translation trait only but never call $this->translate() on any of the strings.

doc/05-Authentication.md

lippserd · 2026-03-20T09:51:47Z

library/Icinga/Authentication/PasswordPolicyHelper.php

+        return $passwordPolicy;
+    }
+
+    public static function addPasswordPolicyDescription(Form $form, PasswordPolicy $passwordPolicy): void


PHPDoc missing.

library/Icinga/Authentication/PasswordPolicyValidator.php

test/php/library/Icinga/Application/AnyPasswordPolicyTest.php

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

TheSyscall · 2026-03-31T12:51:38Z

library/Icinga/Application/Hook/PasswordPolicyHook.php

+ */
+abstract class PasswordPolicyHook implements PasswordPolicy
+{
+    /** @var string Hook name *(


Suggested change

/** @var string Hook name *(

/** @var string Hook name */

This will not parse correctly!

TheSyscall · 2026-03-31T12:52:08Z

library/Icinga/Application/Hook/PasswordPolicyHook.php

+    public static function all(): array
+    {
+        $passwordPolicies = [];
+        foreach (Hook::all('PasswordPolicy') as $class => $policy) {


Suggested change

foreach (Hook::all('PasswordPolicy') as $class => $policy) {

foreach (Hook::all(self::HOOK_NAME) as $class => $policy) {

We have the constant, why not use it?

TheSyscall · 2026-03-31T12:53:05Z

library/Icinga/Authentication/PasswordPolicyHelper.php

+    ) {
+        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'),


Using $this when not in object context. This will throw an exception if executed.

TheSyscall · 2026-03-31T12:54:28Z

library/Icinga/Authentication/PasswordPolicyHelper.php

+                        ],
+                        [['HtmlTag#div' => 'HtmlTag'], ['tag' => 'div', 'class' => 'form-notifications error']],
+                    ],
+                    'value' => $this->translate(


Using $this when not in object context. This will throw an exception if executed.

TheSyscall · 2026-03-31T12:54:45Z

library/Icinga/Authentication/PasswordPolicyHelper.php

+
+        if (! class_exists($passwordPolicyClass)) {
+            throw new ConfigurationError(
+                $this->translate('Password policy class %s does not exist'),


Using $this when not in object context. This will throw an exception if executed.

TheSyscall · 2026-03-31T13:11:35Z

library/Icinga/Authentication/PasswordPolicyHelper.php

+    const DEFAULT_PASSWORD_POLICY = AnyPasswordPolicy::class;
+
+    /** @var string INI configuration section for password policy */
+    const CONFIG_SECTION = 'global';


Consider moving password policy to the security section.

Yeah, you also made a separate permission for the CSP stuff, didn't you?

I was mostly talking about the [security] section of the config.ini file here.

But yes, I added 'config/security' in #5477 for all security related configuration.
The only thing that is currently in this section is CSP, but it seems password policy would fit well there.

TheSyscall · 2026-03-31T13:15:24Z

application/forms/Config/General/PasswordPolicyConfigForm.php

+ *
+ * This form is not used directly but as subform for the {@link GeneralConfigForm}.
+ */
+class PasswordPolicyConfigForm extends Form


There already was some discussion with @lippserd to move this form into a new Security section in IW2.
We should consider converting this form into an ipl-web based form instead during this process.

TheSyscall · 2026-03-31T13:18:49Z

application/forms/Config/User/UserForm.php

                'description'   => $this->translate('Leave empty for not updating the user\'s password'),
                'label'         => $this->translate('Password'),
            )
        );


Why is there no check for the password policy here? We require admins to follow the policy when creating a user, why not during updates?

TheSyscall · 2026-03-31T13:25:40Z

application/forms/Config/General/PasswordPolicyConfigForm.php

+            [
+                'description'  => $this->translate('Enforce password requirements for new passwords'),
+                'label'        => $this->translate('Password Policy'),
+                'value'        => PasswordPolicyHelper::DEFAULT_PASSWORD_POLICY,


Having an invalid value set in the config (class not found or not an instance of PasswordPolicy) causes IW2 to display Common.
I would expect some sort of error message here.

TheSyscall · 2026-03-31T13:27:45Z

application/forms/Config/General/PasswordPolicyConfigForm.php

+
+    public function createElements(array $formData): static
+    {
+        $this->addElement(


I'd like to see the description of the password policy somewhere in this form.
We display it in every place where a password field exists, why not where we actually select the policy?

JolienTrog requested a review from Al2Klimov August 26, 2025 09:22

cla-bot bot added the cla/signed label Aug 26, 2025

This comment was marked as resolved.

Sign in to view

JolienTrog requested a review from Al2Klimov August 26, 2025 15:21

Al2Klimov reviewed Aug 26, 2025

View reviewed changes

JolienTrog requested a review from Al2Klimov August 27, 2025 07:53

Al2Klimov reviewed Aug 27, 2025

View reviewed changes

library/Icinga/Authentication/PasswordValidator.php Outdated Show resolved Hide resolved

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php Outdated Show resolved Hide resolved

library/Icinga/Authentication/PasswordValidator.php Outdated Show resolved Hide resolved

Al2Klimov reviewed Aug 27, 2025

View reviewed changes

library/Icinga/Authentication/PasswordValidator.php Outdated Show resolved Hide resolved

application/forms/Account/ChangePasswordForm.php Outdated Show resolved Hide resolved

JolienTrog requested a review from Al2Klimov August 28, 2025 08:35

Al2Klimov reviewed Aug 28, 2025

View reviewed changes

JolienTrog requested a review from Al2Klimov August 28, 2025 11:05

Al2Klimov reviewed Aug 28, 2025

View reviewed changes

JolienTrog requested a review from Al2Klimov August 28, 2025 13:15

Al2Klimov reviewed Aug 28, 2025

View reviewed changes

library/Icinga/Application/ProvidedHook/DefaultPasswordPolicy.php Outdated Show resolved Hide resolved

library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php Outdated Show resolved Hide resolved

library/Icinga/Application/ProvidedHook/NonePasswordPolicy.php Show resolved Hide resolved

JolienTrog requested a review from Al2Klimov August 28, 2025 14:05

Al2Klimov reviewed Aug 28, 2025

View reviewed changes

application/forms/Account/ChangePasswordForm.php Outdated Show resolved Hide resolved

application/forms/Config/User/UserForm.php Outdated Show resolved Hide resolved

JolienTrog requested a review from Al2Klimov August 28, 2025 15:26

Al2Klimov reviewed Aug 28, 2025

View reviewed changes

JolienTrog force-pushed the password-policy branch 2 times, most recently from 97e07ad to 39bbc53 Compare September 2, 2025 10:24

JolienTrog requested a review from sukhwinder33445 September 2, 2025 12:43

sukhwinder33445 requested changes Sep 2, 2025

View reviewed changes

JolienTrog requested a review from sukhwinder33445 September 2, 2025 14:01

sukhwinder33445 reviewed Sep 2, 2025

View reviewed changes

application/forms/Config/General/PasswordPolicyConfigForm.php Outdated Show resolved Hide resolved

test/php/library/Icinga/Application/NoPasswordPolicyTest.php Show resolved Hide resolved

sukhwinder33445 reviewed Sep 3, 2025

View reviewed changes

test/php/library/Icinga/Application/CommonPasswordPolicyTest.php Outdated Show resolved Hide resolved

sukhwinder33445 reviewed Sep 3, 2025

View reviewed changes

test/php/library/Icinga/Application/CommonPasswordPolicyTest.php Outdated Show resolved Hide resolved

Al2Klimov mentioned this pull request Mar 4, 2026

Introduce \Icinga\Application\Hook\Essentials #5474

Open

Al2Klimov mentioned this pull request Mar 19, 2026

Allow custom csp header #5477

Open

lippserd requested changes Mar 20, 2026

View reviewed changes

JolienTrog and others added 21 commits March 20, 2026 11:42

Update application/forms/Config/General/PasswordPolicyConfigForm.php

4dabe41

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update doc/05-Authentication.md

c29a90a

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update test/php/library/Icinga/Application/AnyPasswordPolicyTest.php

74e549d

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Authentication/PasswordPolicyValidator.php

ca5f843

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Authentication/PasswordPolicyValidator.php

812646e

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Authentication/PasswordPolicyValidator.php

315570f

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update doc/05-Authentication.md

b022b18

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update doc/05-Authentication.md

408f1e5

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update doc/05-Authentication.md

1380632

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Application/ProvidedHook/AnyPasswordPolicy.php

37e7ed6

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Application/ProvidedHook/CommonPasswordPolicy.php

5f229e1

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Authentication/PasswordPolicy.php

a71c4d8

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update library/Icinga/Authentication/PasswordPolicyHelper.php

2c197eb

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Apply suggestions from code review

abef7e6

Co-authored-by: Eric Lippmann <eric.lippmann@icinga.com>

Update ChangePasswordForm.php

d063739

Update UserForm.php

5ed97f1

Update 05-Authentication.md

42abffc

code-review changes

b8a2364

code sniffer changes

69d2d04

code sniffer changes

1ec7a38

Implement Password Policy in setup Guide

a3bc5d9

JolienTrog requested a review from lippserd March 24, 2026 10:46

lippserd added this to the 2.13 milestone Mar 24, 2026

lippserd requested a review from Al2Klimov March 25, 2026 12:19

lippserd removed this from the 2.13 milestone Mar 25, 2026

Al2Klimov requested a review from TheSyscall March 30, 2026 10:59

TheSyscall requested changes Mar 31, 2026

View reviewed changes

	foreach (Hook::all('PasswordPolicy') as $class => $policy) {
	foreach (Hook::all(self::HOOK_NAME) as $class => $policy) {

Conversation

JolienTrog commented Aug 26, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Al2Klimov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!