codereview

This commit is contained in:
Jolien Trog 2025-09-10 13:10:52 +02:00
parent a1ab6975d5
commit 6d2a3ca6cf
6 changed files with 35 additions and 45 deletions

3
.gitignore vendored
View File

@ -20,6 +20,3 @@ build/*
# Exclude dompdf font cache
library/vendor/dompdf/lib/fonts/*.php
library/vendor/dompdf/lib/fonts/log.htm
#locale änderung für Tests sollen nicht gepushed werden
composer.json

View File

@ -4,7 +4,6 @@
namespace Icinga\Forms\Config\General;
use Icinga\Application\Hook;
use Icinga\Application\ProvidedHook\CommonPasswordPolicy;
use Icinga\Application\ProvidedHook\NoPasswordPolicy;
use Icinga\Web\Form;
@ -20,7 +19,7 @@ class PasswordPolicyConfigForm extends Form
$this->setName('form_config_general_password_policy');
}
public function createElements(array $formData): self
public function createElements(array $formData): static
{
$passwordPolicies = [];

View File

@ -23,7 +23,7 @@ interface PasswordPolicyHook
* Validate a given password against the defined policy
*
* @param string $password
* @return array Returns an empty array if the password is valid,
* @return string[] Returns an empty array if the password is valid,
* otherwise returns an error message describing the violations
*/
public function validatePassword(string $password): array;

View File

@ -38,33 +38,23 @@ class CommonPasswordPolicy implements PasswordPolicyHook
$violations = [];
if (mb_strlen($password) < 12) {
$violations[] = $this->translate(
'Password must be at least 12 characters long'
);
$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'
);
$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'
);
$violations[] = $this->translate('Password must contain at least one special character');
}
if (! preg_match('/[A-Z]/', $password)) {
$violations[] = $this->translate(
'Password must contain at least one uppercase letter'
);
$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'
);
$violations[] = $this->translate('Password must contain at least one lowercase letter');
}
return $violations;

View File

@ -4,9 +4,9 @@ namespace Tests\Icinga\Application;
use Icinga\Application\Hook\PasswordPolicyHook;
use Icinga\Application\ProvidedHook\CommonPasswordPolicy;
use Icinga\Test\BaseTestCase;
use PHPUnit\Framework\TestCase;
class CommonPasswordPolicyTest extends BaseTestCase
class CommonPasswordPolicyTest extends TestCase
{
private PasswordPolicyHook $instance;
@ -22,40 +22,40 @@ class CommonPasswordPolicyTest extends BaseTestCase
public function testValidatePasswordTooShort(): void
{
$expectedResults = ['Password must be at least 12 characters long'];
$res = $this->instance->validatePassword('Icinga1#');
$this->assertSame('Password must be at least 12 characters long', $res[0]);
$this->assertCount(1, $res);
$this->assertSame($expectedResults, $res);
}
public function testValidatePasswordNoNumber(): void
{
$expectedResults = ['Password must contain at least one number'];
$res = $this->instance->validatePassword('Icingaadmin#');
$this->assertSame('Password must contain at least one number', $res[0]);
$this->assertCount(1, $res);
$this->assertSame($expectedResults, $res);
}
public function testValidatePasswordNoSpecialCharacter(): void
{
$expectedResult = ['Password must contain at least one special character'];
$res = $this->instance->validatePassword('Icingaadmin1');
$this->assertSame('Password must contain at least one special character', $res[0]);
$this->assertCount(1, $res);
$this->assertSame($expectedResult, $res);
}
public function testValidatePasswordNoUpperCaseLetters(): void
{
$expectedResult = ['Password must contain at least one uppercase letter'];
$res = $this->instance->validatePassword('icingaadmin1#');
$this->assertSame('Password must contain at least one uppercase letter', $res[0]);
$this->assertCount(1, $res);
$this->assertSame($expectedResult, $res);
}
public function testValidatePasswordNoLowerCaseLetters(): void
{
$expectedResult = ['Password must contain at least one lowercase letter'];
$res = $this->instance->validatePassword('ICINGAADMIN1#');
$this->assertSame('Password must contain at least one lowercase letter', $res[0]);
$this->assertCount(1, $res);
$this->assertSame($expectedResult, $res);
}
public function testValidatePasswordValid(): void
public function testMethodValidatePasswordAlwaysReturnAnEmptyArray(): void
{
$res = $this->instance->validatePassword('Icingaadmin1#');
$this->assertEmpty($res);
@ -63,21 +63,25 @@ class CommonPasswordPolicyTest extends BaseTestCase
public function testValidatePasswordOnlyLowerCaseLetters(): void
{
$expectedResult = [
'Password must contain at least one number',
'Password must contain at least one special character',
'Password must contain at least one uppercase letter'
];
$res = $this->instance->validatePassword('icingawebadmin');
$this->assertCount(3, $res);
$this->assertSame('Password must contain at least one number', $res[0]);
$this->assertSame('Password must contain at least one special character', $res[1]);
$this->assertSame('Password must contain at least one uppercase letter', $res[2]);
$this->assertSame($expectedResult, $res);
}
public function testValidatePasswordWithLengthAndUpperCaseLetters(): void
{
$expectedResult = [
'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',
];
$res = $this->instance->validatePassword('ICINGAADMIN');
$this->assertCount(4, $res);
$this->assertSame('Password must be at least 12 characters long', $res[0]);
$this->assertSame('Password must contain at least one number', $res[1]);
$this->assertSame('Password must contain at least one special character', $res[2]);
$this->assertSame('Password must contain at least one lowercase letter', $res[3]);
$this->assertSame($expectedResult, $res);
}
public function testValidatePasswordWithManyCharacters(): void

View File

@ -3,10 +3,10 @@
namespace Tests\Icinga\Application;
use Icinga\Application\Hook\PasswordPolicyHook;
use Icinga\Test\BaseTestCase;
use PHPUnit\Framework\TestCase;
use Icinga\Application\ProvidedHook\NoPasswordPolicy;
class NoPasswordPolicyTest extends BaseTestCase
class NoPasswordPolicyTest extends TestCase
{
private PasswordPolicyHook $instance;