security #23507 [Security] validate empty passwords again (xabbuh)

fabpot · fabpot · commit 559ccb2c665c · 2017-07-17T12:54:11.000+02:00
This PR was merged into the 2.7 branch. Discussion ---------- [Security] validate empty passwords again | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23341 (comment) | License | MIT | Doc PR | It looks like this part of #23341 causes serious security issues for some users who rely on the validator to also compare the empty string with their user's password (see for example #23341 (comment)). Thus I suggest to revert this part of #23341. Commits ------- 878198c [Security] validate empty passwords again
diff --git a/src/Symfony/Component/Security/Core/Tests/Validator/Constraints/UserPasswordValidatorTest.php b/src/Symfony/Component/Security/Core/Tests/Validator/Constraints/UserPasswordValidatorTest.php
@@ -90,6 +90,29 @@ public function testPasswordIsNotValid()
             ->assertRaised();
     }
 
+    /**
+     * @dataProvider emptyPasswordData
+     */
+    public function testEmptyPasswordsAreNotValid($password)
+    {
+        $constraint = new UserPassword(array(
+            'message' => 'myMessage',
+        ));
+
+        $this->validator->validate($password, $constraint);
+
+        $this->buildViolation('myMessage')
+            ->assertRaised();
+    }
+
+    public function emptyPasswordData()
+    {
+        return array(
+            array(null),
+            array(''),
+        );
+    }
+
     /**
      * @expectedException \Symfony\Component\Validator\Exception\ConstraintDefinitionException
      */
diff --git a/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php b/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php
@@ -40,6 +40,8 @@ public function validate($password, Constraint $constraint)
         }
 
         if (null === $password || '' === $password) {
+            $this->context->addViolation($constraint->message);
+
             return;
         }
 

Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,8 @@ public function validate($password, Constraint $constraint)`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`if (null === $password \|\| '' === $password) {`
	`43`	`+ $this->context->addViolation($constraint->message);`
	`44`	`+`
`43`	`45`	`return;`
`44`	`46`	`}`
`45`	`47`