From d133f904d369cf2ca964a5c9f54e4e19843507c2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 20 May 2024 17:23:15 +0100 Subject: [PATCH] Auth: Changed email confirmations to use login attempt user Negates the need for a public confirmation resend form since we can instead just send direct to the last session login attempter. --- .../Controllers/ConfirmEmailController.php | 25 +++++--- .../Controllers/HandlesPartialLogins.php | 2 +- app/Access/EmailConfirmationService.php | 2 +- .../StoppedAuthenticationException.php | 14 ++--- lang/en/errors.php | 1 + ...hp => register-confirm-awaiting.blade.php} | 10 +--- tests/Auth/RegistrationTest.php | 58 +++++++++++++++++++ 7 files changed, 83 insertions(+), 29 deletions(-) rename resources/views/auth/{user-unconfirmed.blade.php => register-confirm-awaiting.blade.php} (66%) diff --git a/app/Access/Controllers/ConfirmEmailController.php b/app/Access/Controllers/ConfirmEmailController.php index 94647e06e..d71b8f450 100644 --- a/app/Access/Controllers/ConfirmEmailController.php +++ b/app/Access/Controllers/ConfirmEmailController.php @@ -32,13 +32,17 @@ class ConfirmEmailController extends Controller /** * Shows a notice that a user's email address has not been confirmed, - * Also has the option to re-send the confirmation email. + * along with the option to re-send the confirmation email. */ public function showAwaiting() { $user = $this->loginService->getLastLoginAttemptUser(); + if ($user === null) { + $this->showErrorNotification(trans('errors.login_user_not_found')); + return redirect('/login'); + } - return view('auth.user-unconfirmed', ['user' => $user]); + return view('auth.register-confirm-awaiting'); } /** @@ -90,19 +94,24 @@ class ConfirmEmailController extends Controller /** * Resend the confirmation email. */ - public function resend(Request $request) + public function resend() { - $this->validate($request, [ - 'email' => ['required', 'email', 'exists:users,email'], - ]); - $user = $this->userRepo->getByEmail($request->get('email')); + $user = $this->loginService->getLastLoginAttemptUser(); + if ($user === null) { + $this->showErrorNotification(trans('errors.login_user_not_found')); + return redirect('/login'); + } try { $this->emailConfirmationService->sendConfirmation($user); + } catch (ConfirmationEmailException $e) { + $this->showErrorNotification($e->getMessage()); + + return redirect('/login'); } catch (Exception $e) { $this->showErrorNotification(trans('auth.email_confirm_send_error')); - return redirect('/register/confirm'); + return redirect('/register/awaiting'); } $this->showSuccessNotification(trans('auth.email_confirm_resent')); diff --git a/app/Access/Controllers/HandlesPartialLogins.php b/app/Access/Controllers/HandlesPartialLogins.php index c5554e473..47a63d19b 100644 --- a/app/Access/Controllers/HandlesPartialLogins.php +++ b/app/Access/Controllers/HandlesPartialLogins.php @@ -17,7 +17,7 @@ trait HandlesPartialLogins $user = auth()->user() ?? $loginService->getLastLoginAttemptUser(); if (!$user) { - throw new NotFoundException('A user for this action could not be found'); + throw new NotFoundException(trans('errors.login_user_not_found')); } return $user; diff --git a/app/Access/EmailConfirmationService.php b/app/Access/EmailConfirmationService.php index de9ea69b8..1a5156d3e 100644 --- a/app/Access/EmailConfirmationService.php +++ b/app/Access/EmailConfirmationService.php @@ -17,7 +17,7 @@ class EmailConfirmationService extends UserTokenService * * @throws ConfirmationEmailException */ - public function sendConfirmation(User $user) + public function sendConfirmation(User $user): void { if ($user->email_confirmed) { throw new ConfirmationEmailException(trans('errors.email_already_confirmed'), '/login'); diff --git a/app/Exceptions/StoppedAuthenticationException.php b/app/Exceptions/StoppedAuthenticationException.php index 51e48aa1c..8a917bc52 100644 --- a/app/Exceptions/StoppedAuthenticationException.php +++ b/app/Exceptions/StoppedAuthenticationException.php @@ -9,16 +9,10 @@ use Illuminate\Http\Request; class StoppedAuthenticationException extends \Exception implements Responsable { - protected $user; - protected $loginService; - - /** - * StoppedAuthenticationException constructor. - */ - public function __construct(User $user, LoginService $loginService) - { - $this->user = $user; - $this->loginService = $loginService; + public function __construct( + protected User $user, + protected LoginService $loginService + ) { parent::__construct(); } diff --git a/lang/en/errors.php b/lang/en/errors.php index 8773a78cb..752eb5672 100644 --- a/lang/en/errors.php +++ b/lang/en/errors.php @@ -37,6 +37,7 @@ return [ 'social_driver_not_found' => 'Social driver not found', 'social_driver_not_configured' => 'Your :socialAccount social settings are not configured correctly.', 'invite_token_expired' => 'This invitation link has expired. You can instead try to reset your account password.', + 'login_user_not_found' => 'A user for this action could not be found.', // System 'path_not_writable' => 'File path :filePath could not be uploaded to. Ensure it is writable to the server.', diff --git a/resources/views/auth/user-unconfirmed.blade.php b/resources/views/auth/register-confirm-awaiting.blade.php similarity index 66% rename from resources/views/auth/user-unconfirmed.blade.php rename to resources/views/auth/register-confirm-awaiting.blade.php index 2f780b8a3..e5d6f61b2 100644 --- a/resources/views/auth/user-unconfirmed.blade.php +++ b/resources/views/auth/register-confirm-awaiting.blade.php @@ -14,15 +14,7 @@

- {!! csrf_field() !!} -
- - @if($user) - @include('form.text', ['name' => 'email', 'model' => $user]) - @else - @include('form.text', ['name' => 'email']) - @endif -
+ {{ csrf_field() }}
diff --git a/tests/Auth/RegistrationTest.php b/tests/Auth/RegistrationTest.php index 42d1120e4..2666fa3b4 100644 --- a/tests/Auth/RegistrationTest.php +++ b/tests/Auth/RegistrationTest.php @@ -25,6 +25,9 @@ class RegistrationTest extends TestCase $resp->assertRedirect('/register/confirm'); $this->assertDatabaseHas('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]); + $resp = $this->get('/register/confirm'); + $resp->assertSee('Thanks for registering!'); + // Ensure notification sent /** @var User $dbUser */ $dbUser = User::query()->where('email', '=', $user->email)->first(); @@ -232,4 +235,59 @@ class RegistrationTest extends TestCase $response->assertStatus(429); } + + public function test_registration_confirmation_resend() + { + Notification::fake(); + $this->setSettings(['registration-enabled' => 'true', 'registration-confirmation' => 'true']); + $user = User::factory()->make(); + + $resp = $this->post('/register', $user->only('name', 'email', 'password')); + $resp->assertRedirect('/register/confirm'); + $dbUser = User::query()->where('email', '=', $user->email)->first(); + + $resp = $this->post('/login', ['email' => $user->email, 'password' => $user->password]); + $resp->assertRedirect('/register/confirm/awaiting'); + + $resp = $this->post('/register/confirm/resend'); + $resp->assertRedirect('/register/confirm'); + Notification::assertSentToTimes($dbUser, ConfirmEmailNotification::class, 2); + } + + public function test_registration_confirmation_expired_resend() + { + Notification::fake(); + $this->setSettings(['registration-enabled' => 'true', 'registration-confirmation' => 'true']); + $user = User::factory()->make(); + + $resp = $this->post('/register', $user->only('name', 'email', 'password')); + $resp->assertRedirect('/register/confirm'); + $dbUser = User::query()->where('email', '=', $user->email)->first(); + + $resp = $this->post('/login', ['email' => $user->email, 'password' => $user->password]); + $resp->assertRedirect('/register/confirm/awaiting'); + + $emailConfirmation = DB::table('email_confirmations')->where('user_id', '=', $dbUser->id)->first(); + $this->travel(2)->days(); + + $resp = $this->post("/register/confirm/accept", [ + 'token' => $emailConfirmation->token, + ]); + $resp->assertRedirect('/register/confirm'); + $this->assertSessionError('The confirmation token has expired, A new confirmation email has been sent.'); + + Notification::assertSentToTimes($dbUser, ConfirmEmailNotification::class, 2); + } + + public function test_registration_confirmation_awaiting_and_resend_returns_to_log_if_no_login_attempt_user_found() + { + $this->setSettings(['registration-enabled' => 'true', 'registration-confirmation' => 'true']); + + $this->get('/register/confirm/awaiting')->assertRedirect('/login'); + $this->assertSessionError('A user for this action could not be found.'); + $this->flushSession(); + + $this->post('/register/confirm/resend')->assertRedirect('/login'); + $this->assertSessionError('A user for this action could not be found.'); + } }