feature #30898 [Validator] Wire NotCompromisedPassword in FrameworkBu…

…ndle and handle non UTF-8 password (tgalopin) This PR was squashed before being merged into the 4.3-dev branch (closes #30898). Discussion ---------- [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30870 | License | MIT | Doc PR | - Live from #eu-fossa Fix #30870 Commits ------- 8ac712b [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password
symfony · Apr 6, 2019 · b045aca · b045aca
2 parents ede6660 + 8ac712b
commit b045aca
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 25 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml
@@ -61,6 +61,12 @@
             <tag name="validator.constraint_validator" alias="Symfony\Component\Validator\Constraints\EmailValidator" />
         </service>
 
+        <service id="validator.not_compromised_password" class="Symfony\Component\Validator\Constraints\NotCompromisedPasswordValidator">
+            <argument type="service" id="http_client" on-invalid="null" />
+            <argument>%kernel.charset%</argument>
+            <tag name="validator.constraint_validator" alias="Symfony\Component\Validator\Constraints\NotCompromisedPasswordValidator" />
+        </service>
+
         <service id="validator.property_info_loader" class="Symfony\Component\Validator\Mapping\Loader\PropertyInfoLoader">
             <argument type="service" id="property_info" />
             <argument type="service" id="property_info" />

diff --git a/src/Symfony/Component/Validator/Constraints/NotCompromisedPasswordValidator.php b/src/Symfony/Component/Validator/Constraints/NotCompromisedPasswordValidator.php
@@ -31,14 +31,16 @@ class NotCompromisedPasswordValidator extends ConstraintValidator
     private const RANGE_API = 'https://api.pwnedpasswords.com/range/%s';
 
     private $httpClient;
+    private $charset;
 
-    public function __construct(HttpClientInterface $httpClient = null)
+    public function __construct(HttpClientInterface $httpClient = null, string $charset = 'UTF-8')
     {
         if (null === $httpClient && !class_exists(HttpClient::class)) {
             throw new \LogicException(sprintf('The "%s" class requires the "HttpClient" component. Try running "composer require symfony/http-client".', self::class));
         }
 
         $this->httpClient = $httpClient ?? HttpClient::create();
+        $this->charset = $charset;
     }
 
     /**
@@ -61,6 +63,10 @@ public function validate($value, Constraint $constraint)
             return;
         }
 
+        if ('UTF-8' !== $this->charset) {
+            $value = mb_convert_encoding($value, 'UTF-8', $this->charset);
+        }
+
         $hash = strtoupper(sha1($value));
         $hashPrefix = substr($hash, 0, 5);
         $url = sprintf(self::RANGE_API, $hashPrefix);

diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NotCompromisedPasswordValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NotCompromisedPasswordValidatorTest.php
@@ -28,40 +28,22 @@ class NotCompromisedPasswordValidatorTest extends ConstraintValidatorTestCase
     private const PASSWORD_TRIGGERING_AN_ERROR_RANGE_URL = 'https://api.pwnedpasswords.com/range/3EF27'; // https://api.pwnedpasswords.com/range/3EF27 is the range for the value "apiError"
     private const PASSWORD_LEAKED = 'maman';
     private const PASSWORD_NOT_LEAKED = ']<0585"%sb^5aa$w6!b38",,72?dp3r4\45b28Hy';
+    private const PASSWORD_NON_UTF8_LEAKED = 'мама';
+    private const PASSWORD_NON_UTF8_NOT_LEAKED = 'м<в0dp3r4\45b28Hy';
 
     private const RETURN = [
         '35E033023A46402F94CFB4F654C5BFE44A1:1',
         '35F079CECCC31812288257CD770AA7968D7:53',
-        '36039744C253F9B2A4E90CBEDB02EBFB82D:5', // this is the matching line, password: maman
+        '36039744C253F9B2A4E90CBEDB02EBFB82D:5', // UTF-8 leaked password: maman
+        '273CA8A2A78C9B2D724144F4FAF4D221C86:6', // ISO-8859-5 leaked password: мама
         '3686792BBC66A72D40D928ED15621124CFE:7',
         '36EEC709091B810AA240179A44317ED415C:2',
     ];
 
     protected function createValidator()
     {
-        $httpClientStub = $this->createMock(HttpClientInterface::class);
-        $httpClientStub->method('request')->will(
-            $this->returnCallback(function (string $method, string $url): ResponseInterface {
-                if (self::PASSWORD_TRIGGERING_AN_ERROR_RANGE_URL === $url) {
-                    throw new class('Problem contacting the Have I been Pwned API.') extends \Exception implements ServerExceptionInterface {
-                        public function getResponse(): ResponseInterface
-                        {
-                            throw new \RuntimeException('Not implemented');
-                        }
-                    };
-                }
-
-                $responseStub = $this->createMock(ResponseInterface::class);
-                $responseStub
-                    ->method('getContent')
-                    ->willReturn(implode("\r\n", self::RETURN));
-
-                return $responseStub;
-            })
-        );
-
-        // Pass HttpClient::create() instead of this mock to run the tests against the real API
-        return new NotCompromisedPasswordValidator($httpClientStub);
+        // Pass HttpClient::create() instead of the mock to run the tests against the real API
+        return new NotCompromisedPasswordValidator($this->createHttpClientStub());
     }
 
     public function testNullIsValid()
@@ -112,6 +94,29 @@ public function testValidPassword()
         $this->assertNoViolation();
     }
 
+    public function testNonUtf8CharsetValid()
+    {
+        $validator = new NotCompromisedPasswordValidator($this->createHttpClientStub(), 'ISO-8859-5');
+        $validator->validate(mb_convert_encoding(self::PASSWORD_NON_UTF8_NOT_LEAKED, 'ISO-8859-5', 'UTF-8'), new NotCompromisedPassword());
+
+        $this->assertNoViolation();
+    }
+
+    public function testNonUtf8CharsetInvalid()
+    {
+        $constraint = new NotCompromisedPassword();
+
+        $this->context = $this->createContext();
+
+        $validator = new NotCompromisedPasswordValidator($this->createHttpClientStub(), 'ISO-8859-5');
+        $validator->initialize($this->context);
+        $validator->validate(mb_convert_encoding(self::PASSWORD_NON_UTF8_LEAKED, 'ISO-8859-5', 'UTF-8'), $constraint);
+
+        $this->buildViolation($constraint->message)
+            ->setCode(NotCompromisedPassword::COMPROMISED_PASSWORD_ERROR)
+            ->assertRaised();
+    }
+
     /**
      * @expectedException \Symfony\Component\Validator\Exception\UnexpectedTypeException
      */
@@ -142,4 +147,30 @@ public function testApiErrorSkipped()
         $this->validator->validate(self::PASSWORD_TRIGGERING_AN_ERROR, new NotCompromisedPassword(['skipOnError' => true]));
         $this->assertTrue(true); // No exception have been thrown
     }
+
+    private function createHttpClientStub(): HttpClientInterface
+    {
+        $httpClientStub = $this->createMock(HttpClientInterface::class);
+        $httpClientStub->method('request')->will(
+            $this->returnCallback(function (string $method, string $url): ResponseInterface {
+                if (self::PASSWORD_TRIGGERING_AN_ERROR_RANGE_URL === $url) {
+                    throw new class('Problem contacting the Have I been Pwned API.') extends \Exception implements ServerExceptionInterface {
+                        public function getResponse(): ResponseInterface
+                        {
+                            throw new \RuntimeException('Not implemented');
+                        }
+                    };
+                }
+
+                $responseStub = $this->createMock(ResponseInterface::class);
+                $responseStub
+                    ->method('getContent')
+                    ->willReturn(implode("\r\n", self::RETURN));
+
+                return $responseStub;
+            })
+        );
+
+        return $httpClientStub;
+    }
 }