Skip to content

[Validator] Remove bjeavons/zxcvbn-php in favor of a builtin solution #49856

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

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@
"symfony/security-acl": "~2.8|~3.0",
"twig/cssinliner-extra": "^2.12|^3",
"twig/inky-extra": "^2.12|^3",
"twig/markdown-extra": "^2.12|^3",
"bjeavons/zxcvbn-php": "^1.0"
"twig/markdown-extra": "^2.12|^3"
},
"conflict": {
"ext-psr": "<1.1|>=2",
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CHANGELOG
* Add `Uuid::TIME_BASED_VERSIONS` to match that a UUID being validated embeds a timestamp
* Add the `pattern` parameter in violations of the `Regex` constraint
* Add a `NoSuspiciousCharacters` constraint to validate a string is not a spoofing attempt
* Add a `PasswordStrength` constraint to check the strength of a password (requires `bjeavons/zxcvbn-php` library)
* Add a `PasswordStrength` constraint to check the strength of a password
* Add the `countUnit` option to the `Length` constraint to allow counting the string length either by code points (like before, now the default setting `Length::COUNT_CODEPOINTS`), bytes (`Length::COUNT_BYTES`) or graphemes (`Length::COUNT_GRAPHEMES`)
* Add the `filenameMaxLength` option to the `File` constraint
* Add the `exclude` option to the `Cascade` constraint
Expand Down
38 changes: 11 additions & 27 deletions src/Symfony/Component/Validator/Constraints/PasswordStrength.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
use Symfony\Component\Validator\Exception\LogicException;
use ZxcvbnPhp\Zxcvbn;

/**
* @Annotation
Expand All @@ -26,42 +24,28 @@
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
final class PasswordStrength extends Constraint
{
public const STRENGTH_VERY_WEAK = 0;
public const STRENGTH_WEAK = 1;
public const STRENGTH_REASONABLE = 2;
public const STRENGTH_STRONG = 3;
public const STRENGTH_VERY_STRONG = 4;

public const PASSWORD_STRENGTH_ERROR = '4234df00-45dd-49a4-b303-a75dbf8b10d8';
public const RESTRICTED_USER_INPUT_ERROR = 'd187ff45-bf23-4331-aa87-c24a36e9b400';

protected const ERROR_NAMES = [
self::PASSWORD_STRENGTH_ERROR => 'PASSWORD_STRENGTH_ERROR',
self::RESTRICTED_USER_INPUT_ERROR => 'RESTRICTED_USER_INPUT_ERROR',
];

public string $lowStrengthMessage = 'The password strength is too low. Please use a stronger password.';

public int $minScore = 2;
public string $message = 'The password strength is too low. Please use a stronger password.';

public string $restrictedDataMessage = 'The password contains the following restricted data: {{ wordList }}.';
public int $minScore;

/**
* @var array<string>
*/
public array $restrictedData = [];

public function __construct(mixed $options = null, array $groups = null, mixed $payload = null)
public function __construct(int $minScore = self::STRENGTH_REASONABLE, mixed $options = null, array $groups = null, mixed $payload = null)
{
if (!class_exists(Zxcvbn::class)) {
throw new LogicException(sprintf('The "%s" class requires the "bjeavons/zxcvbn-php" library. Try running "composer require bjeavons/zxcvbn-php".', self::class));
}

if (isset($options['minScore']) && (!\is_int($options['minScore']) || $options['minScore'] < 1 || $options['minScore'] > 4)) {
if (isset($minScore) && (!\is_int($minScore) || $minScore < 1 || $minScore > 4)) {
throw new ConstraintDefinitionException(sprintf('The parameter "minScore" of the "%s" constraint must be an integer between 1 and 4.', static::class));
}

if (isset($options['restrictedData'])) {
array_walk($options['restrictedData'], static function (mixed $value): void {
if (!\is_string($value)) {
throw new ConstraintDefinitionException(sprintf('The parameter "restrictedData" of the "%s" constraint must be a list of strings.', static::class));
}
});
}
$options['minScore'] = $minScore;
parent::__construct($options, $groups, $payload);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Exception\UnexpectedValueException;
use ZxcvbnPhp\Matchers\DictionaryMatch;
use ZxcvbnPhp\Matchers\MatchInterface;
use ZxcvbnPhp\Zxcvbn;

final class PasswordStrengthValidator extends ConstraintValidator
{
/**
* @param (\Closure(string):PasswordStrength::STRENGTH_*)|null $passwordStrengthEstimator
*/
public function __construct(
private readonly ?\Closure $passwordStrengthEstimator = null,
) {
}

public function validate(#[\SensitiveParameter] mixed $value, Constraint $constraint): void
{
if (!$constraint instanceof PasswordStrength) {
Expand All @@ -34,43 +39,33 @@ public function validate(#[\SensitiveParameter] mixed $value, Constraint $constr
if (!\is_string($value)) {
throw new UnexpectedValueException($value, 'string');
}
$passwordStrengthEstimator = $this->passwordStrengthEstimator ?? self::estimateStrength(...);
$strength = $passwordStrengthEstimator($value);

$zxcvbn = new Zxcvbn();
$strength = $zxcvbn->passwordStrength($value, $constraint->restrictedData);

if ($strength['score'] < $constraint->minScore) {
$this->context->buildViolation($constraint->lowStrengthMessage)
if ($strength < $constraint->minScore) {
$this->context->buildViolation($constraint->message)
->setCode(PasswordStrength::PASSWORD_STRENGTH_ERROR)
->addViolation();
}
$wordList = $this->findRestrictedUserInputs($strength['sequence'] ?? []);
if (0 !== \count($wordList)) {
$this->context->buildViolation($constraint->restrictedDataMessage, [
'{{ wordList }}' => implode(', ', $wordList),
])
->setCode(PasswordStrength::RESTRICTED_USER_INPUT_ERROR)
->addViolation();
}
}

/**
* @param array<MatchInterface> $sequence
* Returns the estimated strength of a password.
*
* The higher the value, the stronger the password.
*
* @return array<string>
* @return PasswordStrength::STRENGTH_*
*/
private function findRestrictedUserInputs(array $sequence): array
private static function estimateStrength(#[\SensitiveParameter] string $password): int
{
$found = [];

foreach ($sequence as $item) {
if (!$item instanceof DictionaryMatch) {
continue;
}
if ('user_inputs' === $item->dictionaryName) {
$found[] = $item->token;
}
}
$entropy = log(\strlen(count_chars($password, 3)) ** \strlen($password), 2);

return $found;
return match (true) {
$entropy >= 120 => PasswordStrength::STRENGTH_VERY_STRONG,
$entropy >= 100 => PasswordStrength::STRENGTH_STRONG,
$entropy >= 80 => PasswordStrength::STRENGTH_REASONABLE,
$entropy >= 60 => PasswordStrength::STRENGTH_WEAK,
default => PasswordStrength::STRENGTH_VERY_WEAK,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,27 @@ class PasswordStrengthTest extends TestCase
public function testConstructor()
{
$constraint = new PasswordStrength();
$this->assertEquals(2, $constraint->minScore);
$this->assertEquals([], $constraint->restrictedData);
$this->assertSame(2, $constraint->minScore);
}

public function testConstructorWithParameters()
{
$constraint = new PasswordStrength([
'minScore' => 3,
'restrictedData' => ['foo', 'bar'],
]);
$constraint = new PasswordStrength(minScore: PasswordStrength::STRENGTH_STRONG);

$this->assertEquals(3, $constraint->minScore);
$this->assertEquals(['foo', 'bar'], $constraint->restrictedData);
$this->assertSame(PasswordStrength::STRENGTH_STRONG, $constraint->minScore);
}

public function testInvalidScoreOfZero()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('The parameter "minScore" of the "Symfony\Component\Validator\Constraints\PasswordStrength" constraint must be an integer between 1 and 4.');
new PasswordStrength(['minScore' => 0]);
new PasswordStrength(minScore: PasswordStrength::STRENGTH_VERY_WEAK);
}

public function testInvalidScoreOfFive()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('The parameter "minScore" of the "Symfony\Component\Validator\Constraints\PasswordStrength" constraint must be an integer between 1 and 4.');
new PasswordStrength(['minScore' => 5]);
}

public function testInvalidRestrictedData()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('The parameter "restrictedData" of the "Symfony\Component\Validator\Constraints\PasswordStrength" constraint must be a list of strings.');
new PasswordStrength(['restrictedData' => [123]]);
new PasswordStrength(minScore: 5);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,19 @@ protected function createValidator(): PasswordStrengthValidator
/**
* @dataProvider getValidValues
*/
public function testValidValues(string $value)
public function testValidValues(string $value, int $expectedStrength)
{
$this->validator->validate($value, new PasswordStrength());
$this->validator->validate($value, new PasswordStrength(minScore: $expectedStrength));

$this->assertNoViolation();
}

public static function getValidValues(): iterable
{
yield ['This 1s a very g00d Pa55word! ;-)'];
yield ['How-is this 🤔?!', PasswordStrength::STRENGTH_WEAK];
yield ['Reasonable-pwd-❤️', PasswordStrength::STRENGTH_REASONABLE];
yield ['This 1s a very g00d Pa55word! ;-)', PasswordStrength::STRENGTH_VERY_STRONG];
yield ['pudding-smack-👌🏼-fox-😎', PasswordStrength::STRENGTH_VERY_STRONG];
}

/**
Expand All @@ -59,23 +62,10 @@ public static function provideInvalidConstraints(): iterable
PasswordStrength::PASSWORD_STRENGTH_ERROR,
];
yield [
new PasswordStrength([
'minScore' => 4,
]),
new PasswordStrength(minScore: PasswordStrength::STRENGTH_VERY_STRONG),
'Good password?',
'The password strength is too low. Please use a stronger password.',
PasswordStrength::PASSWORD_STRENGTH_ERROR,
];
yield [
new PasswordStrength([
'restrictedData' => ['symfony'],
]),
'SyMfONY-framework-john',
'The password contains the following restricted data: {{ wordList }}.',
PasswordStrength::RESTRICTED_USER_INPUT_ERROR,
[
'{{ wordList }}' => 'SyMfONY',
],
];
}
}
3 changes: 1 addition & 2 deletions src/Symfony/Component/Validator/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
"symfony/property-info": "^5.4|^6.0",
"symfony/translation": "^5.4|^6.0",
"doctrine/annotations": "^1.13|^2",
"egulias/email-validator": "^2.1.10|^3|^4",
"bjeavons/zxcvbn-php": "^1.0"
"egulias/email-validator": "^2.1.10|^3|^4"
},
"conflict": {
"doctrine/annotations": "<1.13",
Expand Down