Merge branch 'password_reset_security_fix' of https://github.com/chmv/framework into chmv-password_reset_security_fix

Author: taylorotwell

src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php

@@ -45,6 +45,13 @@ class DatabaseTokenRepository implements TokenRepositoryInterface
      */
     protected $expires;
 
+    /**
+     * Minimum number of seconds before re-redefining the token.
+     *
+     * @var int
+     */
+    protected $timeout;
+
     /**
      * Create a new token repository instance.
      *
@@ -53,15 +60,17 @@ class DatabaseTokenRepository implements TokenRepositoryInterface
      * @param  string  $table
      * @param  string  $hashKey
      * @param  int  $expires
+     * @param  int $timeout
      * @return void
      */
     public function __construct(ConnectionInterface $connection, HasherContract $hasher,
-                                $table, $hashKey, $expires = 60)
+                                $table, $hashKey, $expires = 60, $timeout = 60)
     {
         $this->table = $table;
         $this->hasher = $hasher;
         $this->hashKey = $hashKey;
         $this->expires = $expires * 60;
+        $this->timeout = $timeout;
         $this->connection = $connection;
     }
 
@@ -139,6 +148,32 @@ protected function tokenExpired($createdAt)
         return Carbon::parse($createdAt)->addSeconds($this->expires)->isPast();
     }
 
+    /**
+     * Determine if a token record exists and was recently created.
+     *
+     * @param \Illuminate\Contracts\Auth\CanResetPassword $user
+     * @return bool
+     */
+    public function recentlyCreated(CanResetPasswordContract $user)
+    {
+        $record = (array) $this->getTable()->where(
+            'email', $user->getEmailForPasswordReset()
+        )->first();
+
+        return $record && $this->tokenRecentlyCreated($record['created_at']);
+    }
+
+    /**
+     * Determine if the token was recently created.
+     *
+     * @param string $createdAt
+     * @return bool
+     */
+    protected function tokenRecentlyCreated($createdAt)
+    {
+        return Carbon::parse($createdAt)->addSeconds($this->timeout)->isFuture();
+    }
+
     /**
      * Delete a token record by user.
      *

src/Illuminate/Auth/Passwords/PasswordBroker.php

@@ -55,6 +55,16 @@ public function sendResetLink(array $credentials)
             return static::INVALID_USER;
         }
 
+        // Before 7.x we have to check the existence of a new method.
+        // In 7.x, this code must be removed.
+        if (method_exists($this->tokens, 'recentlyCreated')) {
+            // An attacker can make a lot of password reset requests,
+            // which will lead to spam in user's mailbox.
+            if ($this->tokens->recentlyCreated($user)) {
+                return static::RESEND_TIMEOUT;
+            }
+        }
+
         // Once we have the reset token, we are ready to send the message out to this
         // user with a link to reset their password. We will then redirect back to
         // the current URI having nothing set in the session to indicate errors.

src/Illuminate/Auth/Passwords/PasswordBrokerManager.php

@@ -95,7 +95,10 @@ protected function createTokenRepository(array $config)
             $this->app['hash'],
             $config['table'],
             $key,
-            $config['expire']
+            $config['expire'],
+            // Before 7.x this element in the configuration may not exist.
+            // In 7.x, this check must be removed.
+            $config['timeout'] ?? 0
         );
     }
 

src/Illuminate/Contracts/Auth/PasswordBroker.php

@@ -34,6 +34,13 @@ interface PasswordBroker
      */
     const INVALID_TOKEN = 'passwords.token';
 
+    /**
+     * Constant representing the wait before password reset link resending.
+     *
+     * @var string
+     */
+    const RESEND_TIMEOUT = 'passwords.timeout';
+
     /**
      * Send a password reset link to a user.
      *

tests/Auth/AuthDatabaseTokenRepositoryTest.php

@@ -98,6 +98,44 @@ public function testExistReturnsFalseIfInvalidToken()
         $this->assertFalse($repo->exists($user, 'wrong-token'));
     }
 
+    public function testRecentlyCreatedReturnsFalseIfNoRowFoundForUser()
+    {
+        $repo = $this->getRepo();
+        $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock(stdClass::class));
+        $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
+        $query->shouldReceive('first')->once()->andReturn(null);
+        $user = m::mock(CanResetPassword::class);
+        $user->shouldReceive('getEmailForPasswordReset')->once()->andReturn('email');
+
+        $this->assertFalse($repo->recentlyCreated($user));
+    }
+
+    public function testRecentlyCreatedReturnsTrueIfRecordIsRecentlyCreated()
+    {
+        $repo = $this->getRepo();
+        $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock(stdClass::class));
+        $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
+        $date = Carbon::now()->subSeconds(59)->toDateTimeString();
+        $query->shouldReceive('first')->once()->andReturn((object) ['created_at' => $date, 'token' => 'hashed-token']);
+        $user = m::mock(CanResetPassword::class);
+        $user->shouldReceive('getEmailForPasswordReset')->once()->andReturn('email');
+
+        $this->assertTrue($repo->recentlyCreated($user));
+    }
+
+    public function testRecentlyCreatedReturnsFalseIfValidRecordExists()
+    {
+        $repo = $this->getRepo();
+        $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock(stdClass::class));
+        $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
+        $date = Carbon::now()->subSeconds(61)->toDateTimeString();
+        $query->shouldReceive('first')->once()->andReturn((object) ['created_at' => $date, 'token' => 'hashed-token']);
+        $user = m::mock(CanResetPassword::class);
+        $user->shouldReceive('getEmailForPasswordReset')->once()->andReturn('email');
+
+        $this->assertFalse($repo->recentlyCreated($user));
+    }
+
     public function testDeleteMethodDeletesByToken()
     {
         $repo = $this->getRepo();

tests/Auth/AuthPasswordBrokerTest.php

@@ -29,6 +29,18 @@ public function testIfUserIsNotFoundErrorRedirectIsReturned()
         $this->assertEquals(PasswordBrokerContract::INVALID_USER, $broker->sendResetLink(['credentials']));
     }
 
+    public function testIfTokenIsRecentlyCreated()
+    {
+        $mocks = $this->getMocks();
+        $mocks['tokens'] = m::mock(TestTokenRepositoryInterface::class);
+        $broker = $this->getMockBuilder(PasswordBroker::class)->setMethods(['emailResetLink', 'getUri'])->setConstructorArgs(array_values($mocks))->getMock();
+        $mocks['users']->shouldReceive('retrieveByCredentials')->once()->with(['foo'])->andReturn($user = m::mock(CanResetPassword::class));
+        $mocks['tokens']->shouldReceive('recentlyCreated')->once()->with($user)->andReturn(true);
+        $user->shouldReceive('sendPasswordResetNotification')->with('token');
+
+        $this->assertEquals(PasswordBrokerContract::RESEND_TIMEOUT, $broker->sendResetLink(['foo']));
+    }
+
     public function testGetUserThrowsExceptionIfUserDoesntImplementCanResetPassword()
     {
         $this->expectException(UnexpectedValueException::class);
@@ -54,12 +66,9 @@ public function testBrokerCreatesTokenAndRedirectsWithoutError()
         $broker = $this->getMockBuilder(PasswordBroker::class)->setMethods(['emailResetLink', 'getUri'])->setConstructorArgs(array_values($mocks))->getMock();
         $mocks['users']->shouldReceive('retrieveByCredentials')->once()->with(['foo'])->andReturn($user = m::mock(CanResetPassword::class));
         $mocks['tokens']->shouldReceive('create')->once()->with($user)->andReturn('token');
-        $callback = function () {
-            //
-        };
         $user->shouldReceive('sendPasswordResetNotification')->with('token');
 
-        $this->assertEquals(PasswordBrokerContract::RESET_LINK_SENT, $broker->sendResetLink(['foo'], $callback));
+        $this->assertEquals(PasswordBrokerContract::RESET_LINK_SENT, $broker->sendResetLink(['foo']));
     }
 
     public function testRedirectIsReturnedByResetWhenUserCredentialsInvalid()
@@ -115,3 +124,11 @@ protected function getMocks()
         ];
     }
 }
+
+// Before 7.x we have to check the existence of a new method. In 7.x, this code must be moved to
+// Illuminate\Auth\Passwords\TokenRepositoryInterface
+
+interface TestTokenRepositoryInterface extends TokenRepositoryInterface
+{
+    public function recentlyCreated(CanResetPassword $user);
+}