Skip to content

Commit

Permalink
Prevented auto-login from direct email confirmation actions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ssddanbrown committed Nov 15, 2021
1 parent e29d03a commit 88e6f93
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 14 deletions.
3 changes: 1 addition & 2 deletions app/Http/Controllers/Auth/ConfirmEmailController.php
Expand Up @@ -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');
}

/**
Expand Down
10 changes: 3 additions & 7 deletions app/Http/Controllers/Auth/UserInviteController.php
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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');
}

/**
Expand Down
4 changes: 2 additions & 2 deletions resources/lang/en/auth.php
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions tests/Auth/AuthTest.php
Expand Up @@ -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]);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Auth/UserInviteTest.php
Expand Up @@ -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',
Expand Down

0 comments on commit 88e6f93

Please sign in to comment.