codereview

This commit is contained in:
Jolien Trog 2025-09-11 13:28:29 +02:00
parent bcf8430dd6
commit a0147b8330
8 changed files with 36 additions and 42 deletions

View File

@ -4,8 +4,7 @@
namespace Icinga\Forms\Account; namespace Icinga\Forms\Account;
use Icinga\Application\Config; use Icinga\Application\Config;
use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\AnyPasswordPolicy;
use Icinga\Application\ProvidedHook\NoPasswordPolicy;
use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\PasswordValidator;
use Icinga\Authentication\User\DbUserBackend; use Icinga\Authentication\User\DbUserBackend;
use Icinga\Data\Filter\Filter; use Icinga\Data\Filter\Filter;
@ -38,15 +37,15 @@ class ChangePasswordForm extends Form
*/ */
public function createElements(array $formData) public function createElements(array $formData)
{ {
$passwordPolicy = Config::app()->get( $passwordPolicyClass = Config::app()->get(
'global', 'global',
'password_policy', 'password_policy',
NoPasswordPolicy::class AnyPasswordPolicy::class
); );
$passwordPolicyObject = new $passwordPolicy();
$passwordPolicyDescription = $passwordPolicyObject->getDescription();
if ($passwordPolicyDescription != '') { $passwordPolicy = new $passwordPolicyClass();
$passwordPolicyDescription = $passwordPolicy->getDescription();
if ($passwordPolicyDescription !== '') {
$this->addDescription($passwordPolicyDescription); $this->addDescription($passwordPolicyDescription);
} }
@ -61,11 +60,11 @@ class ChangePasswordForm extends Form
$this->addElement( $this->addElement(
'password', 'password',
'new_password', 'new_password',
array( [
'label' => $this->translate('New Password'), 'label' => $this->translate('New Password'),
'required' => true, 'required' => true,
'validators' => [new PasswordValidator($passwordPolicyObject)] 'validators' => [new PasswordValidator($passwordPolicy)]
) ]
); );
$this->addElement( $this->addElement(
'password', 'password',

View File

@ -4,7 +4,7 @@
namespace Icinga\Forms\Config\General; namespace Icinga\Forms\Config\General;
use Icinga\Application\Hook; use Icinga\Application\Hook;
use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Application\ProvidedHook\AnyPasswordPolicy;
use Icinga\Web\Form; use Icinga\Web\Form;
/** /**
@ -22,26 +22,22 @@ class PasswordPolicyConfigForm extends Form
public function createElements(array $formData): static public function createElements(array $formData): static
{ {
$passwordPolicies = []; $passwordPolicies = [];
foreach (Hook::all('passwordpolicy') as $class => $policy) { foreach (Hook::all('passwordpolicy') as $class => $policy) {
$passwordPolicies[$class] = $policy->getName(); $passwordPolicies[$class] = $policy->getName();
} }
asort($passwordPolicies); asort($passwordPolicies);
$this->addElement( $this->addElement(
'select', 'select',
'global_password_policy', 'global_password_policy',
[ [
'description' => $this->translate( 'description' => $this->translate('Enforce password requirements for new passwords'),
'Enforce strong password requirements for new passwords'
),
'label' => $this->translate('Password Policy'), 'label' => $this->translate('Password Policy'),
'value' => NoPasswordPolicy::class, 'value' => AnyPasswordPolicy::class,
'multiOptions' =>$passwordPolicies 'multiOptions' => $passwordPolicies
] ]
); );
return $this; return $this;
} }
} }

View File

@ -5,8 +5,7 @@ namespace Icinga\Forms\Config\User;
use Icinga\Application\Config; use Icinga\Application\Config;
use Icinga\Application\Hook\ConfigFormEventsHook; use Icinga\Application\Hook\ConfigFormEventsHook;
use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\AnyPasswordPolicy;
use Icinga\Application\ProvidedHook\NoPasswordPolicy;
use Icinga\Authentication\PasswordValidator; use Icinga\Authentication\PasswordValidator;
use Icinga\Data\Filter\Filter; use Icinga\Data\Filter\Filter;
use Icinga\Forms\RepositoryForm; use Icinga\Forms\RepositoryForm;
@ -21,14 +20,14 @@ class UserForm extends RepositoryForm
*/ */
protected function createInsertElements(array $formData) protected function createInsertElements(array $formData)
{ {
$passwordPolicy = Config::app()->get( $passwordPolicyClass = Config::app()->get(
'global', 'global',
'password_policy', 'password_policy',
NoPasswordPolicy::class AnyPasswordPolicy::class
); );
$passwordPolicyObject = new $passwordPolicy();
$passwordPolicyDescription = $passwordPolicyObject->getDescription();
$passwordPolicy = new $passwordPolicyClass();
$passwordPolicyDescription = $passwordPolicy->getDescription();
if ($passwordPolicyDescription != '') { if ($passwordPolicyDescription != '') {
$this->addDescription($passwordPolicyDescription); $this->addDescription($passwordPolicyDescription);
} }
@ -53,11 +52,11 @@ class UserForm extends RepositoryForm
$this->addElement( $this->addElement(
'password', 'password',
'password', 'password',
array( [
'required' => true, 'required' => true,
'label' => $this->translate('Password'), 'label' => $this->translate('Password'),
'validators' => [new PasswordValidator($passwordPolicyObject)] 'validators' => [new PasswordValidator($passwordPolicy)]
) ]
); );
$this->setTitle($this->translate('Add a new user')); $this->setTitle($this->translate('Add a new user'));

View File

@ -162,7 +162,7 @@ in order to manually create users directly inside the database.
Icinga Web 2 supports password policies when using database authentication. Icinga Web 2 supports password policies when using database authentication.
You can configure this under **Configuration > Application > General**. You can configure this under **Configuration > Application > General**.
By default, no password policy is enforced ('None'). By default, no password policy is enforced ('Any').
Icinga Web 2 provides a built-in policy called 'Common' with the following requirements: Icinga Web 2 provides a built-in policy called 'Common' with the following requirements:
* Minimum length of 12 characters * Minimum length of 12 characters

View File

@ -8,7 +8,7 @@ use ErrorException;
use Exception; use Exception;
use Icinga\Application\ProvidedHook\DbMigration; use Icinga\Application\ProvidedHook\DbMigration;
use Icinga\Application\ProvidedHook\CommonPasswordPolicy; use Icinga\Application\ProvidedHook\CommonPasswordPolicy;
use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Application\ProvidedHook\AnyPasswordPolicy;
use ipl\I18n\GettextTranslator; use ipl\I18n\GettextTranslator;
use ipl\I18n\StaticTranslator; use ipl\I18n\StaticTranslator;
use LogicException; use LogicException;
@ -743,7 +743,7 @@ abstract class ApplicationBootstrap
{ {
Hook::register('DbMigration', DbMigration::class, DbMigration::class); Hook::register('DbMigration', DbMigration::class, DbMigration::class);
Hook::register('passwordpolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class); Hook::register('passwordpolicy', CommonPasswordPolicy::class, CommonPasswordPolicy::class);
Hook::register('passwordpolicy', NoPasswordPolicy::class, NoPasswordPolicy::class); Hook::register('passwordpolicy', AnyPasswordPolicy::class, AnyPasswordPolicy::class);
return $this; return $this;
} }

View File

@ -9,13 +9,13 @@ use ipl\I18n\Translation;
/** /**
* None Password Policy to validate all passwords * None Password Policy to validate all passwords
*/ */
class NoPasswordPolicy implements PasswordPolicyHook class AnyPasswordPolicy implements PasswordPolicyHook
{ {
use Translation; use Translation;
public function getName(): string public function getName(): string
{ {
return $this->translate('None'); return $this->translate('Any');
} }
public function getDescription(): string public function getDescription(): string

View File

@ -4,20 +4,20 @@ namespace Tests\Icinga\Application;
use Icinga\Application\Hook\PasswordPolicyHook; use Icinga\Application\Hook\PasswordPolicyHook;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Icinga\Application\ProvidedHook\NoPasswordPolicy; use Icinga\Application\ProvidedHook\AnyPasswordPolicy;
class NoPasswordPolicyTest extends TestCase class AnyPasswordPolicyTest extends TestCase
{ {
private PasswordPolicyHook $instance; private PasswordPolicyHook $instance;
public function setUp(): void public function setUp(): void
{ {
$this->instance = new NoPasswordPolicy(); $this->instance = new AnyPasswordPolicy();
} }
public function testMethodGetName(): void public function testMethodGetName(): void
{ {
$this->assertSame('None', $this->instance->getName()); $this->assertSame('Any', $this->instance->getName());
} }
public function testValidatePasswordValid(): void public function testValidatePasswordValid(): void