Skip to content

fix: Unicode support for NothingPersonalValidator #1406

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.

Sign up for GitHub

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

Open
neznaika0 wants to merge 1 commit into
base: develop
Choose a base branch
from
+219 −16
Open

fix: Unicode support for NothingPersonalValidator #1406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 16 additions & 16 deletions src/Authentication/Passwords/NothingPersonalValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/
public function check(string $password, ?User $user = null): Result
{
$password = strtolower($password);
$password = mb_strtolower($password);

Check warning on line 34 in src/Authentication/Passwords/NothingPersonalValidator.php

View workflow job for this annotation

GitHub Actions / Infection 

Escaped Mutant for Mutator "MBString":

@@ @@
      */
     public function check(string $password, ?User $user = null): Result
     {
-        $password = mb_strtolower($password);
+        $password = strtolower($password);
 
         if ($valid = $this->isNotPersonal($password, $user)) {
             $valid = $this->isNotSimilar($password, $user);

@paulbalandan paulbalandan Nov 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MBString mutation suggests the tests did not cover multibyte passwords, which this change is anchoring on.

@neznaika0 neznaika0 Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven’t used Infection. Does this mean that my version of the tests works in both strtolower() variants?
Perhaps it will work if the exact strings are the same. But we understand that there is a difference:

$ php -r "var_dump(strtolower('ПёТр'), mb_strtolower('ПёТр')); ";
string(8) "ПёТр"
string(8) "пётр"

@paulbalandan paulbalandan Nov 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, your test passes in both variants. infection must see your test fails when it changes things to say you covered the mutant


if ($valid = $this->isNotPersonal($password, $user)) {
$valid = $this->isNotSimilar($password, $user);
Expand All @@ -45,8 +45,6 @@
}

/**
* isNotPersonal()
*
* Looks for personal information in a password. The personal info used
* comes from CodeIgniter\Shield\Entities\User properties username and email.
*
Expand All @@ -58,13 +56,12 @@
*
* public $personalFields = ['firstname', 'lastname'];
*
* isNotPersonal() returns true if no personal information can be found, or false
* if such info is found.
* @return bool `true` if no personal information can be found, or `false` if such info is found.
*/
protected function isNotPersonal(string $password, ?User $user): bool
{
$userName = strtolower($user->username ?? '');
$email = strtolower($user->email);
$userName = mb_strtolower($user->username ?? '');
$email = mb_strtolower($user->email ?? '');
$valid = true;

// The most obvious transgressions
Expand Down Expand Up @@ -95,10 +92,11 @@

foreach ($personalFields as $value) {
if (($user->{$value} ?? '') !== '') {
$needles[] = strtolower($user->{$value});
$needles[] = mb_strtolower($user->{$value});
}
}

$needles = array_unique($needles, SORT_LOCALE_STRING);

Check warning on line 99 in src/Authentication/Passwords/NothingPersonalValidator.php

View workflow job for this annotation

GitHub Actions / Infection 

Escaped Mutant for Mutator "UnwrapArrayUnique":

@@ @@
                 }
             }
 
-            $needles = array_unique($needles, SORT_LOCALE_STRING);
+            $needles = $needles;
             $trivial = [
                 'a',
                 'an',
$trivial = [
'a',
'an',
Expand All @@ -121,12 +119,12 @@
$haystacks = $this->strip_explode($password);

foreach ($haystacks as $haystack) {
if (in_array($haystack, $trivial, true) || mb_strlen($haystack, 'UTF-8') < 3) {
if (in_array($haystack, $trivial, true) || mb_strlen($haystack) < 3) {
continue; // ignore trivial words
}

foreach ($needles as $needle) {
if (in_array($needle, $trivial, true) || mb_strlen($needle, 'UTF-8') < 3) {
if (in_array($needle, $trivial, true) || mb_strlen($needle) < 3) {
continue;
}

Expand All @@ -139,6 +137,7 @@
}
}
}

if ($valid) {
return true;
}
Expand All @@ -150,17 +149,19 @@
}

/**
* notSimilar() uses $password and $userName to calculate a similarity value.
* Method uses password and username to calculate a similarity value.
* Similarity values equal to, or greater than CodeIgniter\Shield\Config::maxSimilarity
* are rejected for being too much alike and false is returned.
* Otherwise, true is returned,
*
* A $maxSimilarity value of 0 (zero) returns true without making a comparison.
* In other words, 0 (zero) turns off similarity testing.
*
* @return bool `true` if a similarity is found, or `false` if such info is found.
*/
protected function isNotSimilar(string $password, ?User $user): bool
{
if ($user->username === null) {
if (! $user instanceof User || $user->username === null) {
return true;
}

Expand All @@ -173,9 +174,10 @@
}

if ($maxSimilarity > 0) {
$userName = strtolower($user->username);
$userName = mb_strtolower($user->username);

similar_text($password, $userName, $similarity);

if ($similarity >= $maxSimilarity) {
$this->error = lang('Auth.errorPasswordTooSimilar');
$this->suggestion = lang('Auth.suggestPasswordTooSimilar');
Expand All @@ -188,16 +190,14 @@
}

/**
* strip_explode($str)
*
* Replaces all non-word characters and underscores in $str with a space.
* Then it explodes that result using the space for a delimiter.
*
* @return array<int, string>
*/
protected function strip_explode(string $str): array

Check warning on line 198 in src/Authentication/Passwords/NothingPersonalValidator.php

View workflow job for this annotation

GitHub Actions / Infection 

Escaped Mutant for Mutator "ProtectedVisibility":

@@ @@
      *
      * @return array<int, string>
      */
-    protected function strip_explode(string $str): array
+    private function strip_explode(string $str): array
     {
         $stripped = preg_replace('/[\W_]+/u', ' ', $str);
         $parts    = explode(' ', trim((string) $stripped));
{
$stripped = preg_replace('/[\W_]+/', ' ', $str);
$stripped = preg_replace('/[\W_]+/u', ' ', $str);
$parts = explode(' ', trim((string) $stripped));

// If it's not already there put the untouched input at the top of the array
Expand Down
203 changes: 203 additions & 0 deletions tests/Unit/NothingPersonalValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,4 +376,207 @@ public static function provideCheckPasswordWithBadEmail(): iterable
],
];
}

#[DataProvider('provideIsolatedMethodIsNotPersonal')]
public function testIsolatedMethodIsNotPersonal(mixed ...$input): void
{
$user = null;
$fields = [];
// Password removes spaces inside `check()`, so we’ll duplicate it here
$password = mb_strtolower($input[0]);

if (is_array($input[1])) {
$user = new User($input[1]);
$fields = array_keys($input[1]);
}

$config = new Auth();
$config->personalFields = array_filter($fields, static fn ($val): bool => $val !== 'email' && $val !== 'username');
$validator = new NothingPersonalValidator($config);

$isNotPersonal = self::getPrivateMethodInvoker($validator, 'isNotPersonal');

/** @var bool */
$result = $isNotPersonal($password, $user);

$this->assertSame($input[2], $result);
}

public static function provideIsolatedMethodIsNotPersonal(): iterable
{
// @todo: strrev() doesn’t work with Unicode
// yield 'Password is equal to reversed username' => [
// 'ПеТьКа',
// [
// 'email' => 'аДмИн@СаЙт.Ру',
// 'username' => 'аКьТеП'
// ],
// false
// ];

yield 'Empty password and username' => [
'',
[
'email' => 'админ@сайт.ру',
],
false,
];

yield 'Password is equal to username' => [
'ПаРоЛь',
[
'email' => 'админ@сайт.ру',
'username' => 'пАрОлЬ',
],
false,
];

yield 'Password is equal to email' => [
'аДмИн@СаЙт.Ру',
[
'email' => 'АдМиН@сАйТ.рУ',
'username' => 'Петя',
],
false,
];

yield 'Password contains the username' => [
'ПеТя-9000',
[
'email' => 'админ@сайт.ру',
'username' => 'пЕтЯ',
],
false,
];

yield 'Password contains the email' => [
'ПеТя-9000',
[
'email' => 'админ@петя.ру',
'username' => 'Пётр',
],
false,
];

yield 'Password contains the personal data' => [
'_ПеТр_9000',
[
'email' => 'админ@caйт.ру',
'username' => 'Пётр',
'lastname' => 'Петров',
],
false,
];

yield 'Empty password' => [
'',
[
'email' => 'админ@сайт.ру',
'username' => 'Петя',
],
true,
];

yield 'Only spaces in password' => [
' ',
[
'email' => 'админ@сайт.ру',
'username' => 'Петя',
],
true,
];

yield 'Password without user' => [
'_ПеТр_9000',
null,
true,
];
}

#[DataProvider('provideIsolatedMethodIsNotSimilar')]
public function testIsolatedMethodIsNotSimilar(mixed ...$input): void
{
$user = null;
// Password removes spaces inside `check()`, so we’ll duplicate it here
$password = mb_strtolower($input[0]);

if (is_array($input[1])) {
$user = new User($input[1]);
}

$config = new Auth();
$config->maxSimilarity = $input[2];
$validator = new NothingPersonalValidator($config);

$isNotSimilar = self::getPrivateMethodInvoker($validator, 'isNotSimilar');

/** @var bool */
$result = $isNotSimilar($password, $user);

$this->assertSame($input[3], $result);
}

public static function provideIsolatedMethodIsNotSimilar(): iterable
{
yield 'Same password and username (similar > 1%)' => [
'ПёТр',
[
'username' => 'ПёТр',
],
1,
false,
];

yield 'Same password and username (similar > 100%)' => [
'ПёТр',
[
'username' => 'ПёТр',
],
150,
false,
];

yield 'Empty password and username (similar < 1%)' => [
'',
[
'username' => '',
],
0,
true,
];

yield 'Empty password and username (similar > 1%)' => [
'',
[
'username' => '',
],
1,
true,
];

yield 'Empty password and specified username (similar > 1%)' => [
'',
[
'username' => 'ПёТр',
],
1,
true,
];

yield 'Empty username and specified password (similar > 1%)' => [
'ПаРоЛь',
[
'username' => '',
],
1,
true,
];

yield 'Empty User and specified password (similar > 1%)' => [
'ПаРоЛь',
null,
1,
true,
];
}
}
Loading