Skip to content
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

[11.x] Rehash user passwords when validating credentials #48665

Merged
merged 13 commits into from
Dec 10, 2023
1 change: 1 addition & 0 deletions src/Illuminate/Auth/AuthManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public function createSessionDriver($name, $config)
$name,
$provider,
$this->app['session.store'],
rehashOnLogin: $this->app['config']->get('hashing.rehash_on_login', true),
);

// When using the remember me functionality of the authentication services we
Expand Down
19 changes: 18 additions & 1 deletion src/Illuminate/Auth/Authenticatable.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@

trait Authenticatable
{
/**
* The column name of the password field using during authentication.
*
* @var string
*/
protected $authPasswordName = 'password';

/**
* The column name of the "remember me" token.
*
Expand Down Expand Up @@ -41,14 +48,24 @@ public function getAuthIdentifierForBroadcasting()
return $this->getAuthIdentifier();
}

/**
* Get the name of the password attribute for the user.
*
* @return string
*/
public function getAuthPasswordName()
{
return $this->authPasswordName;
}

/**
* Get the password for the user.
*
* @return string
*/
public function getAuthPassword()
{
return $this->password;
return $this->{$this->getAuthPasswordName()};
}

/**
Expand Down
18 changes: 18 additions & 0 deletions src/Illuminate/Auth/DatabaseUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,22 @@ public function validateCredentials(UserContract $user, array $credentials)
$credentials['password'], $user->getAuthPassword()
);
}

/**
* Rehash the user's password if required and supported.
*
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $credentials
* @return void
*/
public function rehashPasswordIfRequired(UserContract $user, array $credentials)
{
if (! $this->hasher->needsRehash($user->getAuthPassword())) {
return;
}

$this->connection->table($this->table)
->where($user->getAuthIdentifierName(), $user->getAuthIdentifier())
->update([$user->getAuthPasswordName() => $this->hasher->make($credentials['password'])]);
}
}
18 changes: 18 additions & 0 deletions src/Illuminate/Auth/EloquentUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,24 @@ public function validateCredentials(UserContract $user, array $credentials)
return $this->hasher->check($plain, $user->getAuthPassword());
}

/**
* Rehash the user's password if required and supported.
*
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $credentials
* @return void
*/
public function rehashPasswordIfRequired(UserContract $user, array $credentials)
{
if (! $this->hasher->needsRehash($user->getAuthPassword())) {
return;
}

$user->forceFill([
$user->getAuthPasswordName() => $this->hasher->make($credentials['password']),
])->save();
}

/**
* Get a new query builder for the model instance.
*
Expand Down
10 changes: 10 additions & 0 deletions src/Illuminate/Auth/GenericUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ public function getAuthIdentifier()
return $this->attributes[$this->getAuthIdentifierName()];
}

/**
* Get the name of the password attribute for the user.
*
* @return string
*/
public function getAuthPasswordName()
{
return 'password';
}

/**
* Get the password for the user.
*
Expand Down
30 changes: 29 additions & 1 deletion src/Illuminate/Auth/SessionGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth
*/
protected $timebox;

/**
* Indicates if paswords should be rehashed on login if needed.
valorin marked this conversation as resolved.
Show resolved Hide resolved
*
* @var bool
*/
protected $rehashOnLogin;

/**
* Indicates if the logout method has been called.
*
Expand All @@ -118,19 +125,22 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth
* @param \Illuminate\Contracts\Session\Session $session
* @param \Symfony\Component\HttpFoundation\Request|null $request
* @param \Illuminate\Support\Timebox|null $timebox
* @param bool $rehashOnLogin
* @return void
*/
public function __construct($name,
UserProvider $provider,
Session $session,
Request $request = null,
Timebox $timebox = null)
Timebox $timebox = null,
bool $rehashOnLogin = true)
{
$this->name = $name;
$this->session = $session;
$this->request = $request;
$this->provider = $provider;
$this->timebox = $timebox ?: new Timebox;
$this->rehashOnLogin = $rehashOnLogin;
}

/**
Expand Down Expand Up @@ -384,6 +394,8 @@ public function attempt(array $credentials = [], $remember = false)
// to validate the user against the given credentials, and if they are in
// fact valid we'll log the users into the application and return true.
if ($this->hasValidCredentials($user, $credentials)) {
$this->rehashPasswordIfRequired($user, $credentials);

$this->login($user, $remember);

return true;
Expand Down Expand Up @@ -415,6 +427,8 @@ public function attemptWhen(array $credentials = [], $callbacks = null, $remembe
// the user is retrieved and validated. If one of the callbacks returns falsy we do
// not login the user. Instead, we will fail the specific authentication attempt.
if ($this->hasValidCredentials($user, $credentials) && $this->shouldLogin($callbacks, $user)) {
$this->rehashPasswordIfRequired($user, $credentials);

$this->login($user, $remember);

return true;
Expand Down Expand Up @@ -447,6 +461,20 @@ protected function hasValidCredentials($user, $credentials)
}, 200 * 1000);
}

/**
* Rehash the user's password if enabled and required.
*
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $credentials
* @return void
*/
protected function rehashPasswordIfRequired(AuthenticatableContract $user, array $credentials)
{
if ($this->rehashOnLogin) {
$this->provider->rehashPasswordIfRequired($user, $credentials);
}
}

/**
* Determine if the user should login by executing the given callbacks.
*
Expand Down
7 changes: 7 additions & 0 deletions src/Illuminate/Contracts/Auth/Authenticatable.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ public function getAuthIdentifierName();
*/
public function getAuthIdentifier();

/**
* Get the name of the password attribute for the user.
*
* @return string
*/
public function getAuthPasswordName();

/**
* Get the password for the user.
*
Expand Down
9 changes: 9 additions & 0 deletions src/Illuminate/Contracts/Auth/UserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,13 @@ public function retrieveByCredentials(array $credentials);
* @return bool
*/
public function validateCredentials(Authenticatable $user, array $credentials);

/**
* Rehash the user's password if required and supported.
*
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $credentials
* @return void
*/
public function rehashPasswordIfRequired(Authenticatable $user, array $credentials);
}
4 changes: 2 additions & 2 deletions src/Illuminate/Session/Middleware/AuthenticateSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function handle($request, Closure $next)
if ($this->guard()->viaRemember()) {
$passwordHash = explode('|', $request->cookies->get($this->guard()->getRecallerName()))[2] ?? null;

if (! $passwordHash || $passwordHash != $request->user()->getAuthPassword()) {
if (! $passwordHash || ! hash_equals($request->user()->getAuthPassword(), $passwordHash)) {
$this->logout($request);
}
}
Expand All @@ -53,7 +53,7 @@ public function handle($request, Closure $next)
$this->storePasswordHashInSession($request);
}

if ($request->session()->get('password_hash_'.$this->auth->getDefaultDriver()) !== $request->user()->getAuthPassword()) {
if (! hash_equals($request->session()->get('password_hash_'.$this->auth->getDefaultDriver()), $request->user()->getAuthPassword())) {
$this->logout($request);
}

Expand Down
58 changes: 58 additions & 0 deletions tests/Auth/AuthDatabaseUserProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Contracts\Hashing\Hasher;
use Illuminate\Database\Connection;
use Illuminate\Database\ConnectionInterface;
use Mockery as m;
use PHPUnit\Framework\TestCase;
use stdClass;
Expand Down Expand Up @@ -160,4 +161,61 @@ public function testCredentialValidation()

$this->assertTrue($result);
}

public function testCredentialValidationFailed()
{
$conn = m::mock(Connection::class);
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(false);
$provider = new DatabaseUserProvider($conn, $hasher, 'foo');
$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$result = $provider->validateCredentials($user, ['password' => 'plain']);

$this->assertFalse($result);
}

public function testRehashPasswordIfRequired()
{
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(true);
$hasher->shouldReceive('make')->once()->with('plain')->andReturn('rehashed');

$conn = m::mock(Connection::class);
$table = m::mock(ConnectionInterface::class);
$conn->shouldReceive('table')->once()->with('foo')->andReturn($table);
$table->shouldReceive('where')->once()->with('id', 1)->andReturnSelf();
$table->shouldReceive('update')->once()->with(['password_attribute' => 'rehashed']);

$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthIdentifierName')->once()->andReturn('id');
$user->shouldReceive('getAuthIdentifier')->once()->andReturn(1);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$user->shouldReceive('getAuthPasswordName')->once()->andReturn('password_attribute');

$provider = new DatabaseUserProvider($conn, $hasher, 'foo');
$provider->rehashPasswordIfRequired($user, ['password' => 'plain']);
}

public function testDontRehashPasswordIfNotRequired()
{
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false);
$hasher->shouldNotReceive('make');

$conn = m::mock(Connection::class);
$table = m::mock(ConnectionInterface::class);
$conn->shouldNotReceive('table');
$table->shouldNotReceive('where');
$table->shouldNotReceive('update');

$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$user->shouldNotReceive('getAuthIdentifierName');
$user->shouldNotReceive('getAuthIdentifier');
$user->shouldNotReceive('getAuthPasswordName');

$provider = new DatabaseUserProvider($conn, $hasher, 'foo');
$provider->rehashPasswordIfRequired($user, ['password' => 'plain']);
}
}
44 changes: 44 additions & 0 deletions tests/Auth/AuthEloquentUserProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,50 @@ public function testCredentialValidation()
$this->assertTrue($result);
}

public function testCredentialValidationFailed()
{
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(false);
$provider = new EloquentUserProvider($hasher, 'foo');
$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$result = $provider->validateCredentials($user, ['password' => 'plain']);

$this->assertFalse($result);
}

public function testRehashPasswordIfRequired()
{
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(true);
$hasher->shouldReceive('make')->once()->with('plain')->andReturn('rehashed');

$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$user->shouldReceive('getAuthPasswordName')->once()->andReturn('password_attribute');
$user->shouldReceive('forceFill')->once()->with(['password_attribute' => 'rehashed'])->andReturnSelf();
$user->shouldReceive('save')->once();

$provider = new EloquentUserProvider($hasher, 'foo');
$provider->rehashPasswordIfRequired($user, ['password' => 'plain']);
}

public function testDontRehashPasswordIfNotRequired()
{
$hasher = m::mock(Hasher::class);
$hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false);
$hasher->shouldNotReceive('make');

$user = m::mock(Authenticatable::class);
$user->shouldReceive('getAuthPassword')->once()->andReturn('hash');
$user->shouldNotReceive('getAuthPasswordName');
$user->shouldNotReceive('forceFill');
$user->shouldNotReceive('save');

$provider = new EloquentUserProvider($hasher, 'foo');
$provider->rehashPasswordIfRequired($user, ['password' => 'plain']);
}

public function testModelsCanBeCreated()
{
$hasher = m::mock(Hasher::class);
Expand Down