From 72ab21f71743a783c582fb31495964a06acd1f8f Mon Sep 17 00:00:00 2001 From: Claudio Dekker Date: Mon, 3 Apr 2023 23:35:58 +0200 Subject: [PATCH] Recovery: Skip the recovery code challenge when no codes exist (#30) * Use security indicator in example views * Recovery: Skip the recovery code challenge when no codes exist --- .../AccountRecoveryChallengeViewTests.php | 2 +- .../stubs/defaults/views/home.blade.stub | 4 +- .../views/settings/credentials.blade.stub | 9 ++++ .../AccountRecoveryChallengeController.php | 22 +++++++- .../SubmitAccountRecoveryChallengeTests.php | 52 ++++++++++--------- .../ViewAccountRecoveryChallengePageTests.php | 47 ++++++++++++++--- 6 files changed, 99 insertions(+), 37 deletions(-) diff --git a/packages/bladebones/src/Testing/Partials/AccountRecoveryChallengeViewTests.php b/packages/bladebones/src/Testing/Partials/AccountRecoveryChallengeViewTests.php index e979ebe..8e1313e 100644 --- a/packages/bladebones/src/Testing/Partials/AccountRecoveryChallengeViewTests.php +++ b/packages/bladebones/src/Testing/Partials/AccountRecoveryChallengeViewTests.php @@ -9,7 +9,7 @@ trait AccountRecoveryChallengeViewTests /** @test */ public function the_account_recovery_challenge_page_uses_blade_views(): void { - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $token = Password::getRepository()->create($user); $response = $this->get(route('recover-account.challenge', [ diff --git a/packages/bladebones/stubs/defaults/views/home.blade.stub b/packages/bladebones/stubs/defaults/views/home.blade.stub index 481bda6..3ef2e9e 100644 --- a/packages/bladebones/stubs/defaults/views/home.blade.stub +++ b/packages/bladebones/stubs/defaults/views/home.blade.stub @@ -1,7 +1,7 @@

Authenticated as {{ Auth::user()->name }}

-@if (! Auth::user()->recovery_codes) -

You currently do not have any recovery codes configured

+@if (($indicator = Auth::user()->accountSecurityIndicator()) && $indicator->hasIssues()) +

{{ $indicator->message() }}

@endif View Authentication Settings diff --git a/packages/bladebones/stubs/defaults/views/settings/credentials.blade.stub b/packages/bladebones/stubs/defaults/views/settings/credentials.blade.stub index 7789b59..1db8dd6 100644 --- a/packages/bladebones/stubs/defaults/views/settings/credentials.blade.stub +++ b/packages/bladebones/stubs/defaults/views/settings/credentials.blade.stub @@ -1,4 +1,9 @@

Authentication Settings

+ +@if (($indicator = Auth::user()->accountSecurityIndicator()) && $indicator->hasIssues()) +

{{ $indicator->message() }}

+@endif + Home @if (session('status')) @@ -115,6 +120,10 @@

Recovery Codes

Recovery codes can be used to access your account in the event you lose access to your credentials.

+@if (! Auth::user()->recovery_codes) +

You currently have no recovery codes configured.

+@endif +
diff --git a/packages/core/src/Http/Controllers/Challenges/AccountRecoveryChallengeController.php b/packages/core/src/Http/Controllers/Challenges/AccountRecoveryChallengeController.php index 6112e87..5c1b05a 100644 --- a/packages/core/src/Http/Controllers/Challenges/AccountRecoveryChallengeController.php +++ b/packages/core/src/Http/Controllers/Challenges/AccountRecoveryChallengeController.php @@ -72,6 +72,12 @@ public function create(Request $request, string $token) return $this->sendInvalidRecoveryLinkResponse($request); } + if (! $this->hasRecoveryCodes($request, $user)) { + $this->invalidateRecoveryLink($request, $user); + + return $this->handleAccountRecoveredResponse($request, $user); + } + return $this->sendChallengePageResponse($request, $token); } @@ -105,6 +111,12 @@ public function store(Request $request, string $token) return $this->sendInvalidRecoveryLinkResponse($request); } + if (! $this->hasRecoveryCodes($request, $user)) { + $this->invalidateRecoveryLink($request, $user); + + return $this->handleAccountRecoveredResponse($request, $user); + } + if (! $this->hasValidRecoveryCode($request, $user)) { $this->incrementRateLimitingCounter($request); $this->emitAccountRecoveryFailedEvent($request, $user); @@ -161,12 +173,20 @@ protected function authenticate(Request $request, Authenticatable $user): void Auth::login($user); } + /** + * Determine whether the user has recovery codes. + */ + protected function hasRecoveryCodes(Request $request, Authenticatable $user): bool + { + return (bool) $user->recovery_codes; + } + /** * Determine whether the user has entered a valid confirmation code. */ protected function hasValidRecoveryCode(Request $request, Authenticatable $user): bool { - return RecoveryCodeManager::from($user->recovery_codes ?? [])->contains($request->input('code')); + return RecoveryCodeManager::from($user->recovery_codes)->contains($request->input('code')); } /** diff --git a/packages/core/src/Testing/Partials/Challenges/Recovery/SubmitAccountRecoveryChallengeTests.php b/packages/core/src/Testing/Partials/Challenges/Recovery/SubmitAccountRecoveryChallengeTests.php index 1fffebb..5e24be1 100644 --- a/packages/core/src/Testing/Partials/Challenges/Recovery/SubmitAccountRecoveryChallengeTests.php +++ b/packages/core/src/Testing/Partials/Challenges/Recovery/SubmitAccountRecoveryChallengeTests.php @@ -7,6 +7,7 @@ use ClaudioDekker\LaravelAuth\Events\AccountRecoveryFailed; use ClaudioDekker\LaravelAuth\Events\SudoModeEnabled; use ClaudioDekker\LaravelAuth\Http\Middleware\EnsureSudoMode; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Password; use Illuminate\Validation\ValidationException; @@ -39,6 +40,32 @@ public function the_user_account_can_be_recovered(): void Event::assertNotDispatched(SudoModeEnabled::class); } + /** @test */ + public function the_account_recovery_challenge_code_verification_request_accepts_any_code_when_the_users_recovery_codes_are_cleared(): void + { + Carbon::setTestNow(now()); + Event::fake([AccountRecovered::class, AccountRecoveryFailed::class, SudoModeEnabled::class]); + $user = $this->generateUser(['recovery_codes' => null]); + $repository = Password::getRepository(); + $token = $repository->create($user); + $this->assertTrue($repository->exists($user, $token)); + + $response = $this->post(route('recover-account.challenge', ['token' => $token]), [ + 'email' => $user->getEmailForPasswordReset(), + 'code' => 'INVLD-CODES', + ]); + + $response->assertRedirect(route('auth.settings')); + $response->assertSessionMissing(EnsureSudoMode::REQUIRED_AT_KEY); + $response->assertSessionHas(EnsureSudoMode::CONFIRMED_AT_KEY, now()->unix()); + $this->assertFullyAuthenticatedAs($response, $user); + $this->assertFalse($repository->exists($user, $token)); + Event::assertDispatched(AccountRecovered::class, fn ($event) => $event->user->is($user) && $event->request === request()); + Event::assertNotDispatched(AccountRecoveryFailed::class); + Event::assertNotDispatched(SudoModeEnabled::class); + Carbon::setTestNow(); + } + /** @test */ public function the_user_account_cannot_be_recovered_when_authenticated(): void { @@ -160,29 +187,4 @@ public function the_user_account_cannot_be_recovered_when_an_invalid_recovery_co Event::assertNotDispatched(AccountRecovered::class); Event::assertNotDispatched(SudoModeEnabled::class); } - - /** @test */ - public function the_user_account_cannot_be_recovered_when_the_user_has_no_configured_recovery_codes(): void - { - Event::fake([AccountRecovered::class, AccountRecoveryFailed::class, SudoModeEnabled::class]); - $user = $this->generateUser(['recovery_codes' => null]); - $repository = Password::getRepository(); - $token = $repository->create($user); - - $response = $this->post(route('recover-account.challenge', ['token' => $token]), [ - 'email' => $user->getEmailForPasswordReset(), - 'code' => 'PIPIM-7LTUT', - ]); - - $this->assertInstanceOf(ValidationException::class, $response->exception); - $this->assertSame(['code' => [__('laravel-auth::auth.challenge.recovery')]], $response->exception->errors()); - $this->assertTrue($repository->exists($user, $token)); - $this->assertNull($user->fresh()->recovery_codes); - $this->assertGuest(); - $response->assertSessionMissing(EnsureSudoMode::REQUIRED_AT_KEY); - $response->assertSessionMissing(EnsureSudoMode::CONFIRMED_AT_KEY); - Event::assertDispatched(AccountRecoveryFailed::class, fn ($event) => $event->user->is($user) && $event->request === request()); - Event::assertNotDispatched(AccountRecovered::class); - Event::assertNotDispatched(SudoModeEnabled::class); - } } diff --git a/packages/core/src/Testing/Partials/Challenges/Recovery/ViewAccountRecoveryChallengePageTests.php b/packages/core/src/Testing/Partials/Challenges/Recovery/ViewAccountRecoveryChallengePageTests.php index 4bbcc8d..ecf1521 100644 --- a/packages/core/src/Testing/Partials/Challenges/Recovery/ViewAccountRecoveryChallengePageTests.php +++ b/packages/core/src/Testing/Partials/Challenges/Recovery/ViewAccountRecoveryChallengePageTests.php @@ -3,7 +3,12 @@ namespace ClaudioDekker\LaravelAuth\Testing\Partials\Challenges\Recovery; use App\Providers\RouteServiceProvider; -use Carbon\Carbon; +use ClaudioDekker\LaravelAuth\Events\AccountRecovered; +use ClaudioDekker\LaravelAuth\Events\AccountRecoveryFailed; +use ClaudioDekker\LaravelAuth\Events\SudoModeEnabled; +use ClaudioDekker\LaravelAuth\Http\Middleware\EnsureSudoMode; +use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Password; use Symfony\Component\HttpKernel\Exception\HttpException; @@ -12,7 +17,7 @@ trait ViewAccountRecoveryChallengePageTests /** @test */ public function the_account_recovery_challenge_page_can_be_viewed(): void { - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $token = Password::getRepository()->create($user); $response = $this->get(route('recover-account.challenge', [ @@ -23,10 +28,36 @@ public function the_account_recovery_challenge_page_can_be_viewed(): void $response->assertOk(); } + /** @test */ + public function the_account_recovery_challenge_page_is_skipped_when_the_user_does_not_have_any_recovery_codes(): void + { + Carbon::setTestNow(now()); + Event::fake([AccountRecovered::class, AccountRecoveryFailed::class, SudoModeEnabled::class]); + $user = $this->generateUser(['recovery_codes' => null]); + $repository = Password::getRepository(); + $token = $repository->create($user); + $this->assertTrue($repository->exists($user, $token)); + + $response = $this->get(route('recover-account.challenge', [ + 'token' => $token, + 'email' => $user->getEmailForPasswordReset(), + ])); + + $response->assertRedirect(route('auth.settings')); + $response->assertSessionMissing(EnsureSudoMode::REQUIRED_AT_KEY); + $response->assertSessionHas(EnsureSudoMode::CONFIRMED_AT_KEY, now()->unix()); + $this->assertFullyAuthenticatedAs($response, $user); + $this->assertFalse($repository->exists($user, $token)); + Event::assertDispatched(AccountRecovered::class, fn ($event) => $event->user->is($user) && $event->request === request()); + Event::assertNotDispatched(AccountRecoveryFailed::class); + Event::assertNotDispatched(SudoModeEnabled::class); + Carbon::setTestNow(); + } + /** @test */ public function the_account_recovery_challenge_page_cannot_be_viewed_when_authenticated(): void { - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $response = $this->actingAs($user) ->get(route('recover-account.challenge', ['token' => 'foo'])); @@ -37,7 +68,7 @@ public function the_account_recovery_challenge_page_cannot_be_viewed_when_authen /** @test */ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_provided_email_does_not_resolve_to_an_existing_user(): void { - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $token = Password::getRepository()->create($user); $response = $this->get(route('recover-account.challenge', [ @@ -53,8 +84,8 @@ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_pr /** @test */ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_recovery_token_does_not_belong_to_the_user_that_is_being_recovered(): void { - $userA = $this->generateUser(['id' => 1, 'email' => 'claudio@ubient.net']); - $userB = $this->generateUser(['id' => 2, 'email' => 'another@example.com', $this->usernameField() => $this->anotherUsername()]); + $userA = $this->generateUser(['id' => 1, 'email' => 'claudio@ubient.net', 'recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); + $userB = $this->generateUser(['id' => 2, 'email' => 'another@example.com', $this->usernameField() => $this->anotherUsername(), 'recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $token = Password::getRepository()->create($userA); $response = $this->get(route('recover-account.challenge', [ @@ -70,7 +101,7 @@ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_re /** @test */ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_recovery_token_does_not_exist(): void { - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $response = $this->get(route('recover-account.challenge', [ 'token' => 'invalid-token', @@ -86,7 +117,7 @@ public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_re public function the_account_recovery_challenge_page_cannot_be_viewed_when_the_recovery_token_has_expired(): void { Carbon::setTestNow('2022-01-01 00:00:00'); - $user = $this->generateUser(); + $user = $this->generateUser(['recovery_codes' => ['H4PFK-ENVZV', 'PIPIM-7LTUT', 'GPP13-AEXMR', 'WGAHD-95VNQ', 'BSFYG-VFG2N', 'AWOPQ-NWYJX', '2PVJM-QHPBM', 'STR7J-5ND0P']]); $token = Password::getRepository()->create($user); Carbon::setTestNow(now()->addHour()->addSecond());