From 88e6f93abf54192a69cc8080e0dc6516ee68ccbb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 15 Nov 2021 10:50:28 +0000 Subject: [PATCH] Prevented auto-login from direct email confirmation actions Was done for convenience but could potentially be exploited by an attacker using signing up via one of these routes, then forwarding an email confirmation to another user so they unknowingly utilise an account someone else controls. Tweaks the flow of confirming email, and the user invite flow. For #3050 --- app/Http/Controllers/Auth/ConfirmEmailController.php | 3 +-- app/Http/Controllers/Auth/UserInviteController.php | 10 +++------- resources/lang/en/auth.php | 4 ++-- tests/Auth/AuthTest.php | 4 ++-- tests/Auth/UserInviteTest.php | 2 +- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Auth/ConfirmEmailController.php b/app/Http/Controllers/Auth/ConfirmEmailController.php index 3e7d4a83689..873d88475eb 100644 --- a/app/Http/Controllers/Auth/ConfirmEmailController.php +++ b/app/Http/Controllers/Auth/ConfirmEmailController.php @@ -79,9 +79,8 @@ public function confirm(string $token) $this->emailConfirmationService->deleteByUser($user); $this->showSuccessNotification(trans('auth.email_confirm_success')); - $this->loginService->login($user, auth()->getDefaultDriver()); - return redirect('/'); + return redirect('/login'); } /** diff --git a/app/Http/Controllers/Auth/UserInviteController.php b/app/Http/Controllers/Auth/UserInviteController.php index b3ccf072ad2..df8262e222e 100644 --- a/app/Http/Controllers/Auth/UserInviteController.php +++ b/app/Http/Controllers/Auth/UserInviteController.php @@ -2,7 +2,6 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\UserInviteService; use BookStack\Auth\UserRepo; use BookStack\Exceptions\UserTokenExpiredException; @@ -16,19 +15,17 @@ class UserInviteController extends Controller { protected $inviteService; - protected $loginService; protected $userRepo; /** * Create a new controller instance. */ - public function __construct(UserInviteService $inviteService, LoginService $loginService, UserRepo $userRepo) + public function __construct(UserInviteService $inviteService, UserRepo $userRepo) { $this->middleware('guest'); $this->middleware('guard:standard'); $this->inviteService = $inviteService; - $this->loginService = $loginService; $this->userRepo = $userRepo; } @@ -73,10 +70,9 @@ public function setPassword(Request $request, string $token) $user->save(); $this->inviteService->deleteByUser($user); - $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')])); - $this->loginService->login($user, auth()->getDefaultDriver()); + $this->showSuccessNotification(trans('auth.user_invite_success_login', ['appName' => setting('app-name')])); - return redirect('/'); + return redirect('/login'); } /** diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php index ee0ba2d2150..107585bf0da 100644 --- a/resources/lang/en/auth.php +++ b/resources/lang/en/auth.php @@ -54,7 +54,7 @@ 'email_confirm_text' => 'Please confirm your email address by clicking the button below:', 'email_confirm_action' => 'Confirm Email', 'email_confirm_send_error' => 'Email confirmation required but the system could not send the email. Contact the admin to ensure email is set up correctly.', - 'email_confirm_success' => 'Your email has been confirmed!', + 'email_confirm_success' => 'Your email has been confirmed! You should now be able to login using this email address.', 'email_confirm_resent' => 'Confirmation email resent, Please check your inbox.', 'email_not_confirmed' => 'Email Address Not Confirmed', @@ -71,7 +71,7 @@ 'user_invite_page_welcome' => 'Welcome to :appName!', 'user_invite_page_text' => 'To finalise your account and gain access you need to set a password which will be used to log-in to :appName on future visits.', 'user_invite_page_confirm_button' => 'Confirm Password', - 'user_invite_success' => 'Password set, you now have access to :appName!', + 'user_invite_success_login' => 'Password set, you should now be able to login using your set password to access :appName!', // Multi-factor Authentication 'mfa_setup' => 'Setup Multi-Factor Authentication', diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index 50f56bfb9c6..2c48131a885 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -131,8 +131,8 @@ public function test_confirmed_registration() }); // Check confirmation email confirmation activation. - $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/'); - $this->get('/')->assertSee($user->name); + $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/login'); + $this->get('/login')->assertSee('Your email has been confirmed! You should now be able to login using this email address.'); $this->assertDatabaseMissing('email_confirmations', ['token' => $emailConfirmation->token]); $this->assertDatabaseHas('users', ['name' => $dbUser->name, 'email' => $dbUser->email, 'email_confirmed' => true]); } diff --git a/tests/Auth/UserInviteTest.php b/tests/Auth/UserInviteTest.php index dcf9e23df9b..1e1235f3361 100644 --- a/tests/Auth/UserInviteTest.php +++ b/tests/Auth/UserInviteTest.php @@ -52,7 +52,7 @@ public function test_invite_set_password() $setPasswordResp = $this->followingRedirects()->post('/register/invite/' . $token, [ 'password' => 'my test password', ]); - $setPasswordResp->assertSee('Password set, you now have access to BookStack!'); + $setPasswordResp->assertSee('Password set, you should now be able to login using your set password to access BookStack!'); $newPasswordValid = auth()->validate([ 'email' => $user->email, 'password' => 'my test password',