Skip to content
Permalink
Browse files

Merge branch 'password_reset_security_fix' of https://github.com/chmv…

…/framework into chmv-password_reset_security_fix
  • Loading branch information...
taylorotwell committed Oct 21, 2019
2 parents 998179d + c4c5f4a commit 23041e99833630d93cc7672bd7087eaa350c3a59
@@ -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.
*
@@ -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.
@@ -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
);
}
@@ -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.
*
@@ -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();
@@ -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);
}

0 comments on commit 23041e9

Please sign in to comment.
You can’t perform that action at this time.