fix: PwnedValidator cannot reach the HIBP API (#1372)

Author: michalsn

rector.php

@@ -16,9 +16,7 @@
 use Rector\CodeQuality\Rector\Class_\CompleteDynamicPropertiesRector;
 use Rector\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector;
 use Rector\CodeQuality\Rector\Expression\InlineIfToExplicitIfRector;
-use Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector;
 use Rector\CodeQuality\Rector\FuncCall\ChangeArrayPushToArrayAssignRector;
-use Rector\CodeQuality\Rector\FuncCall\SimplifyRegexPatternRector;
 use Rector\CodeQuality\Rector\FuncCall\SimplifyStrposLowerRector;
 use Rector\CodeQuality\Rector\FuncCall\SingleInArrayToCompareRector;
 use Rector\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector;
@@ -31,15 +29,13 @@
 use Rector\CodeQuality\Rector\Ternary\UnnecessaryTernaryExpressionRector;
 use Rector\CodingStyle\Rector\ClassMethod\FuncGetArgsToVariadicParamRector;
 use Rector\CodingStyle\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector;
-use Rector\CodingStyle\Rector\FuncCall\CountArrayToEmptyArrayComparisonRector;
 use Rector\CodingStyle\Rector\FuncCall\VersionCompareFuncCallToConstantRector;
 use Rector\Config\RectorConfig;
 use Rector\DeadCode\Rector\Cast\RecastingRemovalRector;
 use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector;
 use Rector\DeadCode\Rector\If_\UnwrapFutureCompatibleIfPhpVersionRector;
 use Rector\DeadCode\Rector\MethodCall\RemoveNullArgOnNullDefaultParamRector;
 use Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector;
-use Rector\EarlyReturn\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
 use Rector\EarlyReturn\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector;
 use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector;
 use Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector;
@@ -52,7 +48,6 @@
 use Rector\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector;
 use Rector\Set\ValueObject\LevelSetList;
 use Rector\Set\ValueObject\SetList;
-use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector;
 use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
 use Rector\ValueObject\PhpVersion;
 
@@ -61,7 +56,7 @@
         SetList::DEAD_CODE,
         LevelSetList::UP_TO_PHP_81,
         PHPUnitSetList::PHPUNIT_CODE_QUALITY,
-        PHPUnitSetList::PHPUNIT_100,
+        PHPUnitSetList::COMPOSER_BASED,
     ]);
 
     $rectorConfig->parallel();
@@ -148,8 +143,6 @@
 
     $rectorConfig->rule(SimplifyUselessVariableRector::class);
     $rectorConfig->rule(RemoveAlwaysElseRector::class);
-    $rectorConfig->rule(CountArrayToEmptyArrayComparisonRector::class);
-    $rectorConfig->rule(ChangeNestedForeachIfsToEarlyContinueRector::class);
     $rectorConfig->rule(ChangeIfElseValueAssignToEarlyReturnRector::class);
     $rectorConfig->rule(SimplifyStrposLowerRector::class);
     $rectorConfig->rule(CombineIfRector::class);
@@ -158,17 +151,14 @@
     $rectorConfig->rule(PreparedValueToEarlyReturnRector::class);
     $rectorConfig->rule(ShortenElseIfRector::class);
     $rectorConfig->rule(SimplifyIfElseToTernaryRector::class);
-    $rectorConfig->rule(UnusedForeachValueToArrayKeysRector::class);
     $rectorConfig->rule(ChangeArrayPushToArrayAssignRector::class);
     $rectorConfig->rule(UnnecessaryTernaryExpressionRector::class);
-    $rectorConfig->rule(SimplifyRegexPatternRector::class);
     $rectorConfig->rule(FuncGetArgsToVariadicParamRector::class);
     $rectorConfig->rule(MakeInheritedMethodVisibilitySameAsParentRector::class);
     $rectorConfig->rule(SimplifyEmptyArrayCheckRector::class);
     $rectorConfig->rule(SimplifyEmptyCheckOnEmptyArrayRector::class);
     $rectorConfig->rule(TernaryEmptyArrayArrayDimFetchToCoalesceRector::class);
     $rectorConfig->rule(EmptyOnNullableObjectToInstanceOfRector::class);
-    $rectorConfig->rule(DisallowedEmptyRuleFixerRector::class);
     $rectorConfig->rule(StringClassNameToClassConstantRector::class);
     $rectorConfig->rule(PrivatizeFinalClassPropertyRector::class);
     $rectorConfig->rule(CompleteDynamicPropertiesRector::class);

src/Authentication/Actions/Email2FA.php

@@ -94,7 +94,7 @@ public function handle(IncomingRequest $request)
         $email->setSubject(lang('Auth.email2FASubject'));
         $email->setMessage($this->view(
             setting('Auth.views')['action_email_2fa_email'],
-            ['code'  => $identity->secret, 'user' => $user, 'ipAddress' => $ipAddress, 'userAgent' => $userAgent, 'date' => $date],
+            ['code' => $identity->secret, 'user' => $user, 'ipAddress' => $ipAddress, 'userAgent' => $userAgent, 'date' => $date],
             ['debug' => false],
         ));
 

src/Authentication/Actions/EmailActivator.php

@@ -70,7 +70,7 @@ public function show(): string
         $email->setSubject(lang('Auth.emailActivateSubject'));
         $email->setMessage($this->view(
             setting('Auth.views')['action_email_activate_email'],
-            ['code'  => $code, 'user' => $user, 'ipAddress' => $ipAddress, 'userAgent' => $userAgent, 'date' => $date],
+            ['code' => $code, 'user' => $user, 'ipAddress' => $ipAddress, 'userAgent' => $userAgent, 'date' => $date],
             ['debug' => false],
         ));
 

src/Authentication/Passwords/NothingPersonalValidator.php

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

src/Authentication/Passwords/PwnedValidator.php

@@ -32,6 +32,8 @@
  */
 class PwnedValidator extends BaseValidator implements ValidatorInterface
 {
+    private const API_URL = 'https://api.pwnedpasswords.com/range/';
+
     /**
      * Checks the password against the online database and
      * returns false if a match is found. Returns true if no match is found.
@@ -47,12 +49,10 @@ public function check(string $password, ?User $user = null): Result
         $searchHash  = substr($hashedPword, 5);
 
         try {
-            $client = Services::curlrequest([
-                'base_uri' => 'https://api.pwnedpasswords.com/',
-            ]);
+            $client = Services::curlrequest();
 
             $response = $client->get(
-                'range/' . $rangeHash,
+                self::API_URL . $rangeHash,
                 ['headers' => ['Accept' => 'text/plain']],
             );
         } catch (HTTPException $e) {

tests/Unit/PwnedValidatorTest.php

@@ -55,14 +55,20 @@ public function testCheckFalseOnPwnedPassword(): void
         $response = new Response(new App());
         $response->setBody($body);
 
+        $password = 'admin123';
+
         $curlrequest = $this->createMock('CodeIgniter\HTTP\CURLRequest');
 
-        $curlrequest->method('get')->willReturn($response);
+        $curlrequest->expects($this->once())
+            ->method('get')
+            ->with(
+                'https://api.pwnedpasswords.com/range/' . $this->rangeHash($password),
+                ['headers' => ['Accept' => 'text/plain']],
+            )
+            ->willReturn($response);
 
         Services::injectMock('curlrequest', $curlrequest);
 
-        $password = 'admin123';
-
         $result = $this->validator->check($password);
 
         $this->assertFalse($result->isOK());
@@ -75,14 +81,20 @@ public function testCheckFalseOnPwnedLastInRange(): void
         $response = new Response(new App());
         $response->setBody($body);
 
+        $password = 'ziplock';
+
         $curlrequest = $this->createMock('CodeIgniter\HTTP\CURLRequest');
 
-        $curlrequest->method('get')->willReturn($response);
+        $curlrequest->expects($this->once())
+            ->method('get')
+            ->with(
+                'https://api.pwnedpasswords.com/range/' . $this->rangeHash($password),
+                ['headers' => ['Accept' => 'text/plain']],
+            )
+            ->willReturn($response);
 
         Services::injectMock('curlrequest', $curlrequest);
 
-        $password = 'ziplock';
-
         $result = $this->validator->check($password);
 
         $this->assertFalse($result->isOK());
@@ -105,14 +117,20 @@ public function testCheckTrueOnNotFound(): void
         $response = new Response(new App());
         $response->setBody($body);
 
+        $password = '!!!gerard!!!abootylicious';
+
         $curlrequest = $this->createMock('CodeIgniter\HTTP\CURLRequest');
 
-        $curlrequest->method('get')->willReturn($response);
+        $curlrequest->expects($this->once())
+            ->method('get')
+            ->with(
+                'https://api.pwnedpasswords.com/range/' . $this->rangeHash($password),
+                ['headers' => ['Accept' => 'text/plain']],
+            )
+            ->willReturn($response);
 
         Services::injectMock('curlrequest', $curlrequest);
 
-        $password = '!!!gerard!!!abootylicious';
-
         $result = $this->validator->check($password);
 
         $this->assertTrue($result->isOK());
@@ -132,4 +150,13 @@ public function testCheckCatchesAndRethrowsCurlExceptionAsAuthException(): void
         $this->expectException(AuthenticationException::class);
         $this->validator->check('opensesame');
     }
+
+    /**
+     * The first 5 characters of the uppercased SHA-1 hash, as sent
+     * to the HIBP range endpoint.
+     */
+    private function rangeHash(string $password): string
+    {
+        return substr(strtoupper(sha1($password)), 0, 5);
+    }
 }