bug #78 Fix PasswordRequirements/PasswordStrength unicode validation …

…(sstok) This PR was merged into the 1.5-dev branch. Discussion ---------- | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #68 | License | MIT ## PasswordStrength unicode support This adds a new option `unicodeEquality` to thread `²` as number rather then a special character. This option is false by default, for compatibility. But should be enabled for better security. I'm not sure if `unicodeEquality` is good name for this option, so if you know a better know please let me know. Commits ------- d30a940 Fix PasswordRequirements unicode length validation 12881f3 Fix PasswordStrength unicode support
rollerworks · Apr 19, 2017 · 50132bf · 50132bf
2 parents 87da931 + 12881f3
commit 50132bf
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 34 deletions.
diff --git a/docs/strength-validation.md b/docs/strength-validation.md
@@ -7,9 +7,10 @@ constraint with the following options.
 * message: The validation message (default: password_too_weak)
 * minLength: Minimum length of the password, should be at least 6 (or 8 for better security)
 * minStrength: Minimum required strength of the password.
+* unicodeEquality: Consider characters from other scripts (unicode) as equal.
+  When set to false (default) `²` will seen as a special character rather then then 2 in another script.
 
-The strength is computed from various measures including
-length and usage of (special) characters.
+The strength is computed from various measures including length and usage of (special) characters.
 
 **Note:** A strength is measured by the presence of a character and total length.
 One can have a 'medium' password consisting of only a-z and A-Z, but with a length higher than 12 characters.

diff --git a/src/Validator/Constraints/PasswordRequirementsValidator.php b/src/Validator/Constraints/PasswordRequirementsValidator.php
@@ -27,7 +27,7 @@ public function validate($value, Constraint $constraint)
             return;
         }
 
-        if ($constraint->minLength > 0 && (strlen($value) < $constraint->minLength)) {
+        if ($constraint->minLength > 0 && (mb_strlen($value) < $constraint->minLength)) {
             if ($this->context instanceof ExecutionContextInterface) {
                 $this->context->buildViolation($constraint->tooShortMessage)
                     ->setParameters(array('{{length}}' => $constraint->minLength))
@@ -38,7 +38,7 @@ public function validate($value, Constraint $constraint)
             }
         }
 
-        if ($constraint->requireLetters && !preg_match('/\pL/', $value)) {
+        if ($constraint->requireLetters && !preg_match('/\pL/u', $value)) {
             if ($this->context instanceof ExecutionContextInterface) {
                 $this->context->buildViolation($constraint->missingLettersMessage)
                     ->setInvalidValue($value)
@@ -48,7 +48,7 @@ public function validate($value, Constraint $constraint)
             }
         }
 
-        if ($constraint->requireCaseDiff && !preg_match('/(\p{Ll}+.*\p{Lu})|(\p{Lu}+.*\p{Ll})/', $value)) {
+        if ($constraint->requireCaseDiff && !preg_match('/(\p{Ll}+.*\p{Lu})|(\p{Lu}+.*\p{Ll})/u', $value)) {
             if ($this->context instanceof ExecutionContextInterface) {
                 $this->context->buildViolation($constraint->requireCaseDiffMessage)
                     ->setInvalidValue($value)
@@ -58,7 +58,7 @@ public function validate($value, Constraint $constraint)
             }
         }
 
-        if ($constraint->requireNumbers && !preg_match('/\pN/', $value)) {
+        if ($constraint->requireNumbers && !preg_match('/\pN/u', $value)) {
             if ($this->context instanceof ExecutionContextInterface) {
                 $this->context->buildViolation($constraint->missingNumbersMessage)
                     ->setInvalidValue($value)
@@ -68,9 +68,7 @@ public function validate($value, Constraint $constraint)
             }
         }
 
-        if ($constraint->requireSpecialCharacter &&
-            !preg_match('/[^p{Ll}\p{Lu}\pL\pN]/', $value)
-        ) {
+        if ($constraint->requireSpecialCharacter && !preg_match('/[^p{Ll}\p{Lu}\pL\pN]/u', $value)) {
             if ($this->context instanceof ExecutionContextInterface) {
                 $this->context->buildViolation($constraint->missingSpecialCharacterMessage)
                     ->setInvalidValue($value)

diff --git a/src/Validator/Constraints/PasswordStrength.php b/src/Validator/Constraints/PasswordStrength.php
@@ -22,6 +22,7 @@ class PasswordStrength extends Constraint
     public $message = 'password_too_weak';
     public $minLength = 6;
     public $minStrength;
+    public $unicodeEquality = false;
 
     /**
      * {@inheritdoc}

diff --git a/src/Validator/Constraints/PasswordStrengthValidator.php b/src/Validator/Constraints/PasswordStrengthValidator.php
@@ -85,8 +85,6 @@ public function validate($password, Constraint $constraint)
         }
 
         $password = (string) $password;
-
-        $passwordStrength = 0;
         $passLength = mb_strlen($password);
 
         if ($passLength < $constraint->minLength) {
@@ -103,30 +101,10 @@ public function validate($password, Constraint $constraint)
 
         $tips = array();
 
-        if (preg_match('/[a-zA-Z]/', $password)) {
-            ++$passwordStrength;
-
-            if (!preg_match('/[a-z]/', $password)) {
-                $tips[] = 'lowercase_letters';
-            } elseif (preg_match('/[A-Z]/', $password)) {
-                ++$passwordStrength;
-            } else {
-                $tips[] = 'uppercase_letters';
-            }
-        } else {
-            $tips[] = 'letters';
-        }
-
-        if (preg_match('/\d+/', $password)) {
-            ++$passwordStrength;
+        if ($constraint->unicodeEquality) {
+            $passwordStrength = $this->calculateStrengthUnicode($password, $tips);
         } else {
-            $tips[] = 'numbers';
-        }
-
-        if (preg_match('/[^a-zA-Z0-9]/', $password)) {
-            ++$passwordStrength;
-        } else {
-            $tips[] = 'special_chars';
+            $passwordStrength = $this->calculateStrength($password, $tips);
         }
 
         if ($passLength > 12) {
@@ -162,4 +140,70 @@ public function translateTips($tip)
     {
         return $this->translator->trans('rollerworks_password.tip.'.$tip, array(), 'validators');
     }
+
+    private function calculateStrength($password, &$tips)
+    {
+        $passwordStrength = 0;
+
+        if (preg_match('/[a-zA-Z]/', $password)) {
+            ++$passwordStrength;
+
+            if (!preg_match('/[a-z]/', $password)) {
+                $tips[] = 'lowercase_letters';
+            } elseif (preg_match('/[A-Z]/', $password)) {
+                ++$passwordStrength;
+            } else {
+                $tips[] = 'uppercase_letters';
+            }
+        } else {
+            $tips[] = 'letters';
+        }
+
+        if (preg_match('/\d+/', $password)) {
+            ++$passwordStrength;
+        } else {
+            $tips[] = 'numbers';
+        }
+
+        if (preg_match('/[^a-zA-Z0-9]/', $password)) {
+            ++$passwordStrength;
+        } else {
+            $tips[] = 'special_chars';
+        }
+
+        return $passwordStrength;
+    }
+
+    private function calculateStrengthUnicode($password, &$tips)
+    {
+        $passwordStrength = 0;
+
+        if (preg_match('/\p{L}/u', $password)) {
+            ++$passwordStrength;
+
+            if (!preg_match('/\p{Ll}/u', $password)) {
+                $tips[] = 'lowercase_letters';
+            } elseif (preg_match('/\p{Lu}/u', $password)) {
+                ++$passwordStrength;
+            } else {
+                $tips[] = 'uppercase_letters';
+            }
+        } else {
+            $tips[] = 'letters';
+        }
+
+        if (preg_match('/\p{N}/u', $password)) {
+            ++$passwordStrength;
+        } else {
+            $tips[] = 'numbers';
+        }
+
+        if (preg_match('/[^\p{L}\p{N}]/u', $password)) {
+            ++$passwordStrength;
+        } else {
+            $tips[] = 'special_chars';
+        }
+
+        return $passwordStrength;
+    }
 }
diff --git a/tests/Validator/PasswordRequirementsValidatorTest.php b/tests/Validator/PasswordRequirementsValidatorTest.php
@@ -130,6 +130,9 @@ public function provideViolationConstraints()
         $constraint = new PasswordRequirements();
 
         return array(
+            array('１', new PasswordRequirements(array('minLength' => 2, 'requireLetters' => false)), array(
+                array($constraint->tooShortMessage, array('{{length}}' => 2)),
+            )),
             array('test', new PasswordRequirements(array('requireLetters' => true)), array(
                 array($constraint->tooShortMessage, array('{{length}}' => $constraint->minLength)),
             )),

diff --git a/tests/Validator/PasswordStrengthTest.php b/tests/Validator/PasswordStrengthTest.php
@@ -110,6 +110,40 @@ public function getWeakPasswords()
         );
     }
 
+    public function getWeakPasswordsUnicode()
+    {
+        $pre = 'rollerworks_password.tip.';
+
+        // \u{FD3E} = ﴾ = Arabic ornate left parenthesis
+
+        return array(
+            // Very weak
+            array(2, 'weaker', 1, "{$pre}uppercase_letters, {$pre}numbers, {$pre}special_chars, {$pre}length"),
+            array(2, '123456', 1, "{$pre}letters, {$pre}special_chars, {$pre}length"),
+            array(2, '²²²²²²', 1, "{$pre}letters, {$pre}special_chars, {$pre}length"),
+            array(2, 'foobar', 1, "{$pre}uppercase_letters, {$pre}numbers, {$pre}special_chars, {$pre}length"),
+            array(2, 'ömgwat', 1, "{$pre}uppercase_letters, {$pre}numbers, {$pre}special_chars, {$pre}length"),
+            array(2, '!.!.!.', 1, "{$pre}letters, {$pre}numbers, {$pre}length"),
+            array(2, '!.!.!﴾', 1, "{$pre}letters, {$pre}numbers, {$pre}length"),
+
+            // Weak
+            array(3, 'wee6eak', 2, "{$pre}uppercase_letters, {$pre}special_chars, {$pre}length"),
+            array(3, 'foobar!', 2, "{$pre}uppercase_letters, {$pre}numbers, {$pre}length"),
+            array(3, 'Foobar', 2, "{$pre}numbers, {$pre}special_chars, {$pre}length"),
+            array(3, '123456!', 2, "{$pre}letters, {$pre}length"),
+            array(3, '7857375923752947', 2, "{$pre}letters, {$pre}special_chars"),
+            array(3, 'FSDFJSLKFFSDFDSF', 2, "{$pre}lowercase_letters, {$pre}numbers, {$pre}special_chars"),
+            array(3, 'FÜKFJSLKFFSDFDSF', 2, "{$pre}lowercase_letters, {$pre}numbers, {$pre}special_chars"),
+            array(3, 'fjsfjdljfsjsjjlsj', 2, "{$pre}uppercase_letters, {$pre}numbers, {$pre}special_chars"),
+
+            // Medium
+            array(4, 'Foobar﴾', 3, "{$pre}numbers, {$pre}length"),
+            array(4, 'foo-b0r!', 3, "{$pre}uppercase_letters, {$pre}length"),
+            array(4, 'fjsfjdljfsjsjjls1', 3, "{$pre}uppercase_letters, {$pre}special_chars"),
+            array(4, '785737592375294b', 3, "{$pre}uppercase_letters, {$pre}special_chars"),
+        );
+    }
+
     public static function getStrongPasswords()
     {
         return array(
@@ -179,6 +213,27 @@ public function testWeakPasswordsWillNotPass($minStrength, $value, $currentStren
             ->assertRaised();
     }
 
+    /**
+     * @dataProvider getWeakPasswordsUnicode
+     */
+    public function testWeakPasswordsWithUnicodeWillNotPass($minStrength, $value, $currentStrength, $tips = '')
+    {
+        $constraint = new PasswordStrength(array('minStrength' => $minStrength, 'minLength' => 6, 'unicodeEquality' => true));
+
+        $this->validator->validate($value, $constraint);
+
+        $parameters = array(
+            '{{ length }}' => 6,
+            '{{ min_strength }}' => 'rollerworks_password.strength_level.'.self::$levelToLabel[$minStrength],
+            '{{ current_strength }}' => 'rollerworks_password.strength_level.'.self::$levelToLabel[$currentStrength],
+            '{{ strength_tips }}' => $tips,
+        );
+
+        $this->buildViolation('password_too_weak')
+            ->setParameters($parameters)
+            ->assertRaised();
+    }
+
     /**
      * @dataProvider getVeryStrongPasswords
      */