Refactor to remove support for user provided salt.

jeremykendall · Jan 18, 2016 · 3d5541b · 3d5541b
1 parent 50fdd7d
commit 3d5541b
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 21 deletions.
diff --git a/src/JeremyKendall/Password/Decorator/UpgradeDecorator.php b/src/JeremyKendall/Password/Decorator/UpgradeDecorator.php
@@ -48,12 +48,21 @@ 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);
     }
+
+    private function createHashWhichWillForceRehashInValidator($password)
+    {
+        $cost = 4;
+        $options = $this->getOptions();
+
+        if (isset($options['cost']) && (int) $options['cost'] === 4) {
+            $cost = 5;
+        }
+
+        return password_hash($password, PASSWORD_DEFAULT, array('cost' => $cost));
+    }
 }
diff --git a/tests/JeremyKendall/Password/Tests/BlowfishCallbackConstraintTrait.php b/tests/JeremyKendall/Password/Tests/BlowfishCallbackConstraintTrait.php
@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * Password Validator.
+ *
+ * @link      http://github.com/jeremykendall/password-validator Canonical source repo
+ *
+ * @copyright Copyright (c) 2014 Jeremy Kendall (http://about.me/jeremykendall)
+ * @license   http://github.com/jeremykendall/password-validator/blob/master/LICENSE MIT
+ */
+
+namespace JeremyKendall\Password\Tests;
+
+trait BlowfishCallbackConstraintTrait
+{
+    /**
+     * Gets callback constraint to validate blowfish encrypted password.
+     *
+     * @param string $cost Cost used in pattern. Must be 0 padded if less than 10
+     *
+     * @return callable
+     */
+    public function getBlowfishCallback($cost = '10')
+    {
+        $pattern = sprintf('/^\$2y\$%s\$.{53}$/', $cost);
+
+        return function ($subject) use ($pattern) {
+            return preg_match($pattern, $subject) === 1;
+        };
+    }
+}
diff --git a/tests/JeremyKendall/Password/Tests/Decorator/IntegrationTest.php b/tests/JeremyKendall/Password/Tests/Decorator/IntegrationTest.php
@@ -91,4 +91,31 @@ public function testLegacyPasswordIsValidUpgradedRehashedStored2()
         );
         $this->assertStringStartsWith('$2y$10$', $result->getPassword());
     }
+
+    /**
+     * The UpgradeDecorator hashes valid legacy passwords with a cost of 4 in 
+     * order to force a rehash. Without the salt option to password_hash, 
+     * removed for PHP 7.0 compatibility, the password will NOT be rehashed if 
+     * the user provides a cost of 4 to 
+     * PasswordValidatorInterface::setOptions().
+     *
+     * Adding this test before refactoring to avoid regressions.
+     */
+    public function testLegacyPasswordIsUpgradedEvenWhenUserProvidedCostIsFour()
+    {
+        $validator = new UpgradeDecorator(new PasswordValidator(), $this->callback);
+        $validator->setOptions(array('cost' => 4));
+        $password = 'password';
+        $hash = hash('sha512', $password);
+
+        $result = $validator->isValid($password, $hash);
+
+        $this->assertTrue($result->isValid());
+        $this->assertEquals(
+            ValidationResult::SUCCESS_PASSWORD_REHASHED,
+            $result->getCode(),
+            'Failed asserting that Result::getCode() returns expected Result::SUCCESS_PASSWORD_REHASHED.'
+        );
+        $this->assertStringStartsWith('$2y$04', $result->getPassword());
+    }
 }
diff --git a/tests/JeremyKendall/Password/Tests/Decorator/KarptoniteRehashUpgradeDecoratorTest.php b/tests/JeremyKendall/Password/Tests/Decorator/KarptoniteRehashUpgradeDecoratorTest.php
@@ -12,6 +12,7 @@
 
 use JeremyKendall\Password\Decorator\UpgradeDecorator;
 use JeremyKendall\Password\Result as ValidationResult;
+use JeremyKendall\Password\Tests\BlowfishCallbackConstraintTrait;
 
 /**
  * This test validates the upgrade scenario outlined in Daniel Karp's blog post
@@ -24,6 +25,8 @@
  */
 class KarptoniteRehashUpgradeDecoratorTest extends \PHPUnit_Framework_TestCase
 {
+    use BlowfishCallbackConstraintTrait;
+
     private $decorator;
     private $decoratedValidator;
     private $validationCallback;
@@ -67,14 +70,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 +79,11 @@ public function testRehashingPasswordHashesScenarioCredentialIsValid()
 
         $this->decoratedValidator->expects($this->once())
             ->method('isValid')
-            ->with($this->plainTextPassword, $upgradeValidatorRehash, $this->legacySalt)
+            ->with(
+                $this->plainTextPassword,
+                $this->callback($this->getBlowfishCallback('04')),
+                $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
@@ -12,9 +12,12 @@
 
 use JeremyKendall\Password\Decorator\UpgradeDecorator;
 use JeremyKendall\Password\Result as ValidationResult;
+use JeremyKendall\Password\Tests\BlowfishCallbackConstraintTrait;
 
 class UpgradeDecoratorTest extends \PHPUnit_Framework_TestCase
 {
+    use BlowfishCallbackConstraintTrait;
+
     private $decorator;
 
     private $decoratedValidator;
@@ -31,7 +34,7 @@ protected function setUp()
 
             return false;
         };
- 
+
         $this->decoratedValidator = $this->getMockBuilder('JeremyKendall\Password\PasswordValidatorInterface')
             ->disableOriginalConstructor()
             ->getMock();
@@ -46,11 +49,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);
 
         $result = new ValidationResult(
@@ -60,14 +58,14 @@ public function testPasswordValidAndPasswordRehashed()
 
         $this->decoratedValidator->expects($this->once())
             ->method('isValid')
-            ->with($password, $upgradeRehash)
+            ->with($password, $this->callback($this->getBlowfishCallback('04')))
             ->will($this->returnValue($result));
 
         $result = $this->decorator->isValid($password, $passwordHash);
 
         $this->assertTrue($result->isValid());
         $this->assertEquals(
-            ValidationResult::SUCCESS_PASSWORD_REHASHED, 
+            ValidationResult::SUCCESS_PASSWORD_REHASHED,
             $result->getCode()
         );
         // Rehashed password is a valid hash