Merge pull request #21 from garethellis36/bugfix/php7-warnings

jeremykendall · jeremykendall · commit 1c065e7336cf · 2016-01-26T05:22:43.000-06:00
Bug fix - PHP7 warnings
diff --git a/.travis.yml b/.travis.yml
@@ -5,13 +5,10 @@ php:
   - 5.4
   - 5.5
   - 5.6
+  - 7.0
 
 before_script:
   - composer self-update
   - composer install --prefer-dist
 
-script: phpunit -c travis.phpunit.xml
-
-after_script:
-  - wget https://scrutinizer-ci.com/ocular.phar
-  - php ocular.phar code-coverage:upload --format=php-clover /tmp/jeremykendall/password-validator/coverage.xml
+script: phpunit -c phpunit.xml.dist
diff --git a/src/JeremyKendall/Password/Decorator/UpgradeDecorator.php b/src/JeremyKendall/Password/Decorator/UpgradeDecorator.php
@@ -23,6 +23,8 @@ class UpgradeDecorator extends AbstractDecorator
      */
     protected $validationCallback;
 
+    const DEFAULT_REHASH_COST = 4;
+
     /**
      * Public constructor
      *
@@ -48,12 +50,33 @@ public function isValid($password, $passwordHash, $legacySalt = null, $identity
         );
 
         if ($isValid === true) {
-            $passwordHash = password_hash($password, PASSWORD_DEFAULT, array(
-                'cost' => 4,
-                'salt' => 'CostAndSaltForceRehash',
-            ));
+            $passwordHash = $this->createHashWhichWillForceRehashInValidator($password);
         }
 
         return $this->validator->isValid($password, $passwordHash, $legacySalt, $identity);
     }
+    
+    /**
+     * This method returns an upgraded password, one that is hashed by the
+     * password_hash method in such a way that it forces the PasswordValidator
+     * to rehash the password. This results in PasswordValidator::isValid()
+     * returning a Result::$code of Result::SUCCESS_PASSWORD_REHASHED,
+     * notifying the StorageDecorator or custom application code that the
+     * returned password hash should be persisted.
+     *
+     * @param string $password Password to upgrade
+     *
+     * @return string Hashed password
+     */
+    private function createHashWhichWillForceRehashInValidator($password)
+    {
+        $cost = static::DEFAULT_REHASH_COST;
+        $options = $this->getOptions();
+
+        if (isset($options['cost']) && (int) $options['cost'] === $cost) {
+            $cost++;
+        }
+
+        return password_hash($password, PASSWORD_DEFAULT, array('cost' => $cost));
+    }
 }
diff --git a/tests/JeremyKendall/Password/Tests/Decorator/IntegrationTest.php b/tests/JeremyKendall/Password/Tests/Decorator/IntegrationTest.php
@@ -91,4 +91,22 @@ public function testLegacyPasswordIsValidUpgradedRehashedStored2()
         );
         $this->assertStringStartsWith('$2y$10$', $result->getPassword());
     }
+
+    public function testLegacyPasswordIsValidUpgradedRehashedWhenClientCodeUsesSameParametersAsUpgradeDecorator()
+    {
+        $validator = new UpgradeDecorator(
+            new PasswordValidator(),
+            $this->callback
+        );
+        $password = 'password';
+        $hash = hash('sha512', $password);
+        $validator->setOptions(array('cost' => UpgradeDecorator::DEFAULT_REHASH_COST));
+        $result = $validator->isValid($password, $hash);
+        $this->assertTrue($result->isValid(), "Failed asserting that result is valid");
+        $this->assertEquals(
+            ValidationResult::SUCCESS_PASSWORD_REHASHED,
+            $result->getCode(),
+            "Failed asserting that password was rehashed"
+        );
+    }
 }
diff --git a/tests/JeremyKendall/Password/Tests/Decorator/KarptoniteRehashUpgradeDecoratorTest.php b/tests/JeremyKendall/Password/Tests/Decorator/KarptoniteRehashUpgradeDecoratorTest.php
@@ -67,14 +67,6 @@ protected function setUp()
 
     public function testRehashingPasswordHashesScenarioCredentialIsValid()
     {
-        $upgradeValidatorRehash = password_hash(
-            $this->plainTextPassword,
-            PASSWORD_DEFAULT,
-            array(
-                'cost' => 4,
-                'salt' => 'CostAndSaltForceRehash',
-            )
-        );
         $finalValidatorRehash = password_hash($this->plainTextPassword, PASSWORD_DEFAULT);
 
         $validResult = new ValidationResult(
@@ -84,7 +76,7 @@ public function testRehashingPasswordHashesScenarioCredentialIsValid()
 
         $this->decoratedValidator->expects($this->once())
             ->method('isValid')
-            ->with($this->plainTextPassword, $upgradeValidatorRehash, $this->legacySalt)
+            ->with($this->plainTextPassword, $this->isType('string'), $this->legacySalt)
             ->will($this->returnValue($validResult));
 
         $result = $this->decorator->isValid(
diff --git a/tests/JeremyKendall/Password/Tests/Decorator/UpgradeDecoratorTest.php b/tests/JeremyKendall/Password/Tests/Decorator/UpgradeDecoratorTest.php
@@ -46,10 +46,6 @@ public function testPasswordValidAndPasswordRehashed()
     {
         $password = 'password';
         $passwordHash = hash('sha512', $password);
-        $upgradeRehash = password_hash($password, PASSWORD_DEFAULT, array(
-            'cost' => 4,
-            'salt' => 'CostAndSaltForceRehash',
-        ));
 
         $validatorRehash = password_hash($password, PASSWORD_DEFAULT);
 
@@ -60,7 +56,7 @@ public function testPasswordValidAndPasswordRehashed()
 
         $this->decoratedValidator->expects($this->once())
             ->method('isValid')
-            ->with($password, $upgradeRehash)
+            ->with($password, $this->isType('string'))
             ->will($this->returnValue($result));
 
         $result = $this->decorator->isValid($password, $passwordHash);
