Merge pull request laravel-doctrine#508 from rosamarsky/506-fix-password-resets-hashing-issue

[FIX] Hashing password-reset tokens before storing

Author: eigan

src/Auth/Passwords/DoctrineTokenRepository.php

@@ -7,6 +7,7 @@
 use Doctrine\DBAL\Schema\Table;
 use Illuminate\Auth\Passwords\TokenRepositoryInterface;
 use Illuminate\Contracts\Auth\CanResetPassword;
+use Illuminate\Contracts\Hashing\Hasher as HasherContract;
 use Illuminate\Support\Str;
 
 class DoctrineTokenRepository implements TokenRepositoryInterface
@@ -18,6 +19,13 @@ class DoctrineTokenRepository implements TokenRepositoryInterface
      */
     protected $connection;
 
+    /**
+     * The Hasher implementation.
+     *
+     * @var HasherContract
+     */
+    protected $hasher;
+
     /**
      * The token database table.
      *
@@ -49,14 +57,22 @@ class DoctrineTokenRepository implements TokenRepositoryInterface
     /**
      * Create a new token repository instance.
      *
-     * @param Connection $connection
-     * @param string     $table
-     * @param string     $hashKey
-     * @param int        $expires
+     * @param Connection     $connection
+     * @param HasherContract $hasher
+     * @param string         $table
+     * @param string         $hashKey
+     * @param int            $expires
      */
-    public function __construct(Connection $connection, $table, $hashKey, $expires = 60, $throttle = 60)
-    {
+    public function __construct(
+        Connection $connection,
+        HasherContract $hasher,
+        $table,
+        $hashKey,
+        $expires = 60,
+        $throttle = 60
+    ) {
         $this->table      = $table;
+        $this->hasher     = $hasher;
         $this->hashKey    = $hashKey;
         $this->expires    = $expires * 60;
         $this->connection = $connection;
@@ -89,7 +105,7 @@ public function create(CanResetPassword $user)
              ])
              ->setParameters([
                  'email' => $email,
-                 'token' => $token,
+                 'token' => $this->hasher->make($token),
                  'date'  => new Carbon('now')
              ])
              ->execute();
@@ -123,27 +139,28 @@ public function exists(CanResetPassword $user, $token)
     {
         $email = $user->getEmailForPasswordReset();
 
-        $token = $this->getTable()
+        $record = $this->getTable()
                       ->select('*')
                       ->from($this->table)
                       ->where('email = :email')
-                      ->andWhere('token = :token')
                       ->setParameter('email', $email)
-                      ->setParameter('token', $token)
+                      ->setMaxResults(1)
                       ->execute()->fetch();
 
-        return $token && !$this->tokenExpired($token);
+        return $record
+            && !$this->tokenExpired($record['created_at'])
+            && $this->hasher->check($token, $record['token']);
     }
 
     /**
      * Determine if the token has expired.
      *
-     * @param  array $token
+     * @param  string $createdAt
      * @return bool
      */
-    protected function tokenExpired($token)
+    protected function tokenExpired($createdAt)
     {
-        $expiresAt = Carbon::parse($token['created_at'])->addSeconds($this->expires);
+        $expiresAt = Carbon::parse($createdAt)->addSeconds($this->expires);
 
         return $expiresAt->isPast();
     }

src/Auth/Passwords/PasswordBrokerManager.php

@@ -2,6 +2,8 @@
 
 namespace LaravelDoctrine\ORM\Auth\Passwords;
 
+use Illuminate\Support\Str;
+
 class PasswordBrokerManager extends \Illuminate\Auth\Passwords\PasswordBrokerManager
 {
     /**
@@ -13,12 +15,19 @@ class PasswordBrokerManager extends \Illuminate\Auth\Passwords\PasswordBrokerMan
      */
     protected function createTokenRepository(array $config)
     {
-        $connection = isset($config['connection']) ? $config['connection'] : null;
+        $hashKey = $this->app['config']['app.key'];
+
+        if (Str::startsWith($hashKey, 'base64:')) {
+            $hashKey = base64_decode(substr($hashKey, 7));
+        }
+
+        $connection = $config['connection'] ?? null;
 
         return new DoctrineTokenRepository(
             $this->app->make('registry')->getConnection($connection),
+            $this->app->make('hash'),
             $config['table'],
-            $this->app['config']['app.key'],
+            $hashKey,
             $config['expire'],
             $config['throttle'] ?? 60
         );

tests/Auth/Passwords/DoctrineTokenRepositoryTest.php

@@ -5,6 +5,7 @@
 use Doctrine\DBAL\Query\QueryBuilder;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Illuminate\Contracts\Auth\CanResetPassword;
+use Illuminate\Contracts\Hashing\Hasher;
 use LaravelDoctrine\ORM\Auth\Passwords\DoctrineTokenRepository;
 use Mockery as m;
 use Mockery\Mock;
@@ -32,6 +33,11 @@ class DoctrineTokenRepositoryTest extends TestCase
      */
     protected $connection;
 
+    /**
+     * @var Mock
+     */
+    protected $hasher;
+
     /**
      * @var Mock
      */
@@ -40,6 +46,7 @@ class DoctrineTokenRepositoryTest extends TestCase
     protected function setUp(): void
     {
         $this->connection = m::mock(Connection::class);
+        $this->hasher     = m::mock(Hasher::class);
         $this->builder    = m::mock(QueryBuilder::class);
         $this->schema     = m::mock(AbstractSchemaManager::class);
 
@@ -52,6 +59,7 @@ protected function setUp(): void
 
         $this->repository = new DoctrineTokenRepository(
             $this->connection,
+            $this->hasher,
             'password_resets',
             'hashkey',
             60
@@ -64,6 +72,10 @@ public function test_can_create_a_token()
                          ->twice()
                          ->andReturn($this->builder);
 
+        $this->hasher->shouldReceive('make')
+                     ->once()
+                     ->andReturn('token');
+
         $this->builder->shouldReceive('delete')
                       ->once()
                       ->with('password_resets')
@@ -108,6 +120,11 @@ public function test_can_check_if_exists()
                          ->once()
                          ->andReturn($this->builder);
 
+        $this->hasher->shouldReceive('check')
+                     ->once()
+                     ->with('token', 'token')
+                     ->andReturn(true);
+
         $this->builder->shouldReceive('select')
                       ->once()
                       ->with('*')
@@ -123,21 +140,16 @@ public function test_can_check_if_exists()
                       ->with('email = :email')
                       ->andReturnSelf();
 
-        $this->builder->shouldReceive('andWhere')
+        $this->builder->shouldReceive('setMaxResults')
                       ->once()
-                      ->with('token = :token')
+                      ->with(1)
                       ->andReturnSelf();
 
         $this->builder->shouldReceive('setParameter')
                       ->once()
                       ->with('email', '[email protected]')
                       ->andReturnSelf();
 
-        $this->builder->shouldReceive('setParameter')
-                      ->once()
-                      ->with('token', 'token')
-                      ->andReturnSelf();
-
         $this->builder->shouldReceive('execute')
                       ->once()
                       ->andReturnSelf();