From d320d829df267b00e3a694a16e480875be121370 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 8 Oct 2023 08:10:48 +1000 Subject: [PATCH 01/13] Rehash user passwords when validating credentials --- src/Illuminate/Auth/Authenticatable.php | 10 +++++ src/Illuminate/Auth/DatabaseUserProvider.php | 30 +++++++++++++-- src/Illuminate/Auth/EloquentUserProvider.php | 24 +++++++++++- src/Illuminate/Auth/GenericUser.php | 10 +++++ .../Contracts/Auth/Authenticatable.php | 7 ++++ tests/Auth/AuthDatabaseUserProviderTest.php | 37 +++++++++++++++++++ tests/Auth/AuthEloquentUserProviderTest.php | 30 +++++++++++++++ 7 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Auth/Authenticatable.php b/src/Illuminate/Auth/Authenticatable.php index f1c0115916c8..c9a759467604 100644 --- a/src/Illuminate/Auth/Authenticatable.php +++ b/src/Illuminate/Auth/Authenticatable.php @@ -41,6 +41,16 @@ public function getAuthIdentifierForBroadcasting() return $this->getAuthIdentifier(); } + /** + * Get the name of the password attribute for the user. + * + * @return string + */ + public function getAuthPasswordName() + { + return 'password'; + } + /** * Get the password for the user. * diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 16b70ee9c76a..ffb1f706a11f 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -154,8 +154,32 @@ protected function getGenericUser($user) */ public function validateCredentials(UserContract $user, array $credentials) { - return $this->hasher->check( - $credentials['password'], $user->getAuthPassword() - ); + if (is_null($plain = $credentials['password'])) { + return false; + } + + if (! $this->hasher->check($plain, $hash = $user->getAuthPassword())) { + return false; + } + + if ($this->hasher->needsRehash($hash)) { + $this->rehashUserPassword($user, $plain); + } + + return true; + } + + /** + * Rehash the user's password. + * + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param string $plain + * @return void + */ + public function rehashUserPassword(UserContract $user, string $plain): void + { + $this->connection->table($this->table) + ->where($user->getAuthIdentifierName(), $user->getAuthIdentifier()) + ->update([$user->getAuthPasswordName() => $this->hasher->make($plain)]); } } diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index 39a744e0c098..e4577e344526 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -152,7 +152,29 @@ public function validateCredentials(UserContract $user, array $credentials) return false; } - return $this->hasher->check($plain, $user->getAuthPassword()); + if (! $this->hasher->check($plain, $hash = $user->getAuthPassword())) { + return false; + } + + if ($this->hasher->needsRehash($hash)) { + $this->rehashUserPassword($user, $plain); + } + + return true; + } + + /** + * Rehash the user's password. + * + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param string $plain + * @return void + */ + public function rehashUserPassword(UserContract $user, string $plain): void + { + $user->forceFill([ + $user->getAuthPasswordName() => $this->hasher->make($plain), + ])->save(); } /** diff --git a/src/Illuminate/Auth/GenericUser.php b/src/Illuminate/Auth/GenericUser.php index c87bc2382cb2..57bab7c8435d 100755 --- a/src/Illuminate/Auth/GenericUser.php +++ b/src/Illuminate/Auth/GenericUser.php @@ -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. * diff --git a/src/Illuminate/Contracts/Auth/Authenticatable.php b/src/Illuminate/Contracts/Auth/Authenticatable.php index ac4ed886d1b7..f5ed4987b44b 100644 --- a/src/Illuminate/Contracts/Auth/Authenticatable.php +++ b/src/Illuminate/Contracts/Auth/Authenticatable.php @@ -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. * diff --git a/tests/Auth/AuthDatabaseUserProviderTest.php b/tests/Auth/AuthDatabaseUserProviderTest.php index 382d3532a976..d2aa85ccef10 100755 --- a/tests/Auth/AuthDatabaseUserProviderTest.php +++ b/tests/Auth/AuthDatabaseUserProviderTest.php @@ -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; @@ -153,6 +154,7 @@ public function testCredentialValidation() $conn = m::mock(Connection::class); $hasher = m::mock(Hasher::class); $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); + $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false); $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); $user = m::mock(Authenticatable::class); $user->shouldReceive('getAuthPassword')->once()->andReturn('hash'); @@ -160,4 +162,39 @@ public function testCredentialValidation() $this->assertTrue($result); } + + public function testCredentialValidationRequiresRehash() + { + $hasher = m::mock(Hasher::class); + $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); + $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']); + $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); + $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'); + $result = $provider->validateCredentials($user, ['password' => 'plain']); + + $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); + } } diff --git a/tests/Auth/AuthEloquentUserProviderTest.php b/tests/Auth/AuthEloquentUserProviderTest.php index 9ef0e29636b0..69d8029aefb0 100755 --- a/tests/Auth/AuthEloquentUserProviderTest.php +++ b/tests/Auth/AuthEloquentUserProviderTest.php @@ -132,6 +132,7 @@ public function testCredentialValidation() { $hasher = m::mock(Hasher::class); $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); + $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false); $provider = new EloquentUserProvider($hasher, 'foo'); $user = m::mock(Authenticatable::class); $user->shouldReceive('getAuthPassword')->once()->andReturn('hash'); @@ -140,6 +141,35 @@ public function testCredentialValidation() $this->assertTrue($result); } + public function testCredentialValidationRequiresRehash() + { + $hasher = m::mock(Hasher::class); + $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); + $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(true); + $hasher->shouldReceive('make')->once()->with('plain')->andReturn('rehashed'); + $provider = new EloquentUserProvider($hasher, 'foo'); + $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(); + $result = $provider->validateCredentials($user, ['password' => 'plain']); + + $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 testModelsCanBeCreated() { $hasher = m::mock(Hasher::class); From 96bb0d1a713781ecc54a4c1f3a91384a1ec9356f Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 8 Oct 2023 10:46:21 +1000 Subject: [PATCH 02/13] Fix style violations --- src/Illuminate/Auth/DatabaseUserProvider.php | 4 ++-- src/Illuminate/Auth/EloquentUserProvider.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index ffb1f706a11f..43f9dab3e55c 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -172,8 +172,8 @@ public function validateCredentials(UserContract $user, array $credentials) /** * Rehash the user's password. * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param string $plain + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param string $plain * @return void */ public function rehashUserPassword(UserContract $user, string $plain): void diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index e4577e344526..a56f7e3e298a 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -166,8 +166,8 @@ public function validateCredentials(UserContract $user, array $credentials) /** * Rehash the user's password. * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param string $plain + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param string $plain * @return void */ public function rehashUserPassword(UserContract $user, string $plain): void From d6e06b5607b628b67c320284f07f07771383b267 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Mon, 9 Oct 2023 13:31:49 +1000 Subject: [PATCH 03/13] Remove hardcoded password when it's changable --- src/Illuminate/Auth/Authenticatable.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Auth/Authenticatable.php b/src/Illuminate/Auth/Authenticatable.php index c9a759467604..b939f1c60d41 100644 --- a/src/Illuminate/Auth/Authenticatable.php +++ b/src/Illuminate/Auth/Authenticatable.php @@ -11,6 +11,13 @@ trait Authenticatable */ protected $rememberTokenName = 'remember_token'; + /** + * The column name of the auth password field. + * + * @var string + */ + protected $authPasswordName = 'password'; + /** * Get the name of the unique identifier for the user. * @@ -48,7 +55,7 @@ public function getAuthIdentifierForBroadcasting() */ public function getAuthPasswordName() { - return 'password'; + return $this->authPasswordName; } /** @@ -58,7 +65,7 @@ public function getAuthPasswordName() */ public function getAuthPassword() { - return $this->password; + return $this->{$this->getAuthPasswordName()}; } /** From cce60ba604a063bedf9c88c7ac2ab7df78f822c1 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 26 Nov 2023 10:35:13 +1000 Subject: [PATCH 04/13] Shift rehashing into SessionGuard The Session guard's attempt() method is a better place to apply rehashing than the validateCredentials() method on the provider. The latter shouldn't have side-effects, as per it's name. --- src/Illuminate/Auth/DatabaseUserProvider.php | 31 ++++++------- src/Illuminate/Auth/EloquentUserProvider.php | 24 ++++------ src/Illuminate/Auth/SessionGuard.php | 2 + .../Contracts/Auth/UserProvider.php | 9 ++++ tests/Auth/AuthDatabaseUserProviderTest.php | 46 ++++++++++++++----- tests/Auth/AuthEloquentUserProviderTest.php | 36 ++++++++++----- tests/Auth/AuthGuardTest.php | 23 ++++++++++ 7 files changed, 116 insertions(+), 55 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 43f9dab3e55c..8f54bd503660 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -3,6 +3,7 @@ namespace Illuminate\Auth; use Closure; +use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\Authenticatable as UserContract; use Illuminate\Contracts\Auth\UserProvider; use Illuminate\Contracts\Hashing\Hasher as HasherContract; @@ -154,32 +155,26 @@ protected function getGenericUser($user) */ public function validateCredentials(UserContract $user, array $credentials) { - if (is_null($plain = $credentials['password'])) { - return false; - } - - if (! $this->hasher->check($plain, $hash = $user->getAuthPassword())) { - return false; - } - - if ($this->hasher->needsRehash($hash)) { - $this->rehashUserPassword($user, $plain); - } - - return true; + return $this->hasher->check( + $credentials['password'], $user->getAuthPassword() + ); } /** - * Rehash the user's password. + * Rehash the user's password if required and supported. * * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param string $plain - * @return void + * @param array $credentials + * @return string|null */ - public function rehashUserPassword(UserContract $user, string $plain): void + public function rehashPasswordIfRequired(Authenticatable $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($plain)]); + ->update([$user->getAuthPasswordName() => $this->hasher->make($credentials['password'])]); } } diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index a56f7e3e298a..4bf82ea3d673 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -152,28 +152,24 @@ public function validateCredentials(UserContract $user, array $credentials) return false; } - if (! $this->hasher->check($plain, $hash = $user->getAuthPassword())) { - return false; - } - - if ($this->hasher->needsRehash($hash)) { - $this->rehashUserPassword($user, $plain); - } - - return true; + return $this->hasher->check($plain, $user->getAuthPassword()); } /** - * Rehash the user's password. + * Rehash the user's password if required and supported. * * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param string $plain - * @return void + * @param array $credentials + * @return string|null */ - public function rehashUserPassword(UserContract $user, string $plain): void + public function rehashPasswordIfRequired(UserContract $user, array $credentials) { + if (! $this->hasher->needsRehash($user->getAuthPassword())) { + return; + } + $user->forceFill([ - $user->getAuthPasswordName() => $this->hasher->make($plain), + $user->getAuthPasswordName() => $this->hasher->make($credentials['password']), ])->save(); } diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index b475dbc6ca2e..dc69b7df8901 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -384,6 +384,7 @@ 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->provider->rehashPasswordIfRequired($user, $credentials); $this->login($user, $remember); return true; @@ -415,6 +416,7 @@ 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->provider->rehashPasswordIfRequired($user, $credentials); $this->login($user, $remember); return true; diff --git a/src/Illuminate/Contracts/Auth/UserProvider.php b/src/Illuminate/Contracts/Auth/UserProvider.php index a2ab122718c6..bfc1061a9781 100644 --- a/src/Illuminate/Contracts/Auth/UserProvider.php +++ b/src/Illuminate/Contracts/Auth/UserProvider.php @@ -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); } diff --git a/tests/Auth/AuthDatabaseUserProviderTest.php b/tests/Auth/AuthDatabaseUserProviderTest.php index d2aa85ccef10..158a12d13376 100755 --- a/tests/Auth/AuthDatabaseUserProviderTest.php +++ b/tests/Auth/AuthDatabaseUserProviderTest.php @@ -3,6 +3,7 @@ namespace Illuminate\Tests\Auth; use Illuminate\Auth\DatabaseUserProvider; +use Illuminate\Auth\EloquentUserProvider; use Illuminate\Auth\GenericUser; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Hashing\Hasher; @@ -154,7 +155,6 @@ public function testCredentialValidation() $conn = m::mock(Connection::class); $hasher = m::mock(Hasher::class); $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); - $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false); $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); $user = m::mock(Authenticatable::class); $user->shouldReceive('getAuthPassword')->once()->andReturn('hash'); @@ -163,38 +163,60 @@ public function testCredentialValidation() $this->assertTrue($result); } - public function testCredentialValidationRequiresRehash() + 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('check')->once()->with('plain', 'hash')->andReturn(true); $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']); - $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); + $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'); - $result = $provider->validateCredentials($user, ['password' => 'plain']); - $this->assertTrue($result); + $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); + $provider->rehashPasswordIfRequired($user, ['password' => 'plain']); } - public function testCredentialValidationFailed() + public function testDontRehashPasswordIfNotRequired() { - $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'); + $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'); - $result = $provider->validateCredentials($user, ['password' => 'plain']); + $user->shouldNotReceive('getAuthIdentifierName'); + $user->shouldNotReceive('getAuthIdentifier'); + $user->shouldNotReceive('getAuthPasswordName'); - $this->assertFalse($result); + $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); + $provider->rehashPasswordIfRequired($user, ['password' => 'plain']); } } diff --git a/tests/Auth/AuthEloquentUserProviderTest.php b/tests/Auth/AuthEloquentUserProviderTest.php index 69d8029aefb0..21d181b97ad0 100755 --- a/tests/Auth/AuthEloquentUserProviderTest.php +++ b/tests/Auth/AuthEloquentUserProviderTest.php @@ -132,7 +132,6 @@ public function testCredentialValidation() { $hasher = m::mock(Hasher::class); $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(true); - $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false); $provider = new EloquentUserProvider($hasher, 'foo'); $user = m::mock(Authenticatable::class); $user->shouldReceive('getAuthPassword')->once()->andReturn('hash'); @@ -141,33 +140,48 @@ public function testCredentialValidation() $this->assertTrue($result); } - public function testCredentialValidationRequiresRehash() + 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('check')->once()->with('plain', 'hash')->andReturn(true); $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(true); $hasher->shouldReceive('make')->once()->with('plain')->andReturn('rehashed'); - $provider = new EloquentUserProvider($hasher, 'foo'); + $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(); - $result = $provider->validateCredentials($user, ['password' => 'plain']); - $this->assertTrue($result); + $provider = new EloquentUserProvider($hasher, 'foo'); + $provider->rehashPasswordIfRequired($user, ['password' => 'plain']); } - public function testCredentialValidationFailed() + public function testDontRehashPasswordIfNotRequired() { $hasher = m::mock(Hasher::class); - $hasher->shouldReceive('check')->once()->with('plain', 'hash')->andReturn(false); - $provider = new EloquentUserProvider($hasher, 'foo'); + $hasher->shouldReceive('needsRehash')->once()->with('hash')->andReturn(false); + $hasher->shouldNotReceive('make'); + $user = m::mock(Authenticatable::class); $user->shouldReceive('getAuthPassword')->once()->andReturn('hash'); - $result = $provider->validateCredentials($user, ['password' => 'plain']); + $user->shouldNotReceive('getAuthPasswordName'); + $user->shouldNotReceive('forceFill'); + $user->shouldNotReceive('save'); - $this->assertFalse($result); + $provider = new EloquentUserProvider($hasher, 'foo'); + $provider->rehashPasswordIfRequired($user, ['password' => 'plain']); } public function testModelsCanBeCreated() diff --git a/tests/Auth/AuthGuardTest.php b/tests/Auth/AuthGuardTest.php index 52c4cfe7d1c8..9a35d59d322b 100755 --- a/tests/Auth/AuthGuardTest.php +++ b/tests/Auth/AuthGuardTest.php @@ -103,6 +103,7 @@ public function testAttemptCallsRetrieveByCredentials() $events->shouldReceive('dispatch')->once()->with(m::type(Failed::class)); $events->shouldNotReceive('dispatch')->with(m::type(Validated::class)); $guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->with(['foo']); + $guard->getProvider()->shouldNotReceive('rehashPasswordIfRequired'); $guard->attempt(['foo']); } @@ -119,6 +120,7 @@ public function testAttemptReturnsUserInterface() $user = $this->createMock(Authenticatable::class); $guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->andReturn($user); $guard->getProvider()->shouldReceive('validateCredentials')->with($user, ['foo'])->andReturn(true); + $guard->getProvider()->shouldReceive('rehashPasswordIfRequired')->with($user, ['foo'])->once(); $guard->expects($this->once())->method('login')->with($this->equalTo($user)); $this->assertTrue($guard->attempt(['foo'])); } @@ -135,6 +137,7 @@ public function testAttemptReturnsFalseIfUserNotGiven() $events->shouldReceive('dispatch')->once()->with(m::type(Failed::class)); $events->shouldNotReceive('dispatch')->with(m::type(Validated::class)); $mock->getProvider()->shouldReceive('retrieveByCredentials')->once()->andReturn(null); + $mock->getProvider()->shouldNotReceive('rehashPasswordIfRequired'); $this->assertFalse($mock->attempt(['foo'])); } @@ -159,6 +162,7 @@ public function testAttemptAndWithCallbacks() $mock->getProvider()->shouldReceive('retrieveByCredentials')->times(3)->with(['foo'])->andReturn($user); $mock->getProvider()->shouldReceive('validateCredentials')->twice()->andReturnTrue(); $mock->getProvider()->shouldReceive('validateCredentials')->once()->andReturnFalse(); + $mock->getProvider()->shouldReceive('rehashPasswordIfRequired')->with($user, ['foo'])->once(); $this->assertTrue($mock->attemptWhen(['foo'], function ($user, $guard) { static::assertInstanceOf(Authenticatable::class, $user); @@ -183,6 +187,24 @@ public function testAttemptAndWithCallbacks() $this->assertFalse($executed); } + public function testAttemptRehashesPasswordWhenRequired() + { + [$session, $provider, $request, $cookie, $timebox] = $this->getMocks(); + $guard = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['login'])->setConstructorArgs(['default', $provider, $session, $request, $timebox])->getMock(); + $guard->setDispatcher($events = m::mock(Dispatcher::class)); + $timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback, $microseconds) use ($timebox) { + return $callback($timebox->shouldReceive('returnEarly')->once()->getMock()); + }); + $events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class)); + $events->shouldReceive('dispatch')->once()->with(m::type(Validated::class)); + $user = $this->createMock(Authenticatable::class); + $guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->andReturn($user); + $guard->getProvider()->shouldReceive('validateCredentials')->with($user, ['foo'])->andReturn(true); + $guard->getProvider()->shouldReceive('rehashPasswordIfRequired')->with($user, ['foo'])->once(); + $guard->expects($this->once())->method('login')->with($this->equalTo($user)); + $this->assertTrue($guard->attempt(['foo'])); + } + public function testLoginStoresIdentifierInSession() { [$session, $provider, $request, $cookie] = $this->getMocks(); @@ -235,6 +257,7 @@ public function testFailedAttemptFiresFailedEvent() $events->shouldReceive('dispatch')->once()->with(m::type(Failed::class)); $events->shouldNotReceive('dispatch')->with(m::type(Validated::class)); $guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->with(['foo'])->andReturn(null); + $guard->getProvider()->shouldNotReceive('rehashPasswordIfRequired'); $guard->attempt(['foo']); } From 6a972abc0b320e8fecdd2af44cb0a78cc4bdf6ca Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 26 Nov 2023 10:39:17 +1000 Subject: [PATCH 05/13] Fix style violation --- tests/Auth/AuthDatabaseUserProviderTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Auth/AuthDatabaseUserProviderTest.php b/tests/Auth/AuthDatabaseUserProviderTest.php index 158a12d13376..ad317be84253 100755 --- a/tests/Auth/AuthDatabaseUserProviderTest.php +++ b/tests/Auth/AuthDatabaseUserProviderTest.php @@ -3,7 +3,6 @@ namespace Illuminate\Tests\Auth; use Illuminate\Auth\DatabaseUserProvider; -use Illuminate\Auth\EloquentUserProvider; use Illuminate\Auth\GenericUser; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Hashing\Hasher; From 5032d143d727d8c39255efd43243179c622857b5 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 26 Nov 2023 11:10:22 +1000 Subject: [PATCH 06/13] Add config option to disable rehashing on login --- src/Illuminate/Auth/AuthManager.php | 1 + src/Illuminate/Auth/CreatesUserProviders.php | 4 +-- src/Illuminate/Auth/SessionGuard.php | 29 ++++++++++++++++++-- src/Illuminate/Hashing/HashManager.php | 10 +++++++ tests/Auth/AuthGuardTest.php | 20 ++++++++++++++ 5 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Auth/AuthManager.php b/src/Illuminate/Auth/AuthManager.php index e95da5ec4ae4..03558cfaff48 100755 --- a/src/Illuminate/Auth/AuthManager.php +++ b/src/Illuminate/Auth/AuthManager.php @@ -128,6 +128,7 @@ public function createSessionDriver($name, $config) $name, $provider, $this->app['session.store'], + $this->app['hash']->rehashOnLogin(), ); // When using the remember me functionality of the authentication services we diff --git a/src/Illuminate/Auth/CreatesUserProviders.php b/src/Illuminate/Auth/CreatesUserProviders.php index 761a427668ee..302ee2f3f918 100644 --- a/src/Illuminate/Auth/CreatesUserProviders.php +++ b/src/Illuminate/Auth/CreatesUserProviders.php @@ -65,7 +65,7 @@ protected function createDatabaseProvider($config) { $connection = $this->app['db']->connection($config['connection'] ?? null); - return new DatabaseUserProvider($connection, $this->app['hash'], $config['table']); + return new DatabaseUserProvider($connection, $this->app['hash'], $config['table'], $this->app['hash']->rehasOnLogin()); } /** @@ -76,7 +76,7 @@ protected function createDatabaseProvider($config) */ protected function createEloquentProvider($config) { - return new EloquentUserProvider($this->app['hash'], $config['model']); + return new EloquentUserProvider($this->app['hash'], $config['model'], $this->app['hash']->rehasOnLogin()); } /** diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index dc69b7df8901..9cbd598b4df7 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -96,6 +96,13 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth */ protected $timebox; + /** + * Rehash passwords during login. + * + * @var bool + */ + protected $rehashOnLogin; + /** * Indicates if the logout method has been called. * @@ -124,13 +131,15 @@ 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; } /** @@ -384,7 +393,7 @@ 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->provider->rehashPasswordIfRequired($user, $credentials); + $this->rehashPasswordIfRequired($user, $credentials); $this->login($user, $remember); return true; @@ -416,7 +425,7 @@ 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->provider->rehashPasswordIfRequired($user, $credentials); + $this->rehashPasswordIfRequired($user, $credentials); $this->login($user, $remember); return true; @@ -449,6 +458,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 bool + */ + 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. * diff --git a/src/Illuminate/Hashing/HashManager.php b/src/Illuminate/Hashing/HashManager.php index 564411e4df54..7a5af4dce819 100644 --- a/src/Illuminate/Hashing/HashManager.php +++ b/src/Illuminate/Hashing/HashManager.php @@ -99,6 +99,16 @@ public function isHashed($value) return password_get_info($value)['algo'] !== null; } + /** + * Determine if rehashing should be performed during login. + * + * @return bool + */ + public function rehashOnLogin() + { + return $this->config->get('hashing.rehash_on_login', true); + } + /** * Get the default driver name. * diff --git a/tests/Auth/AuthGuardTest.php b/tests/Auth/AuthGuardTest.php index 9a35d59d322b..e75efd642c37 100755 --- a/tests/Auth/AuthGuardTest.php +++ b/tests/Auth/AuthGuardTest.php @@ -205,6 +205,26 @@ public function testAttemptRehashesPasswordWhenRequired() $this->assertTrue($guard->attempt(['foo'])); } + public function testAttemptDoesntRehashPasswordWhenDisabled() + { + [$session, $provider, $request, $cookie, $timebox] = $this->getMocks(); + $guard = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['login']) + ->setConstructorArgs(['default', $provider, $session, $request, $timebox, $rehashOnLogin = false]) + ->getMock(); + $guard->setDispatcher($events = m::mock(Dispatcher::class)); + $timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback, $microseconds) use ($timebox) { + return $callback($timebox->shouldReceive('returnEarly')->once()->getMock()); + }); + $events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class)); + $events->shouldReceive('dispatch')->once()->with(m::type(Validated::class)); + $user = $this->createMock(Authenticatable::class); + $guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->andReturn($user); + $guard->getProvider()->shouldReceive('validateCredentials')->with($user, ['foo'])->andReturn(true); + $guard->getProvider()->shouldNotReceive('rehashPasswordIfRequired'); + $guard->expects($this->once())->method('login')->with($this->equalTo($user)); + $this->assertTrue($guard->attempt(['foo'])); + } + public function testLoginStoresIdentifierInSession() { [$session, $provider, $request, $cookie] = $this->getMocks(); From f72f8652e2b8f7cf8496f6a7a500428abbfc6d76 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 26 Nov 2023 17:15:40 +1000 Subject: [PATCH 07/13] Clean up rehash flag injection --- src/Illuminate/Auth/AuthManager.php | 2 +- src/Illuminate/Auth/CreatesUserProviders.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Auth/AuthManager.php b/src/Illuminate/Auth/AuthManager.php index 03558cfaff48..ac48713c584e 100755 --- a/src/Illuminate/Auth/AuthManager.php +++ b/src/Illuminate/Auth/AuthManager.php @@ -128,7 +128,7 @@ public function createSessionDriver($name, $config) $name, $provider, $this->app['session.store'], - $this->app['hash']->rehashOnLogin(), + rehashOnLogin: $this->app['hash']->rehashOnLogin(), ); // When using the remember me functionality of the authentication services we diff --git a/src/Illuminate/Auth/CreatesUserProviders.php b/src/Illuminate/Auth/CreatesUserProviders.php index 302ee2f3f918..761a427668ee 100644 --- a/src/Illuminate/Auth/CreatesUserProviders.php +++ b/src/Illuminate/Auth/CreatesUserProviders.php @@ -65,7 +65,7 @@ protected function createDatabaseProvider($config) { $connection = $this->app['db']->connection($config['connection'] ?? null); - return new DatabaseUserProvider($connection, $this->app['hash'], $config['table'], $this->app['hash']->rehasOnLogin()); + return new DatabaseUserProvider($connection, $this->app['hash'], $config['table']); } /** @@ -76,7 +76,7 @@ protected function createDatabaseProvider($config) */ protected function createEloquentProvider($config) { - return new EloquentUserProvider($this->app['hash'], $config['model'], $this->app['hash']->rehasOnLogin()); + return new EloquentUserProvider($this->app['hash'], $config['model']); } /** From 1c0a949cbbee899ebb23f2372d1af576deb3ade6 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Sun, 26 Nov 2023 17:18:42 +1000 Subject: [PATCH 08/13] Fix contract in DatabaseUserProvider --- src/Illuminate/Auth/DatabaseUserProvider.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 8f54bd503660..97712fba8e1b 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -3,7 +3,6 @@ namespace Illuminate\Auth; use Closure; -use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\Authenticatable as UserContract; use Illuminate\Contracts\Auth\UserProvider; use Illuminate\Contracts\Hashing\Hasher as HasherContract; @@ -167,7 +166,7 @@ public function validateCredentials(UserContract $user, array $credentials) * @param array $credentials * @return string|null */ - public function rehashPasswordIfRequired(Authenticatable $user, array $credentials) + public function rehashPasswordIfRequired(UserContract $user, array $credentials) { if (! $this->hasher->needsRehash($user->getAuthPassword())) { return; From 787de1a3d85420260e45f38468306f312d160658 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Tue, 28 Nov 2023 06:59:18 +1000 Subject: [PATCH 09/13] Fixing return type in the docblocks --- src/Illuminate/Auth/DatabaseUserProvider.php | 2 +- src/Illuminate/Auth/EloquentUserProvider.php | 2 +- src/Illuminate/Auth/SessionGuard.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 97712fba8e1b..82c38b7f551d 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -164,7 +164,7 @@ public function validateCredentials(UserContract $user, array $credentials) * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials - * @return string|null + * @return void */ public function rehashPasswordIfRequired(UserContract $user, array $credentials) { diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index 4bf82ea3d673..daaa23d09767 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -160,7 +160,7 @@ public function validateCredentials(UserContract $user, array $credentials) * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials - * @return string|null + * @return void */ public function rehashPasswordIfRequired(UserContract $user, array $credentials) { diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index 9cbd598b4df7..70ba73cc1ff0 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -463,7 +463,7 @@ protected function hasValidCredentials($user, $credentials) * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials - * @return bool + * @return void */ protected function rehashPasswordIfRequired(AuthenticatableContract $user, array $credentials) { From 33ae3492cb7c207e3f926a8b293950c58174ed65 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Tue, 28 Nov 2023 07:18:51 +1000 Subject: [PATCH 10/13] Use hash_equals() for a secure string comparison --- src/Illuminate/Session/Middleware/AuthenticateSession.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Session/Middleware/AuthenticateSession.php b/src/Illuminate/Session/Middleware/AuthenticateSession.php index 43982374b456..b6122cb11eb2 100644 --- a/src/Illuminate/Session/Middleware/AuthenticateSession.php +++ b/src/Illuminate/Session/Middleware/AuthenticateSession.php @@ -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); } } @@ -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); } From 9bce9ddb467c4f1bf062ebd59ea47b4bfaabb3b4 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 6 Dec 2023 14:47:12 -0600 Subject: [PATCH 11/13] formatting --- src/Illuminate/Auth/AuthManager.php | 2 +- src/Illuminate/Auth/Authenticatable.php | 8 ++++---- src/Illuminate/Auth/SessionGuard.php | 5 ++++- src/Illuminate/Hashing/HashManager.php | 10 ---------- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/Illuminate/Auth/AuthManager.php b/src/Illuminate/Auth/AuthManager.php index ac48713c584e..4bba02800ad7 100755 --- a/src/Illuminate/Auth/AuthManager.php +++ b/src/Illuminate/Auth/AuthManager.php @@ -128,7 +128,7 @@ public function createSessionDriver($name, $config) $name, $provider, $this->app['session.store'], - rehashOnLogin: $this->app['hash']->rehashOnLogin(), + rehashOnLogin: $this->app['config']->get('hashing.rehash_on_login', true), ); // When using the remember me functionality of the authentication services we diff --git a/src/Illuminate/Auth/Authenticatable.php b/src/Illuminate/Auth/Authenticatable.php index b939f1c60d41..0145c1cc8901 100644 --- a/src/Illuminate/Auth/Authenticatable.php +++ b/src/Illuminate/Auth/Authenticatable.php @@ -5,18 +5,18 @@ trait Authenticatable { /** - * The column name of the "remember me" token. + * The column name of the password field using during authentication. * * @var string */ - protected $rememberTokenName = 'remember_token'; + protected $authPasswordName = 'password'; /** - * The column name of the auth password field. + * The column name of the "remember me" token. * * @var string */ - protected $authPasswordName = 'password'; + protected $rememberTokenName = 'remember_token'; /** * Get the name of the unique identifier for the user. diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index 70ba73cc1ff0..9807c4bf68a3 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -97,7 +97,7 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth protected $timebox; /** - * Rehash passwords during login. + * Indicates if paswords should be rehashed on login if needed. * * @var bool */ @@ -125,6 +125,7 @@ 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, @@ -394,6 +395,7 @@ public function attempt(array $credentials = [], $remember = false) // 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; @@ -426,6 +428,7 @@ public function attemptWhen(array $credentials = [], $callbacks = null, $remembe // 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; diff --git a/src/Illuminate/Hashing/HashManager.php b/src/Illuminate/Hashing/HashManager.php index 7a5af4dce819..564411e4df54 100644 --- a/src/Illuminate/Hashing/HashManager.php +++ b/src/Illuminate/Hashing/HashManager.php @@ -99,16 +99,6 @@ public function isHashed($value) return password_get_info($value)['algo'] !== null; } - /** - * Determine if rehashing should be performed during login. - * - * @return bool - */ - public function rehashOnLogin() - { - return $this->config->get('hashing.rehash_on_login', true); - } - /** * Get the default driver name. * From 1fceb3ef8eac8f349a018b709773724df3544d14 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 6 Dec 2023 15:10:00 -0600 Subject: [PATCH 12/13] formatting, leverage method on logoutOtherDevices --- src/Illuminate/Auth/DatabaseUserProvider.php | 5 +- src/Illuminate/Auth/EloquentUserProvider.php | 5 +- src/Illuminate/Auth/SessionGuard.php | 48 +++++++++---------- .../Contracts/Auth/UserProvider.php | 3 +- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 82c38b7f551d..1be060cb831a 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -164,11 +164,12 @@ public function validateCredentials(UserContract $user, array $credentials) * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials + * @param bool $force * @return void */ - public function rehashPasswordIfRequired(UserContract $user, array $credentials) + public function rehashPasswordIfRequired(UserContract $user, array $credentials, bool $force = false) { - if (! $this->hasher->needsRehash($user->getAuthPassword())) { + if (! $this->hasher->needsRehash($user->getAuthPassword()) && ! $force) { return; } diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index daaa23d09767..646c2187f595 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -160,11 +160,12 @@ public function validateCredentials(UserContract $user, array $credentials) * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials + * @param bool $force * @return void */ - public function rehashPasswordIfRequired(UserContract $user, array $credentials) + public function rehashPasswordIfRequired(UserContract $user, array $credentials, bool $force = false) { - if (! $this->hasher->needsRehash($user->getAuthPassword())) { + if (! $this->hasher->needsRehash($user->getAuthPassword()) && ! $force) { return; } diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index 9807c4bf68a3..123a8521b037 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -461,20 +461,6 @@ 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. * @@ -493,6 +479,20 @@ protected function shouldLogin($callbacks, AuthenticatableContract $user) return true; } + /** + * 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); + } + } + /** * Log the given user ID into the application. * @@ -684,18 +684,17 @@ protected function cycleRememberToken(AuthenticatableContract $user) * The application must be using the AuthenticateSession middleware. * * @param string $password - * @param string $attribute * @return \Illuminate\Contracts\Auth\Authenticatable|null * * @throws \Illuminate\Auth\AuthenticationException */ - public function logoutOtherDevices($password, $attribute = 'password') + public function logoutOtherDevices($password) { if (! $this->user()) { return; } - $result = $this->rehashUserPassword($password, $attribute); + $result = $this->rehashUserPasswordForDeviceLogout($password); if ($this->recaller() || $this->getCookieJar()->hasQueued($this->getRecallerName())) { @@ -708,23 +707,24 @@ public function logoutOtherDevices($password, $attribute = 'password') } /** - * Rehash the current user's password. + * Rehash the current user's password for logging out other devices via AuthenticateSession. * * @param string $password - * @param string $attribute * @return \Illuminate\Contracts\Auth\Authenticatable|null * * @throws \InvalidArgumentException */ - protected function rehashUserPassword($password, $attribute) + protected function rehashUserPasswordForDeviceLogout($password) { - if (! Hash::check($password, $this->user()->{$attribute})) { + $user = $this->user(); + + if (! Hash::check($password, $user->getAuthPassword())) { throw new InvalidArgumentException('The given password does not match the current password.'); } - return tap($this->user()->forceFill([ - $attribute => Hash::make($password), - ]))->save(); + $this->provider->rehashPasswordIfRequired( + $user, ['password' => $password], force: true + ); } /** diff --git a/src/Illuminate/Contracts/Auth/UserProvider.php b/src/Illuminate/Contracts/Auth/UserProvider.php index bfc1061a9781..4ed51bf00e9c 100644 --- a/src/Illuminate/Contracts/Auth/UserProvider.php +++ b/src/Illuminate/Contracts/Auth/UserProvider.php @@ -52,7 +52,8 @@ public function validateCredentials(Authenticatable $user, array $credentials); * * @param \Illuminate\Contracts\Auth\Authenticatable $user * @param array $credentials + * @param bool $force * @return void */ - public function rehashPasswordIfRequired(Authenticatable $user, array $credentials); + public function rehashPasswordIfRequired(Authenticatable $user, array $credentials, bool $force = false); } From 26467617253f99b230a21109382bcbae41849ce7 Mon Sep 17 00:00:00 2001 From: Stephen Rees-Carter Date: Fri, 8 Dec 2023 09:17:15 +1000 Subject: [PATCH 13/13] Fix spelling of passwords Co-authored-by: Chrysanthos <48060191+chrysanthos@users.noreply.github.com> --- src/Illuminate/Auth/SessionGuard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index 123a8521b037..7966eaa7da12 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -97,7 +97,7 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth protected $timebox; /** - * Indicates if paswords should be rehashed on login if needed. + * Indicates if passwords should be rehashed on login if needed. * * @var bool */