Skip to content

[FIX] Hashing password-reset tokens before storing #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions src/Auth/Passwords/DoctrineTokenRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,6 +19,13 @@ class DoctrineTokenRepository implements TokenRepositoryInterface
*/
protected $connection;

/**
* The Hasher implementation.
*
* @var HasherContract
*/
protected $hasher;

/**
* The token database table.
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -89,7 +105,7 @@ public function create(CanResetPassword $user)
])
->setParameters([
'email' => $email,
'token' => $token,
'token' => $this->hasher->make($token),
'date' => new Carbon('now')
])
->execute();
Expand Down Expand Up @@ -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();
}
Expand Down
13 changes: 11 additions & 2 deletions src/Auth/Passwords/PasswordBrokerManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace LaravelDoctrine\ORM\Auth\Passwords;

use Illuminate\Support\Str;

class PasswordBrokerManager extends \Illuminate\Auth\Passwords\PasswordBrokerManager
{
/**
Expand All @@ -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
);
Expand Down
26 changes: 19 additions & 7 deletions tests/Auth/Passwords/DoctrineTokenRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -32,6 +33,11 @@ class DoctrineTokenRepositoryTest extends TestCase
*/
protected $connection;

/**
* @var Mock
*/
protected $hasher;

/**
* @var Mock
*/
Expand All @@ -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);

Expand All @@ -52,6 +59,7 @@ protected function setUp(): void

$this->repository = new DoctrineTokenRepository(
$this->connection,
$this->hasher,
'password_resets',
'hashkey',
60
Expand All @@ -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')
Expand Down Expand Up @@ -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('*')
Expand All @@ -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();
Expand Down