Merge pull request FriendsOfSymfony#2313 from stof/regenerate_hash

Regenerate the hash on password update

Author: XWB

Changelog.md

@@ -25,6 +25,7 @@ Changelog
 * [BC break] The signature of the `UserManager` constructor has changed.
 * [BC break] The translation key `resetting.request.invalid_username` has been removed.
 * [BC break] The propel dependency was dropped.
+* [BC break] The `salt` field of the `User` class is now nullable.
 
 ### 2.0.0-alpha3 (2015-09-15)
 

Model/User.php

@@ -105,7 +105,6 @@ abstract class User implements UserInterface, GroupableInterface
      */
     public function __construct()
     {
-        $this->salt = base_convert(sha1(uniqid(mt_rand(), true)), 16, 36);
         $this->enabled = false;
         $this->roles = array();
     }
@@ -358,6 +357,14 @@ public function setUsernameCanonical($usernameCanonical)
         return $this;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function setSalt($salt)
+    {
+        $this->salt = $salt;
+    }
+
     public function setEmail($email)
     {
         $this->email = $email;

Model/UserInterface.php

@@ -55,6 +55,11 @@ public function getUsernameCanonical();
      */
     public function setUsernameCanonical($usernameCanonical);
 
+    /**
+     * @param string|null $salt
+     */
+    public function setSalt($salt);
+
     /**
      * Gets email.
      *

Resources/config/doctrine-mapping/User.orm.xml

@@ -16,7 +16,7 @@
 
         <field name="enabled" column="enabled" type="boolean" />
 
-        <field name="salt" column="salt" type="string" />
+        <field name="salt" column="salt" type="string" nullable="true" />
 
         <field name="password" column="password" type="string" />
 

Tests/Util/PasswordUpdaterTest.php

@@ -37,15 +37,42 @@ public function testUpdatePassword()
 
         $this->encoderFactory->expects($this->once())
             ->method('getEncoder')
+            ->with($user)
             ->will($this->returnValue($encoder));
 
         $encoder->expects($this->once())
             ->method('encodePassword')
-            ->with('password', $user->getSalt())
+            ->with('password', $this->isType('string'))
             ->will($this->returnValue('encodedPassword'));
 
         $this->updater->hashPassword($user);
         $this->assertSame('encodedPassword', $user->getPassword(), '->updatePassword() sets encoded password');
+        $this->assertNotNull($user->getSalt());
+        $this->assertNull($user->getPlainPassword(), '->updatePassword() erases credentials');
+    }
+
+    public function testUpdatePasswordWithBCrypt()
+    {
+        $encoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $user = new TestUser();
+        $user->setPlainPassword('password');
+        $user->setSalt('old_salt');
+
+        $this->encoderFactory->expects($this->once())
+            ->method('getEncoder')
+            ->with($user)
+            ->will($this->returnValue($encoder));
+
+        $encoder->expects($this->once())
+            ->method('encodePassword')
+            ->with('password', $this->isNull())
+            ->will($this->returnValue('encodedPassword'));
+
+        $this->updater->hashPassword($user);
+        $this->assertSame('encodedPassword', $user->getPassword(), '->updatePassword() sets encoded password');
+        $this->assertNull($user->getSalt());
         $this->assertNull($user->getPlainPassword(), '->updatePassword() erases credentials');
     }
 

Util/PasswordUpdater.php

@@ -12,6 +12,7 @@
 namespace FOS\UserBundle\Util;
 
 use FOS\UserBundle\Model\UserInterface;
+use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 
 /**
@@ -37,6 +38,14 @@ public function hashPassword(UserInterface $user)
         }
 
         $encoder = $this->encoderFactory->getEncoder($user);
+
+        if ($encoder instanceof BCryptPasswordEncoder) {
+            $user->setSalt(null);
+        } else {
+            $salt = rtrim(str_replace('+', '.', base64_encode(random_bytes(32))), '=');
+            $user->setSalt($salt);
+        }
+
         $hashedPassword = $encoder->encodePassword($plainPassword, $user->getSalt());
         $user->setPassword($hashedPassword);
         $user->eraseCredentials();