From 231091c19945f058c576005907a5518fcc7a2bb1 Mon Sep 17 00:00:00 2001 From: Graham Bradley Date: Mon, 14 Oct 2024 22:02:22 +0100 Subject: [PATCH] [11.x] Gracefully handle null passwords when verifying credentials (#53156) * Gracefully handle null passwords when verifying credentials * Removed method calls in tests. --------- Co-authored-by: Graham Bradley --- src/Illuminate/Auth/DatabaseUserProvider.php | 12 +++++++++--- src/Illuminate/Auth/EloquentUserProvider.php | 6 +++++- tests/Auth/AuthDatabaseUserProviderTest.php | 15 ++++++++++++++- tests/Auth/AuthEloquentUserProviderTest.php | 12 ++++++++++++ 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php index 3fd6b4061895..aaaafd8a8b45 100755 --- a/src/Illuminate/Auth/DatabaseUserProvider.php +++ b/src/Illuminate/Auth/DatabaseUserProvider.php @@ -154,9 +154,15 @@ protected function getGenericUser($user) */ public function validateCredentials(UserContract $user, #[\SensitiveParameter] array $credentials) { - return $this->hasher->check( - $credentials['password'], $user->getAuthPassword() - ); + if (is_null($plain = $credentials['password'])) { + return false; + } + + if (is_null($hashed = $user->getAuthPassword())) { + return false; + } + + return $this->hasher->check($plain, $hashed); } /** diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php index 5328329cce15..77c8fef712b1 100755 --- a/src/Illuminate/Auth/EloquentUserProvider.php +++ b/src/Illuminate/Auth/EloquentUserProvider.php @@ -152,7 +152,11 @@ public function validateCredentials(UserContract $user, #[\SensitiveParameter] a return false; } - return $this->hasher->check($plain, $user->getAuthPassword()); + if (is_null($hashed = $user->getAuthPassword())) { + return false; + } + + return $this->hasher->check($plain, $hashed); } /** diff --git a/tests/Auth/AuthDatabaseUserProviderTest.php b/tests/Auth/AuthDatabaseUserProviderTest.php index ad317be84253..e0faf75bb26d 100755 --- a/tests/Auth/AuthDatabaseUserProviderTest.php +++ b/tests/Auth/AuthDatabaseUserProviderTest.php @@ -162,7 +162,7 @@ public function testCredentialValidation() $this->assertTrue($result); } - public function testCredentialValidationFailed() + public function testCredentialValidationFails() { $conn = m::mock(Connection::class); $hasher = m::mock(Hasher::class); @@ -175,6 +175,19 @@ public function testCredentialValidationFailed() $this->assertFalse($result); } + public function testCredentialValidationFailsGracefullyWithNullPassword() + { + $conn = m::mock(Connection::class); + $hasher = m::mock(Hasher::class); + $hasher->shouldReceive('check')->never(); + $provider = new DatabaseUserProvider($conn, $hasher, 'foo'); + $user = m::mock(Authenticatable::class); + $user->shouldReceive('getAuthPassword')->once()->andReturn(null); + $result = $provider->validateCredentials($user, ['password' => 'plain']); + + $this->assertFalse($result); + } + public function testRehashPasswordIfRequired() { $hasher = m::mock(Hasher::class); diff --git a/tests/Auth/AuthEloquentUserProviderTest.php b/tests/Auth/AuthEloquentUserProviderTest.php index 21d181b97ad0..bb1aa7d1a1ee 100755 --- a/tests/Auth/AuthEloquentUserProviderTest.php +++ b/tests/Auth/AuthEloquentUserProviderTest.php @@ -152,6 +152,18 @@ public function testCredentialValidationFailed() $this->assertFalse($result); } + public function testCredentialValidationFailsGracefullyWithNullPassword() + { + $hasher = m::mock(Hasher::class); + $hasher->shouldReceive('check')->never(); + $provider = new EloquentUserProvider($hasher, 'foo'); + $user = m::mock(Authenticatable::class); + $user->shouldReceive('getAuthPassword')->once()->andReturn(null); + $result = $provider->validateCredentials($user, ['password' => 'plain']); + + $this->assertFalse($result); + } + public function testRehashPasswordIfRequired() { $hasher = m::mock(Hasher::class);