diff --git a/app/Http/Controllers/Auth/ForgotPasswordController.php b/app/Http/Controllers/Auth/ForgotPasswordController.php index a60fcc1c9..fadac641e 100644 --- a/app/Http/Controllers/Auth/ForgotPasswordController.php +++ b/app/Http/Controllers/Auth/ForgotPasswordController.php @@ -5,7 +5,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\SendsPasswordResetEmails; use Illuminate\Http\Request; -use Password; +use Illuminate\Support\Facades\Password; class ForgotPasswordController extends Controller { @@ -52,8 +52,8 @@ class ForgotPasswordController extends Controller $request->only('email') ); - if ($response === Password::RESET_LINK_SENT) { - $message = trans('auth.reset_password_sent_success', ['email' => $request->get('email')]); + if ($response === Password::RESET_LINK_SENT || $response === Password::INVALID_USER) { + $message = trans('auth.reset_password_sent', ['email' => $request->get('email')]); $this->showSuccessNotification($message); return back()->with('status', trans($response)); } diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index 214647cd5..efdf00159 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -5,6 +5,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\ResetsPasswords; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Password; class ResetPasswordController extends Controller { @@ -49,4 +50,24 @@ class ResetPasswordController extends Controller return redirect($this->redirectPath()) ->with('status', trans($response)); } + + /** + * Get the response for a failed password reset. + * + * @param \Illuminate\Http\Request $request + * @param string $response + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse + */ + protected function sendResetFailedResponse(Request $request, $response) + { + // We show invalid users as invalid tokens as to not leak what + // users may exist in the system. + if ($response === Password::INVALID_USER) { + $response = Password::INVALID_TOKEN; + } + + return redirect()->back() + ->withInput($request->only('email')) + ->withErrors(['email' => trans($response)]); + } } diff --git a/resources/js/components/notification.js b/resources/js/components/notification.js index f7edb08aa..35bab4ea6 100644 --- a/resources/js/components/notification.js +++ b/resources/js/components/notification.js @@ -28,7 +28,11 @@ class Notification { this.elem.classList.add('showing'); }, 1); - if (this.autohide) setTimeout(this.hide.bind(this), 2000); + if (this.autohide) { + const words = textToShow.split(' ').length; + const timeToShow = Math.max(2000, 1000 + (250 * words)); + setTimeout(this.hide.bind(this), timeToShow); + } } hide() { diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php index 6961e049b..d64fce93a 100644 --- a/resources/lang/en/auth.php +++ b/resources/lang/en/auth.php @@ -43,7 +43,7 @@ return [ 'reset_password' => 'Reset Password', 'reset_password_send_instructions' => 'Enter your email below and you will be sent an email with a password reset link.', 'reset_password_send_button' => 'Send Reset Link', - 'reset_password_sent_success' => 'A password reset link has been sent to :email.', + 'reset_password_sent' => 'A password reset link will be sent to :email if that email address is found in the system.', 'reset_password_success' => 'Your password has been successfully reset.', 'email_reset_subject' => 'Reset your :appName password', 'email_reset_text' => 'You are receiving this email because we received a password reset request for your account.', diff --git a/resources/lang/en/passwords.php b/resources/lang/en/passwords.php index f41ca7868..b408f3c2f 100644 --- a/resources/lang/en/passwords.php +++ b/resources/lang/en/passwords.php @@ -8,7 +8,7 @@ return [ 'password' => 'Passwords must be at least eight characters and match the confirmation.', 'user' => "We can't find a user with that e-mail address.", - 'token' => 'This password reset token is invalid.', + 'token' => 'The password reset token is invalid for this email address.', 'sent' => 'We have e-mailed your password reset link!', 'reset' => 'Your password has been reset!', diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index cb27de961..40bcda713 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -1,9 +1,13 @@ press('Resend Confirmation Email'); // Get confirmation and confirm notification matches - $emailConfirmation = \DB::table('email_confirmations')->where('user_id', '=', $dbUser->id)->first(); + $emailConfirmation = DB::table('email_confirmations')->where('user_id', '=', $dbUser->id)->first(); Notification::assertSentTo($dbUser, ConfirmEmail::class, function($notification, $channels) use ($emailConfirmation) { return $notification->token === $emailConfirmation->token; }); @@ -257,7 +261,7 @@ class AuthTest extends BrowserKitTest ->seePageIs('/settings/users'); $userPassword = User::find($user->id)->password; - $this->assertTrue(\Hash::check('newpassword', $userPassword)); + $this->assertTrue(Hash::check('newpassword', $userPassword)); } public function test_user_deletion() @@ -276,7 +280,7 @@ class AuthTest extends BrowserKitTest public function test_user_cannot_be_deleted_if_last_admin() { - $adminRole = \BookStack\Auth\Role::getRole('admin'); + $adminRole = Role::getRole('admin'); // Delete all but one admin user if there are more than one $adminUsers = $adminRole->users; @@ -309,14 +313,13 @@ class AuthTest extends BrowserKitTest public function test_reset_password_flow() { - Notification::fake(); $this->visit('/login')->click('Forgot Password?') ->seePageIs('/password/email') ->type('admin@admin.com', 'email') ->press('Send Reset Link') - ->see('A password reset link has been sent to admin@admin.com'); + ->see('A password reset link will be sent to admin@admin.com if that email address is found in the system.'); $this->seeInDatabase('password_resets', [ 'email' => 'admin@admin.com' @@ -324,8 +327,8 @@ class AuthTest extends BrowserKitTest $user = User::where('email', '=', 'admin@admin.com')->first(); - Notification::assertSentTo($user, \BookStack\Notifications\ResetPassword::class); - $n = Notification::sent($user, \BookStack\Notifications\ResetPassword::class); + Notification::assertSentTo($user, ResetPassword::class); + $n = Notification::sent($user, ResetPassword::class); $this->visit('/password/reset/' . $n->first()->token) ->see('Reset Password') @@ -337,6 +340,28 @@ class AuthTest extends BrowserKitTest ->see('Your password has been successfully reset'); } + public function test_reset_password_flow_shows_success_message_even_if_wrong_password_to_prevent_user_discovery() + { + $this->visit('/login')->click('Forgot Password?') + ->seePageIs('/password/email') + ->type('barry@admin.com', 'email') + ->press('Send Reset Link') + ->see('A password reset link will be sent to barry@admin.com if that email address is found in the system.') + ->dontSee('We can\'t find a user'); + + + $this->visit('/password/reset/arandometokenvalue') + ->see('Reset Password') + ->submitForm('Reset Password', [ + 'email' => 'barry@admin.com', + 'password' => 'randompass', + 'password_confirmation' => 'randompass' + ])->followRedirects() + ->seePageIs('/password/reset/arandometokenvalue') + ->dontSee('We can\'t find a user') + ->see('The password reset token is invalid for this email address.'); + } + public function test_reset_password_page_shows_sign_links() { $this->setSettings(['registration-enabled' => 'true']);