From 18d57973ce452e9fcdba9dd04524be9a2d298046 Mon Sep 17 00:00:00 2001 From: Alfonso Bribiesca Date: Tue, 4 May 2021 05:32:53 -0500 Subject: [PATCH] feat: 2auth reset password workflow (#133) --- config/fortify.php | 3 +- resources/lang/en/validation.php | 5 +- resources/views/auth/reset-password.blade.php | 2 +- .../views/auth/two-factor-challenge.blade.php | 16 +- src/Components/ResetPasswordForm.php | 16 +- src/FortifyServiceProvider.php | 20 +-- ...orAuthenticatedPasswordResetController.php | 62 +++++++ .../TwoFactorResetPasswordRequest.php | 62 +++++++ tests/Components/ResetPasswordFormTest.php | 20 +++ ...thenticatedPasswordResetControllerTest.php | 149 +++++++++++++++++ .../TwoFactorResetPasswordRequestTest.php | 151 ++++++++++++++++++ 11 files changed, 486 insertions(+), 20 deletions(-) create mode 100644 src/Http/Controllers/TwoFactorAuthenticatedPasswordResetController.php create mode 100644 src/Http/Requests/TwoFactorResetPasswordRequest.php create mode 100644 tests/Http/Controllers/TwoFactorAuthenticatedPasswordResetControllerTest.php create mode 100644 tests/Http/Requests/TwoFactorResetPasswordRequestTest.php diff --git a/config/fortify.php b/config/fortify.php index fc58568c..8b7c5d9b 100644 --- a/config/fortify.php +++ b/config/fortify.php @@ -168,7 +168,8 @@ */ 'routes' => [ - 'feedback_thank_you' => env('ROUTE_FEEDBACK_THANK_YOU', '/feedback/thank-you'), + 'feedback_thank_you' => env('ROUTE_FEEDBACK_THANK_YOU', '/feedback/thank-you'), + 'two_factor_reset_password' => env('ROUTE_TWO_RESET_PASSWORD', '/two-factor/reset-password/{token}'), ], ]; diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 17845cc7..88c3c4f4 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -3,8 +3,9 @@ declare(strict_types=1); return [ - 'password_doesnt_match' => 'The provided password does not match your current password.', - 'password_doesnt_match_records' => 'This password does not match our records.', + 'password_doesnt_match' => 'The provided password does not match your current password.', + 'password_doesnt_match_records' => 'This password does not match our records.', + 'password_reset_link_invalid' => 'Your password reset link expired or is invalid.', 'messages' => [ 'one_time_password' => 'We were not able to enable two-factor authentication with this one-time password.', diff --git a/resources/views/auth/reset-password.blade.php b/resources/views/auth/reset-password.blade.php index 0615c720..b22f61c6 100644 --- a/resources/views/auth/reset-password.blade.php +++ b/resources/views/auth/reset-password.blade.php @@ -17,6 +17,6 @@
- +
@endsection diff --git a/resources/views/auth/two-factor-challenge.blade.php b/resources/views/auth/two-factor-challenge.blade.php index 453e0c14..bd0ba1b2 100644 --- a/resources/views/auth/two-factor-challenge.blade.php +++ b/resources/views/auth/two-factor-challenge.blade.php @@ -26,15 +26,21 @@ x-cloak class="max-w-xl p-8 mx-auto" > + +
@csrf
+ @isset($resetPassword) + + @endisset +
@csrf
+ @isset($resetPassword) + + @endisset +
'', 'password' => '', 'password_confirmation' => '', ]; - public function mount() + public function mount(?string $token = null, ?string $email = null) { - $this->token = request()->route('token'); - $this->state['email'] = old('email', request()->email); + $this->token = $token; + + if ($email !== null) { + $this->state['email'] = $email; + + $user = Models::user()::where('email', $email)->firstOrFail(); + + $this->twoFactorSecret = $user->two_factor_secret; + } } /** diff --git a/src/FortifyServiceProvider.php b/src/FortifyServiceProvider.php index eeacaebf..2f02cb32 100644 --- a/src/FortifyServiceProvider.php +++ b/src/FortifyServiceProvider.php @@ -21,6 +21,7 @@ use ARKEcosystem\Fortify\Components\UpdateProfilePhotoForm; use ARKEcosystem\Fortify\Components\UpdateTimezoneForm; use ARKEcosystem\Fortify\Components\VerifyEmail; +use ARKEcosystem\Fortify\Http\Controllers\TwoFactorAuthenticatedPasswordResetController; use ARKEcosystem\Fortify\Http\Responses\FailedPasswordResetLinkRequestResponse as FortifyFailedPasswordResetLinkRequestResponse; use ARKEcosystem\Fortify\Http\Responses\SuccessfulPasswordResetLinkRequestResponse as FortifySuccessfulPasswordResetLinkRequestResponse; use ARKEcosystem\Fortify\Responses\FailedTwoFactorLoginResponse; @@ -180,16 +181,7 @@ private function registerViews(): void $user = Models::user()::where('email', $request->get('email'))->firstOrFail(); if ($user->two_factor_secret) { - if (! $request->session()->get('errors')) { - $request->session()->put([ - 'login.idFailure' => $user->getKey(), - 'login.id' => $user->getKey(), - 'login.remember' => true, - 'url.intended' => route('account.settings.password'), - ]); - - return redirect()->route('two-factor.login'); - } + return redirect()->route('two-factor.reset-password', ['token' => $request->token, 'email' => $user->email]); } return view('ark-fortify::auth.reset-password', ['request' => $request]); @@ -255,6 +247,14 @@ public function registerRoutes(): void Route::view(config('fortify.routes.feedback_thank_you'), 'ark-fortify::profile.feedback-thank-you') ->name('profile.feedback.thank-you') ->middleware('signed'); + + Route::get(config('fortify.routes.two_factor_reset_password'), [TwoFactorAuthenticatedPasswordResetController::class, 'create']) + ->name('two-factor.reset-password') + ->middleware('guest'); + + Route::post(config('fortify.routes.two_factor_reset_password'), [TwoFactorAuthenticatedPasswordResetController::class, 'store']) + ->name('two-factor.reset-password-store') + ->middleware('guest'); }); } } diff --git a/src/Http/Controllers/TwoFactorAuthenticatedPasswordResetController.php b/src/Http/Controllers/TwoFactorAuthenticatedPasswordResetController.php new file mode 100644 index 00000000..e4499e1b --- /dev/null +++ b/src/Http/Controllers/TwoFactorAuthenticatedPasswordResetController.php @@ -0,0 +1,62 @@ +hasChallengedUser()) { + throw new HttpResponseException(redirect()->route('login')); + } + + if (! $request->hasValidToken()) { + throw new HttpResponseException(redirect()->route('login')->withErrors(['email' => trans('fortify::validation.password_reset_link_invalid')])); + } + + return view('ark-fortify::auth.two-factor-challenge', [ + 'token' => $token, + 'resetPassword' => true, + 'email' => $request->challengedUser()->email, + ]); + } + + /** + * Validates the 2fa code and shows the reset password form. + * + * @param TwoFactorResetPasswordRequest $request + * + * @return mixed + */ + public function store(TwoFactorResetPasswordRequest $request) + { + $user = $request->challengedUser(); + + if (! $request->hasValidToken()) { + throw new HttpResponseException(redirect()->route('login')->withErrors(['email' => trans('fortify::validation.password_reset_link_invalid')])); + } + + if ($code = $request->validRecoveryCode()) { + $user->replaceRecoveryCode($code); + } elseif (! $request->hasValidCode()) { + return app(FailedTwoFactorLoginResponse::class); + } + + return view('ark-fortify::auth.reset-password'); + } +} diff --git a/src/Http/Requests/TwoFactorResetPasswordRequest.php b/src/Http/Requests/TwoFactorResetPasswordRequest.php new file mode 100644 index 00000000..f99e736f --- /dev/null +++ b/src/Http/Requests/TwoFactorResetPasswordRequest.php @@ -0,0 +1,62 @@ +challengedUser(); + + return $user && app(PasswordBroker::class)->tokenExists($user, $this->route('token')); + } + + /** + * Determine if there is a challenged user in the current session. + * + * @return bool + */ + public function hasChallengedUser() + { + $model = app(StatefulGuard::class)->getProvider()->getModel(); + + return $this->has('email') && + $model::whereEmail($this->get('email'))->exists(); + } + + /** + * Get the user that is attempting the two factor challenge. + * + * @return mixed + */ + public function challengedUser() + { + if ($this->challengedUser) { + return $this->challengedUser; + } + + $model = app(StatefulGuard::class)->getProvider()->getModel(); + + if (! $this->has('email') || + ! $user = $model::whereEmail($this->get('email'))->first()) { + throw new HttpResponseException( + app(FailedTwoFactorLoginResponse::class)->toResponse($this) + ); + } + + return $this->challengedUser = $user; + } +} diff --git a/tests/Components/ResetPasswordFormTest.php b/tests/Components/ResetPasswordFormTest.php index a12a7396..bd6e87a5 100644 --- a/tests/Components/ResetPasswordFormTest.php +++ b/tests/Components/ResetPasswordFormTest.php @@ -5,6 +5,7 @@ namespace Tests\Components; use ARKEcosystem\Fortify\Components\ResetPasswordForm; +use Illuminate\Support\Facades\Config; use Livewire\Livewire; use function Tests\createUserModel; @@ -20,3 +21,22 @@ ]) ->assertViewIs('ark-fortify::auth.reset-password-form'); }); + +it('gets the two factor code and the email', function () { + Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class); + + $user = createUserModel(); + + $user->two_factor_secret = 'secret'; + $user->save(); + + Livewire::actingAs($user) + ->test(ResetPasswordForm::class, ['email' => $user->email]) + ->assertSet('state', [ + 'email' => $user->email, + 'password' => '', + 'password_confirmation' => '', + ]) + ->assertSet('twoFactorSecret', 'secret') + ->assertViewIs('ark-fortify::auth.reset-password-form'); +}); diff --git a/tests/Http/Controllers/TwoFactorAuthenticatedPasswordResetControllerTest.php b/tests/Http/Controllers/TwoFactorAuthenticatedPasswordResetControllerTest.php new file mode 100644 index 00000000..8d4f3760 --- /dev/null +++ b/tests/Http/Controllers/TwoFactorAuthenticatedPasswordResetControllerTest.php @@ -0,0 +1,149 @@ +mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('hasChallengedUser') + ->andReturn(true) + ->shouldReceive('hasValidToken') + ->andReturn(true) + ->shouldReceive('challengedUser') + ->andReturn($user); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $response = $controller->create($request, $token); + expect($response)->toBeInstanceOf(View::class); + expect($response->getName())->toBe('ark-fortify::auth.two-factor-challenge'); + expect($response->getData())->toEqual([ + 'token' => $token, + 'resetPassword' => true, + 'email' => $user->email, + ]); +}); + +it('throws an http exception if the token is invalid', function () { + $this->expectException(HttpResponseException::class); + + createUserModel(); + + $this->mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('hasChallengedUser') + ->andReturn(true) + ->shouldReceive('hasValidToken') + ->andReturn(false); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $controller->create($request, $token); +}); + +it('throws an http exception if is not able to get the user', function () { + $this->expectException(HttpResponseException::class); + + createUserModel(); + + $request = $this->mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('hasChallengedUser') + ->andReturn(false); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $controller->create($request, $token); +}); + +it('shows the reset form after validating the token', function () { + $user = createUserModel(); + + $user->two_factor_recovery_codes = encrypt('wharever'); + $user->save(); + + $this->mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('hasChallengedUser') + ->andReturn(true) + ->shouldReceive('hasValidToken') + ->andReturn(true) + ->shouldReceive('challengedUser') + ->andReturn($user) + ->shouldReceive('validRecoveryCode') + ->andReturn(encrypt('the_code')); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $response = $controller->store($request, $token); + expect($response)->toBeInstanceOf(View::class); + expect($response->getName())->toBe('ark-fortify::auth.reset-password'); +}); + +it('throws an http exception if the token is invalid when validating 2fa', function () { + $this->expectException(HttpResponseException::class); + + $user = createUserModel(); + + $this->mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('challengedUser') + ->andReturn($user) + ->shouldReceive('hasChallengedUser') + ->andReturn(true) + ->shouldReceive('hasValidToken') + ->andReturn(false); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $controller->store($request, $token); +}); + +it('returns a failed two factor login response if the code is invalid', function () { + $user = createUserModel(); + + $this->mock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('challengedUser') + ->andReturn($user) + ->shouldReceive('hasChallengedUser') + ->andReturn(true) + ->shouldReceive('hasValidToken') + ->andReturn(true) + ->shouldReceive('validRecoveryCode') + ->andReturn(false) + ->shouldReceive('hasValidCode') + ->andReturn(false); + + $request = app(TwoFactorResetPasswordRequest::class); + + $controller = app(TwoFactorAuthenticatedPasswordResetController::class); + + $token = 'abcd'; + + $response = $controller->store($request, $token); + expect($response)->toBeInstanceOf(FailedTwoFactorLoginResponse::class); +}); diff --git a/tests/Http/Requests/TwoFactorResetPasswordRequestTest.php b/tests/Http/Requests/TwoFactorResetPasswordRequestTest.php new file mode 100644 index 00000000..62725cec --- /dev/null +++ b/tests/Http/Requests/TwoFactorResetPasswordRequestTest.php @@ -0,0 +1,151 @@ +mock(PasswordBroker::class) + ->shouldReceive('tokenExists') + ->with($user, $token) + ->andReturn(true); + + $this->partialMock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('route') + ->with('token') + ->andReturn($token) + ->shouldReceive('challengedUser') + ->andReturn($user); + + $request = app(TwoFactorResetPasswordRequest::class); + + expect($request->hasValidToken())->toBeTrue(); + + $this->mock(PasswordBroker::class) + ->shouldReceive('tokenExists') + ->with($user, $token) + ->andReturn(false); + + $request = app(TwoFactorResetPasswordRequest::class); + + expect($request->hasValidToken())->toBeFalse(); +}); + +it('validates the user', function () { + $user = createUserModel(); + + $this->partialMock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('get') + ->with('email') + ->andReturn($user->email) + ->shouldReceive('has') + ->with('email') + ->andReturn(true); + + $this->mock(UserProvider::class) + ->shouldReceive('getModel') + ->andReturn(User::class); + + $this->mock(StatefulGuard::class) + ->shouldReceive('getProvider') + ->andReturn(app(UserProvider::class)); + + $request = app(TwoFactorResetPasswordRequest::class); + + expect($request->hasChallengedUser())->toBeTrue(); +}); + +it('validates not existing user', function () { + createUserModel(); + + $this->partialMock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('get') + ->with('email') + ->andReturn('other@example.com') + ->shouldReceive('has') + ->with('email') + ->andReturn(true); + + $this->mock(UserProvider::class) + ->shouldReceive('getModel') + ->andReturn(User::class); + + $this->mock(StatefulGuard::class) + ->shouldReceive('getProvider') + ->andReturn(app(UserProvider::class)); + + $request = app(TwoFactorResetPasswordRequest::class); + + expect($request->hasChallengedUser())->toBeFalse(); +}); + +it('gets the challenged user', function () { + $user = createUserModel(); + + $this->partialMock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('get') + ->with('email') + ->andReturn($user->email) + ->shouldReceive('has') + ->with('email') + ->andReturn(true); + + $this->mock(UserProvider::class) + ->shouldReceive('getModel') + ->andReturn(User::class); + + $this->mock(StatefulGuard::class) + ->shouldReceive('getProvider') + ->andReturn(app(UserProvider::class)); + + $request = app(TwoFactorResetPasswordRequest::class); + + expect($request->challengedUser()->is($user))->toBeTrue(); + + // A second time, now it comes from the protected property + expect($request->challengedUser()->is($user))->toBeTrue(); +}); + +it('throws an exception if email not found', function () { + $this->expectException(HttpResponseException::class); + + createUserModel(); + + $this->partialMock(TwoFactorResetPasswordRequest::class) + ->shouldReceive('get') + ->with('email') + ->andReturn('other@example.com') + ->shouldReceive('has') + ->with('email') + ->andReturn(true) + ->shouldReceive('only') + ->andReturn([]) + ->shouldReceive('wantsJson') + ->andReturn(false); + + $this->mock(UserProvider::class) + ->shouldReceive('getModel') + ->andReturn(User::class); + + $this->mock(StatefulGuard::class) + ->shouldReceive('getProvider') + ->andReturn(app(UserProvider::class)); + + $this->mock(FailedTwoFactorLoginResponse::class) + ->shouldReceive('toResponse') + ->andReturn(new \Symfony\Component\HttpFoundation\Response()); + + $request = app(TwoFactorResetPasswordRequest::class); + + $request->challengedUser(); +});