From df5bfd5b9eaec297eea7a2a65d46224c94b16e72 Mon Sep 17 00:00:00 2001 From: Dan Harrin Date: Sat, 30 Nov 2024 10:44:35 +0000 Subject: [PATCH] Hash recovery codes --- .../GoogleTwoFactorAuthentication.php | 19 +++++++++++++++++-- tests/database/factories/UserFactory.php | 11 +++++++++-- .../EmailCodeAuthenticationChallengeTest.php | 0 ...emoveEmailCodeAuthenticationActionTest.php | 0 ...SetUpEmailCodeAuthenticationActionTest.php | 0 ...rAuthenticationRecoveryCodesActionTest.php | 0 ...oogleTwoFactorAuthenticationActionTest.php | 14 +++++++------- ...oogleTwoFactorAuthenticationActionTest.php | 9 +++++++-- ...leTwoFactorAuthenticationChallengeTest.php | 8 ++++---- 9 files changed, 44 insertions(+), 17 deletions(-) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/EmailCode/EmailCodeAuthenticationChallengeTest.php (100%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/EmailCode/RemoveEmailCodeAuthenticationActionTest.php (100%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/EmailCode/SetUpEmailCodeAuthenticationActionTest.php (100%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/GoogleTwoFactor/Actions/RegenerateGoogleTwoFactorAuthenticationRecoveryCodesActionTest.php (100%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php (95%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php (96%) rename tests/src/Panels/{ => Auth}/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php (97%) diff --git a/packages/panels/src/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthentication.php b/packages/panels/src/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthentication.php index bed6fb8207..a7d845d9ae 100644 --- a/packages/panels/src/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthentication.php +++ b/packages/panels/src/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthentication.php @@ -20,6 +20,7 @@ use Filament\Schemas\Components\Utilities\Get; use Filament\Schemas\Components\Utilities\Set; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; use PragmaRX\Google2FAQRCode\Google2FA; @@ -89,7 +90,16 @@ class GoogleTwoFactorAuthentication implements MultiFactorAuthenticationProvider */ public function saveRecoveryCodes(HasGoogleTwoFactorAuthenticationRecovery $user, ?array $codes): void { - $user->saveGoogleTwoFactorAuthenticationRecoveryCodes($codes); + if (! is_array($codes)) { + $user->saveGoogleTwoFactorAuthenticationRecoveryCodes(null); + + return; + } + + $user->saveGoogleTwoFactorAuthenticationRecoveryCodes(array_map( + fn (string $code): string => Hash::make($code), + $codes, + )); } public function generateSecret(): string @@ -135,8 +145,13 @@ class GoogleTwoFactorAuthentication implements MultiFactorAuthenticationProvider $user ??= Filament::auth()->user(); /** @var HasGoogleTwoFactorAuthenticationRecovery $user */ + foreach ($this->getRecoveryCodes($user) as $hashedRecoveryCode) { + if (Hash::check($recoveryCode, $hashedRecoveryCode)) { + return true; + } + } - return in_array($recoveryCode, $this->getRecoveryCodes($user)); + return false; } /** diff --git a/tests/database/factories/UserFactory.php b/tests/database/factories/UserFactory.php index 68db90c23b..c3dc1205aa 100644 --- a/tests/database/factories/UserFactory.php +++ b/tests/database/factories/UserFactory.php @@ -6,6 +6,7 @@ use Filament\MultiFactorAuthentication\EmailCode\EmailCodeAuthentication; use Filament\MultiFactorAuthentication\GoogleTwoFactor\GoogleTwoFactorAuthentication; use Filament\Tests\Models\User; use Illuminate\Database\Eloquent\Factories\Factory; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; class UserFactory extends Factory @@ -32,13 +33,19 @@ class UserFactory extends Factory ]); } - public function hasGoogleTwoFactorAuthentication(): self + /** + * @param ?array $recoveryCodes + */ + public function hasGoogleTwoFactorAuthentication(?array $recoveryCodes = null): self { $googleTwoFactorAuthentication = GoogleTwoFactorAuthentication::make(); return $this->state(fn (): array => [ 'google_two_factor_authentication_secret' => $googleTwoFactorAuthentication->generateSecret(), - 'google_two_factor_authentication_recovery_codes' => $googleTwoFactorAuthentication->generateRecoveryCodes(), + 'google_two_factor_authentication_recovery_codes' => array_map( + fn (string $code): string => Hash::make($code), + $recoveryCodes ?? $googleTwoFactorAuthentication->generateRecoveryCodes(), + ), ]); } } diff --git a/tests/src/Panels/MultiFactorAuthentication/EmailCode/EmailCodeAuthenticationChallengeTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/EmailCodeAuthenticationChallengeTest.php similarity index 100% rename from tests/src/Panels/MultiFactorAuthentication/EmailCode/EmailCodeAuthenticationChallengeTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/EmailCodeAuthenticationChallengeTest.php diff --git a/tests/src/Panels/MultiFactorAuthentication/EmailCode/RemoveEmailCodeAuthenticationActionTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/RemoveEmailCodeAuthenticationActionTest.php similarity index 100% rename from tests/src/Panels/MultiFactorAuthentication/EmailCode/RemoveEmailCodeAuthenticationActionTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/RemoveEmailCodeAuthenticationActionTest.php diff --git a/tests/src/Panels/MultiFactorAuthentication/EmailCode/SetUpEmailCodeAuthenticationActionTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/SetUpEmailCodeAuthenticationActionTest.php similarity index 100% rename from tests/src/Panels/MultiFactorAuthentication/EmailCode/SetUpEmailCodeAuthenticationActionTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/EmailCode/SetUpEmailCodeAuthenticationActionTest.php diff --git a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/RegenerateGoogleTwoFactorAuthenticationRecoveryCodesActionTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/RegenerateGoogleTwoFactorAuthenticationRecoveryCodesActionTest.php similarity index 100% rename from tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/RegenerateGoogleTwoFactorAuthenticationRecoveryCodesActionTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/RegenerateGoogleTwoFactorAuthenticationRecoveryCodesActionTest.php diff --git a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php similarity index 95% rename from tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php index 1800fb4511..5d2a075f37 100644 --- a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php +++ b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/RemoveGoogleTwoFactorAuthenticationActionTest.php @@ -16,8 +16,12 @@ uses(TestCase::class); beforeEach(function () { Filament::setCurrentPanel('google-two-factor-authentication'); + $googleTwoFactorAuthentication = Arr::first(filament::getCurrentPanel()->getMultiFactorAuthenticationProviders()); + + $this->recoveryCodes = $googleTwoFactorAuthentication->generateRecoveryCodes(); + actingAs(User::factory() - ->hasGoogleTwoFactorAuthentication() + ->hasGoogleTwoFactorAuthentication($this->recoveryCodes) ->create()); }); @@ -58,8 +62,6 @@ it('can remove authentication when valid challenge code is used', function () { it('can remove authentication when a valid recovery code is used', function () { $user = auth()->user(); - $recoveryCodes = $user->getGoogleTwoFactorAuthenticationRecoveryCodes(); - expect($user->hasGoogleTwoFactorAuthentication()) ->toBeTrue(); @@ -76,7 +78,7 @@ it('can remove authentication when a valid recovery code is used', function () { ->callAction(TestAction::make('useRecoveryCode') ->schemaComponent('mountedActionSchema0.code')) ->setActionData([ - 'recoveryCode' => Arr::first($recoveryCodes), + 'recoveryCode' => Arr::first($this->recoveryCodes), ]) ->callMountedAction() ->assertHasNoActionErrors(); @@ -237,8 +239,6 @@ it('will not remove authentication with a recovery code if recovery is disabled' $user = auth()->user(); - $recoveryCodes = $user->getGoogleTwoFactorAuthenticationRecoveryCodes(); - expect($user->hasGoogleTwoFactorAuthentication()) ->toBeTrue(); @@ -253,7 +253,7 @@ it('will not remove authentication with a recovery code if recovery is disabled' ->callAction( TestAction::make('removeGoogleTwoFactorAuthentication') ->schemaComponent('form.google_two_factor.removeGoogleTwoFactorAuthenticationAction'), - ['recoveryCode' => Arr::first($recoveryCodes)], + ['recoveryCode' => Arr::first($this->recoveryCodes)], ) ->assertHasActionErrors(); diff --git a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php similarity index 96% rename from tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php index 9018088281..18114a9a89 100644 --- a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php +++ b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/Actions/SetUpGoogleTwoFactorAuthenticationActionTest.php @@ -6,6 +6,7 @@ use Filament\Pages\Auth\EditProfile; use Filament\Tests\Models\User; use Filament\Tests\TestCase; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; use function Filament\Tests\livewire; @@ -94,8 +95,12 @@ it('can save the secret and recovery codes to the user when the action is submit expect($user->getGoogleTwoFactorAuthenticationRecoveryCodes()) ->toBeArray() - ->toHaveCount(8) - ->toBe($recoveryCodes); + ->toHaveCount(8); + + foreach ($user->getGoogleTwoFactorAuthenticationRecoveryCodes() as $hashedRecoveryCode) { + expect(Hash::check(array_shift($recoveryCodes), $hashedRecoveryCode)) + ->toBeTrue(); + } }); it('will not set up authentication when an invalid code is used', function () { diff --git a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php similarity index 97% rename from tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php rename to tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php index af922fcd4b..7ca2d49924 100644 --- a/tests/src/Panels/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php +++ b/tests/src/Panels/Auth/MultiFactorAuthentication/GoogleTwoFactor/GoogleTwoFactorAuthenticationChallengeTest.php @@ -98,7 +98,7 @@ it('will authenticate the user after a valid recovery code is used', function () $googleTwoFactorAuthentication = Arr::first(filament::getCurrentPanel()->getMultiFactorAuthenticationProviders()); $userToAuthenticate = User::factory() - ->hasGoogleTwoFactorAuthentication() + ->hasGoogleTwoFactorAuthentication($recoveryCodes = $googleTwoFactorAuthentication->generateRecoveryCodes()) ->create(); livewire(Login::class) @@ -113,7 +113,7 @@ it('will authenticate the user after a valid recovery code is used', function () ->schemaComponent("multiFactorChallengeForm.{$googleTwoFactorAuthentication->getId()}.code")) ->fillForm([ $googleTwoFactorAuthentication->getId() => [ - 'recoveryCode' => Arr::random($googleTwoFactorAuthentication->getRecoveryCodes($userToAuthenticate)), + 'recoveryCode' => Arr::random($recoveryCodes), ], ], 'multiFactorChallengeForm') ->call('authenticate') @@ -321,7 +321,7 @@ it('will not authenticate the user with a valid recovery code if recovery is dis ->recoverable(false); $userToAuthenticate = User::factory() - ->hasGoogleTwoFactorAuthentication() + ->hasGoogleTwoFactorAuthentication($recoveryCodes = $googleTwoFactorAuthentication->generateRecoveryCodes()) ->create(); livewire(Login::class) @@ -334,7 +334,7 @@ it('will not authenticate the user with a valid recovery code if recovery is dis ->assertNoRedirect() ->fillForm([ $googleTwoFactorAuthentication->getId() => [ - 'recoveryCode' => Arr::random($googleTwoFactorAuthentication->getRecoveryCodes($userToAuthenticate)), + 'recoveryCode' => Arr::random($recoveryCodes), ], ], 'multiFactorChallengeForm') ->call('authenticate')