Skip to content

Commit

Permalink
[11.x] Gracefully handle null passwords when verifying credentials (#…
Browse files Browse the repository at this point in the history
…53156)

* Gracefully handle null passwords when verifying credentials

* Removed  method calls in tests.

---------

Co-authored-by: Graham Bradley <gbradley@onlyexcel.com>
  • Loading branch information
gbradley and Graham Bradley authored Oct 14, 2024
1 parent d62e92d commit 231091c
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
12 changes: 9 additions & 3 deletions src/Illuminate/Auth/DatabaseUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/Illuminate/Auth/EloquentUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
15 changes: 14 additions & 1 deletion tests/Auth/AuthDatabaseUserProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions tests/Auth/AuthEloquentUserProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 231091c

Please sign in to comment.