From 6b4a44bcf66d3f97af4ea3c3f1c51283b0c83384 Mon Sep 17 00:00:00 2001
From: Tang Rufus <tangrufus@gmail.com>
Date: Tue, 19 Feb 2019 11:14:16 +0000
Subject: [PATCH 1/2] Downgrade to PHP 7.0

Because wp.org svn pre-commit hook rejects PHP 7.1 syntax.
---
 .circleci/config.yml                          | 36 ++++++++++++++-----
 composer.json                                 |  8 ++---
 src/Handlers/FormSubmission.php               | 10 +++---
 src/Handlers/WCCheckoutFormSubmission.php     |  2 +-
 src/HaveIBeenPwned/Client.php                 | 34 ++++++++----------
 src/HaveIBeenPwned/ClientInterface.php        |  4 +--
 src/HaveIBeenPwned/ObjectCachedClient.php     | 14 ++++----
 src/Plugin.php                                | 10 +++---
 src/Predicate.php                             |  6 ++--
 src/PredicateInterface.php                    |  4 +--
 tests/unit/Handlers/FormSubmissionTest.php    |  2 +-
 .../Handlers/WCCheckoutFormSubmissionTest.php |  2 +-
 .../WCRegistrationFormSubmissionTest.php      |  2 +-
 tests/unit/HaveIBeenPwned/ClientTest.php      |  4 +--
 .../HaveIBeenPwned/ObjectCachedClientTest.php |  4 +--
 tests/unit/PredicateTest.php                  |  6 ++--
 16 files changed, 82 insertions(+), 66 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 5f3dd75..1f1c675 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -4,14 +4,13 @@ orbs:
   composer: itinerisltd/composer@0
 
   itinerisltd:
+    orbs:
+      composer: itinerisltd/composer@0
+
     executors:
       default:
-        parameters:
-          image:
-            type: string
-            default: 'cibuilds/github:latest'
         docker:
-          - image: << parameters.image >>
+          - image: 'cibuilds/github:latest'
 
     jobs:
       publishing-to-github-releases:
@@ -62,18 +61,26 @@ workflows:
             - composer/exec:
                 command: test
 
+      - composer/install:
+          name: test-7.0
+          executor: composer/seven_zero
+          post-steps:
+            - composer/exec:
+                command: test
+
       - composer/install:
           name: style-check
           post-steps:
             - composer/exec:
                 command: style:check
 
-  build_release:
+  # Build with lowest support PHP version
+  # wp.org svn pre-commit hook rejects PHP 7.1+ syntax
+  build_lint_release:
     jobs:
       - composer/install:
           name: build
-          # Build with lowest support PHP version
-          executor: composer/seven_one
+          executor: composer/seven_zero
           post-steps:
             - composer/exec:
                 command: release:build
@@ -87,10 +94,23 @@ workflows:
             tags:
               only: /.*/
 
+      - composer/install:
+          name: lint
+          executor: composer/seven_zero
+          post-steps:
+            - composer/exec:
+                command: release:build
+            - run: unzip release/disallow-pwned-passwords.zip -d /tmp/plugin-source-code
+            - run: find /tmp/plugin-source-code -name "*.php" -print0 | xargs -n 1 -0 php -l
+          filters:
+            tags:
+              only: /.*/
+
       - itinerisltd/publishing-to-github-releases:
           name: publishing-to-github-releases
           requires:
             - build
+            - lint
           artifact_dir: release
           pre-steps:
             - attach_workspace:
diff --git a/composer.json b/composer.json
index bf2b2c1..f5ecc6c 100644
--- a/composer.json
+++ b/composer.json
@@ -29,12 +29,12 @@
         }
     ],
     "require": {
-        "php": "^7.1",
+        "php": "^7.0",
         "league/container": "^3.2",
-        "typisttech/wp-contained-hook": "^0.3.0"
+        "typisttech/wp-contained-hook": "dev-seven-zero"
     },
     "require-dev": {
-        "10up/wp_mock": "^0.3.0",
+        "10up/wp_mock": "^0.3.0 || ^0.4.0",
         "codeception/base": "^2.4",
         "itinerisltd/itineris-wp-coding-standards": "^0.2.1",
         "mockery/mockery": "^1.1",
@@ -48,7 +48,7 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "0.2.x-dev"
+            "dev-master": "0.3.x-dev"
         }
     },
     "autoload": {
diff --git a/src/Handlers/FormSubmission.php b/src/Handlers/FormSubmission.php
index 32a557c..c043cb8 100644
--- a/src/Handlers/FormSubmission.php
+++ b/src/Handlers/FormSubmission.php
@@ -15,12 +15,12 @@ class FormSubmission extends AbstractFormSubmission
      *
      * @return void
      */
-    public function handle(WP_Error $error): void
+    public function handle(WP_Error $error)
     {
         $errorCode = $error->get_error_code();
         $cleartext = $this->getPasswordCleartextFromSuperglobals();
 
-        if (! empty($errorCode) || null === $cleartext) {
+        if (! empty($errorCode) || empty($cleartext)) {
             return;
         }
 
@@ -30,12 +30,12 @@ public function handle(WP_Error $error): void
     /**
      * Make password instance from superglobals.
      *
-     * @return string|null returns null if password not found.
+     * @return string returns empty string if password not found.
      */
-    protected function getPasswordCleartextFromSuperglobals(): ?string
+    protected function getPasswordCleartextFromSuperglobals(): string
     {
         if (empty($_POST['pass1']) && empty($_POST['password_1'])) { // WPCS: input var, CSRF ok.
-            return null;
+            return '';
         }
 
         return wp_unslash($_POST['pass1'] ?? $_POST['password_1']); // WPCS: input var, CSRF ok.
diff --git a/src/Handlers/WCCheckoutFormSubmission.php b/src/Handlers/WCCheckoutFormSubmission.php
index c33184d..787577d 100644
--- a/src/Handlers/WCCheckoutFormSubmission.php
+++ b/src/Handlers/WCCheckoutFormSubmission.php
@@ -16,7 +16,7 @@ class WCCheckoutFormSubmission extends AbstractFormSubmission
      *
      * @return void
      */
-    public function handle(array $data, WP_Error $error): void
+    public function handle(array $data, WP_Error $error)
     {
         $errorCode = $error->get_error_code();
         $cleartext = $data['account_password'] ?? '';
diff --git a/src/HaveIBeenPwned/Client.php b/src/HaveIBeenPwned/Client.php
index 2f3b581..216ed50 100644
--- a/src/HaveIBeenPwned/Client.php
+++ b/src/HaveIBeenPwned/Client.php
@@ -13,14 +13,14 @@ class Client implements ClientInterface
      *
      * @param Password $password The password to be checked.
      *
-     * @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
+     * @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
      */
-    public function getPwnedTimes(Password $password): ?int
+    public function getPwnedTimes(Password $password): int
     {
         $pwned = $this->fetchAndDecode($password);
 
-        if (null === $pwned) {
-            return null;
+        if (empty($pwned)) {
+            return -1;
         }
 
         return $pwned[$password->getHashSuffix()] ?? 0;
@@ -37,13 +37,13 @@ public function getPwnedTimes(Password $password): ?int
      *
      * @param Password $password The password to be checked.
      *
-     * @return array|null
+     * @return array
      */
-    public function fetchAndDecode(Password $password): ?array
+    public function fetchAndDecode(Password $password): array
     {
         $responseBody = $this->fetch($password);
-        if (null === $responseBody) {
-            return null;
+        if (empty($responseBody)) {
+            return [];
         }
 
         return $this->decode($responseBody);
@@ -54,16 +54,16 @@ public function fetchAndDecode(Password $password): ?array
      *
      * @param Password $password The password to be checked.
      *
-     * @return string|null Body of the response if successful. Otherwise, null.
+     * @return string Body of the response if successful. Otherwise, empty string.
      */
-    protected function fetch(Password $password): ?string
+    protected function fetch(Password $password): string
     {
         $url = static::ENDPOINT . $password->getHashPrefix();
         $response = wp_remote_get($url);
 
         $responseCode = wp_remote_retrieve_response_code($response);
         if (200 !== $responseCode) {
-            return null;
+            return '';
         }
 
         return (string) $response['body'];
@@ -89,15 +89,11 @@ protected function decode(string $responseBody): array
 
         $pwned = [];
         foreach ($suffixesWithPwnedTimes as $suffixWithPwnedTimes) {
-            [
-                // phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
-                0 => $suffix,
-                // phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
-                1 => $pwnedTimes,
-            ] = explode(':', trim($suffixWithPwnedTimes));
+            $exploded = explode(':', trim($suffixWithPwnedTimes));
+            $suffix = (string) $exploded[0];
+            $pwnedTimes = (int) $exploded[1];
 
-            // phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
-            $pwned[(string) $suffix] = (int) $pwnedTimes;
+            $pwned[$suffix] = $pwnedTimes;
         }
 
         return $pwned;
diff --git a/src/HaveIBeenPwned/ClientInterface.php b/src/HaveIBeenPwned/ClientInterface.php
index 193978a..457f01b 100644
--- a/src/HaveIBeenPwned/ClientInterface.php
+++ b/src/HaveIBeenPwned/ClientInterface.php
@@ -10,7 +10,7 @@ interface ClientInterface
      *
      * @param Password $password The password to be checked.
      *
-     * @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
+     * @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
      */
-    public function getPwnedTimes(Password $password): ?int;
+    public function getPwnedTimes(Password $password): int;
 }
diff --git a/src/HaveIBeenPwned/ObjectCachedClient.php b/src/HaveIBeenPwned/ObjectCachedClient.php
index 02f8831..4350a6d 100644
--- a/src/HaveIBeenPwned/ObjectCachedClient.php
+++ b/src/HaveIBeenPwned/ObjectCachedClient.php
@@ -33,14 +33,14 @@ public function __construct(Client $client)
      *
      * @param Password $password The password to be checked.
      *
-     * @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
+     * @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
      */
-    public function getPwnedTimes(Password $password): ?int
+    public function getPwnedTimes(Password $password): int
     {
         $pwned = $this->fetchAndDecode($password);
 
-        if (null === $pwned) {
-            return null;
+        if (empty($pwned)) {
+            return -1;
         }
 
         return $pwned[$password->getHashSuffix()] ?? 0;
@@ -59,9 +59,9 @@ public function getPwnedTimes(Password $password): ?int
      *
      * @param Password $password The password to be checked.
      *
-     * @return array|null
+     * @return array
      */
-    protected function fetchAndDecode(Password $password): ?array
+    protected function fetchAndDecode(Password $password): array
     {
         $result = wp_cache_get(
             $password->getHashPrefix(),
@@ -73,7 +73,7 @@ protected function fetchAndDecode(Password $password): ?array
         }
 
         $result = $this->client->fetchAndDecode($password);
-        if (null !== $result) {
+        if (! empty($result)) {
             // phpcs:ignore WordPressVIPMinimum.Cache.LowExpiryCacheTime.LowCacheTime -- Because of phpcs bug.
             wp_cache_set(
                 $password->getHashPrefix(),
diff --git a/src/Plugin.php b/src/Plugin.php
index dcead74..a6747f6 100644
--- a/src/Plugin.php
+++ b/src/Plugin.php
@@ -37,17 +37,17 @@ public function __construct()
      *
      * @return void
      */
-    public function run(): void
+    public function run()
     {
         $this->setUpContainer();
         $this->setUpLoader();
 
-        add_action('plugins_loaded', function (): void {
+        add_action('plugins_loaded', function () {
             // phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
             do_action(static::PREFIX . '_register', $this->container);
         }, PHP_INT_MAX - 1000);
 
-        add_action('plugins_loaded', function (): void {
+        add_action('plugins_loaded', function () {
             // phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
             do_action(static::PREFIX . '_boot', $this->container);
         }, PHP_INT_MIN + 1000);
@@ -58,7 +58,7 @@ public function run(): void
      *
      * @return void
      */
-    protected function setUpContainer(): void
+    protected function setUpContainer()
     {
         // Register the reflection container as a delegate to enable auto wiring.
         $this->container->delegate(
@@ -76,7 +76,7 @@ protected function setUpContainer(): void
      *
      * @return void
      */
-    protected function setUpLoader(): void
+    protected function setUpLoader()
     {
         $this->loader->add(
             new Action('user_profile_update_errors', FormSubmission::class, 'handle'),
diff --git a/src/Predicate.php b/src/Predicate.php
index f673c9f..8ee0428 100644
--- a/src/Predicate.php
+++ b/src/Predicate.php
@@ -9,12 +9,12 @@ class Predicate implements PredicateInterface
     /**
      * Whether the password should be allowed according to the number of times it has been pwned.
      *
-     * @param int|null $pwnedTimes Number of times the password has been pwned.
+     * @param int $pwnedTimes Number of times the password has been pwned.
      *
      * @return bool
      */
-    public function shouldDisallow(?int $pwnedTimes): bool
+    public function shouldDisallow(int $pwnedTimes): bool
     {
-        return null !== $pwnedTimes && $pwnedTimes > 0;
+        return $pwnedTimes > 0;
     }
 }
diff --git a/src/PredicateInterface.php b/src/PredicateInterface.php
index 32c3b92..0a3a323 100644
--- a/src/PredicateInterface.php
+++ b/src/PredicateInterface.php
@@ -8,9 +8,9 @@ interface PredicateInterface
     /**
      * Whether the password should be allowed according to the number of times it has been pwned.
      *
-     * @param int|null $pwnedTimes Number of times the password has been pwned.
+     * @param int $pwnedTimes Number of times the password has been pwned.
      *
      * @return bool
      */
-    public function shouldDisallow(?int $pwnedTimes): bool;
+    public function shouldDisallow(int $pwnedTimes): bool;
 }
diff --git a/tests/unit/Handlers/FormSubmissionTest.php b/tests/unit/Handlers/FormSubmissionTest.php
index 3b9d836..86707e0 100644
--- a/tests/unit/Handlers/FormSubmissionTest.php
+++ b/tests/unit/Handlers/FormSubmissionTest.php
@@ -178,7 +178,7 @@ public function testHandleUnreachable()
                    return $password->getHashPrefix() === '486B5' &&
                           $password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
                }))
-               ->andReturnNull()
+               ->andReturn(-1)
                ->once();
 
         $formSubmission = new FormSubmission(
diff --git a/tests/unit/Handlers/WCCheckoutFormSubmissionTest.php b/tests/unit/Handlers/WCCheckoutFormSubmissionTest.php
index afb0f3e..d09de14 100644
--- a/tests/unit/Handlers/WCCheckoutFormSubmissionTest.php
+++ b/tests/unit/Handlers/WCCheckoutFormSubmissionTest.php
@@ -165,7 +165,7 @@ public function testHandleUnreachable()
                    return $password->getHashPrefix() === '486B5' &&
                           $password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
                }))
-               ->andReturnNull()
+               ->andReturn(-1)
                ->once();
 
         $formSubmission = new WCCheckoutFormSubmission(
diff --git a/tests/unit/Handlers/WCRegistrationFormSubmissionTest.php b/tests/unit/Handlers/WCRegistrationFormSubmissionTest.php
index 284ac21..7f7e6ee 100644
--- a/tests/unit/Handlers/WCRegistrationFormSubmissionTest.php
+++ b/tests/unit/Handlers/WCRegistrationFormSubmissionTest.php
@@ -159,7 +159,7 @@ public function testHandleUnreachable()
                    return $password->getHashPrefix() === '486B5' &&
                           $password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
                }))
-               ->andReturnNull()
+               ->andReturn(-1)
                ->once();
 
         $formSubmission = new WCRegistrationFormSubmission(
diff --git a/tests/unit/HaveIBeenPwned/ClientTest.php b/tests/unit/HaveIBeenPwned/ClientTest.php
index 9e96cae..e3da966 100644
--- a/tests/unit/HaveIBeenPwned/ClientTest.php
+++ b/tests/unit/HaveIBeenPwned/ClientTest.php
@@ -68,7 +68,7 @@ public function testGetPwnedTimesUnreachable()
 
         $actual = $client->getPwnedTimes($password);
 
-        $this->assertNull($actual);
+        $this->assertSame(-1, $actual);
     }
 
     public function testFetchAndDecode()
@@ -120,6 +120,6 @@ public function testFetchAndDecodeUnreachable()
 
         $actual = $client->fetchAndDecode($password);
 
-        $this->assertNull($actual);
+        $this->assertEmpty($actual);
     }
 }
diff --git a/tests/unit/HaveIBeenPwned/ObjectCachedClientTest.php b/tests/unit/HaveIBeenPwned/ObjectCachedClientTest.php
index 02d9951..1c0fd3b 100644
--- a/tests/unit/HaveIBeenPwned/ObjectCachedClientTest.php
+++ b/tests/unit/HaveIBeenPwned/ObjectCachedClientTest.php
@@ -95,13 +95,13 @@ public function testGetPwnedTimesCacheMissAndApiUnreachable()
         $originalClient = Mockery::mock(Client::class);
         $originalClient->expects('fetchAndDecode')
                        ->with($password)
-                       ->andReturnNull()
+                       ->andReturn([])
                        ->once();
 
         $client = new ObjectCachedClient($originalClient);
 
         $actual = $client->getPwnedTimes($password);
 
-        $this->assertNull($actual);
+        $this->assertSame(-1, $actual);
     }
 }
diff --git a/tests/unit/PredicateTest.php b/tests/unit/PredicateTest.php
index 0c482dc..a69ceed 100644
--- a/tests/unit/PredicateTest.php
+++ b/tests/unit/PredicateTest.php
@@ -31,7 +31,7 @@ public function testShouldDisallow()
         $this->assertTrue($actual);
     }
 
-    public function testShouldDisallowZero()
+    public function testShouldAllowZero()
     {
         $predicate = new Predicate();
 
@@ -40,11 +40,11 @@ public function testShouldDisallowZero()
         $this->assertFalse($actual);
     }
 
-    public function testShouldDisallowNull()
+    public function testShouldAllowMinusOne()
     {
         $predicate = new Predicate();
 
-        $actual = $predicate->shouldDisallow(null);
+        $actual = $predicate->shouldDisallow(-1);
 
         $this->assertFalse($actual);
     }

From 58adb61e8666b85bbcbe2ae8ef652a2019cb2d00 Mon Sep 17 00:00:00 2001
From: Tang Rufus <tangrufus@gmail.com>
Date: Tue, 19 Feb 2019 14:19:19 +0000
Subject: [PATCH 2/2] Enforce return type hint

IssueId: 35443450  Message: The expression ``return wp_unslash($_POST['pass1'] ?? $_POST['password_1'])`` could return the type ``array`` which is incompatible with the type-hinted return ``string``. Consider adding an additional type-check to rule them out.  Filename: src/Handlers/FormSubmission.php  LineNumber: 41  Link: https://scrutinizer-ci.com/g/ItinerisLtd/disallow-pwned-passwords/inspections/70057df8-67e6-4069-97df-3c98bc02845a/issues/files/src/Handlers/FormSubmission.php?status=new&orderField=path&order=asc&honorSelectedPaths=0&issueId=35443450
---
 src/Handlers/FormSubmission.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Handlers/FormSubmission.php b/src/Handlers/FormSubmission.php
index c043cb8..9b3036a 100644
--- a/src/Handlers/FormSubmission.php
+++ b/src/Handlers/FormSubmission.php
@@ -38,6 +38,6 @@ protected function getPasswordCleartextFromSuperglobals(): string
             return '';
         }
 
-        return wp_unslash($_POST['pass1'] ?? $_POST['password_1']); // WPCS: input var, CSRF ok.
+        return (string) wp_unslash($_POST['pass1'] ?? $_POST['password_1']); // WPCS: input var, CSRF ok.
     }
 }