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

diff --git a/app/Http/Controllers/Auth/ConfirmEmailController.php b/app/Http/Controllers/Auth/ConfirmEmailController.php
index 3e7d4a8368974979970d71a25aaace5863be94c7..873d88475eb2bab2c3bcd8033136aa2ff2cce0bd 100644 (file)
--- a/app/Http/Controllers/Auth/ConfirmEmailController.php
+++ b/app/Http/Controllers/Auth/ConfirmEmailController.php
@@ -79,9 +79,8 @@ class ConfirmEmailController extends Controller
 
         $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 b3ccf072ad2e98513bb860d7e3b64d5255277b78..df8262e222e0ddd19ce2afaeff6e5096e2bd3c53 100644 (file)
--- 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 @@ use Illuminate\Routing\Redirector;
 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 @@ class UserInviteController extends Controller
         $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 ee0ba2d2150cd8c7c21de629871c41c284f1f223..107585bf0da5496aa23d58f0273ec9835bca9c8e 100644 (file)
--- a/resources/lang/en/auth.php
+++ b/resources/lang/en/auth.php
@@ -54,7 +54,7 @@ return [
     '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 @@ return [
     '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 50f56bfb9c6d8d49926d29b1737aa0e4d8866390..2c48131a885e559cf114e13c4d0e9cdfa6f8b59f 100644 (file)
--- a/tests/Auth/AuthTest.php
+++ b/tests/Auth/AuthTest.php
@@ -131,8 +131,8 @@ class AuthTest extends TestCase
         });
 
         // 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 dcf9e23df9b829a10cfd175800d058247c1333fe..1e1235f3361e34c9736fc0a4bf345b6420cad7e9 100644 (file)
--- a/tests/Auth/UserInviteTest.php
+++ b/tests/Auth/UserInviteTest.php
@@ -52,7 +52,7 @@ class UserInviteTest extends TestCase
         $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',