diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 8450ba31021..edf43935f3c 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -16,27 +16,6 @@ class PasswordsController < Devise::PasswordsController end end - # After a user resets their password, prompt for 2FA code if enabled instead - # of signing in automatically - # - # See http://git.io/vURrI - def update - super do |resource| - # TODO (rspeicher): In Devise master (> 3.4.1), we can set - # `Devise.sign_in_after_reset_password = false` and avoid this mess. - if resource.errors.empty? && resource.try(:two_factor_enabled?) - resource.unlock_access! if unlockable?(resource) - - # Since we are not signing this user in, we use the :updated_not_active - # message which only contains "Your password was changed successfully." - set_flash_message(:notice, :updated_not_active) if is_flashing_format? - - # Redirect to sign in so they can enter 2FA code - respond_with(resource, location: new_session_path(resource)) and return - end - end - end - def edit super reset_password_token = Devise.token_generator.digest( diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 2ce24592f8b..29506970af2 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -148,6 +148,10 @@ Devise.setup do |config| # When someone else invites you to GitLab this time is also used so it should be pretty long. config.reset_password_within = 2.days + # When set to false, does not sign a user in automatically after their password is + # reset. Defaults to true, so a user is signed in automatically after a reset. + config.sign_in_after_reset_password = false + # ==> Configuration for :encryptable # Allow you to use another encryption algorithm besides bcrypt (default). You can use # :sha1, :sha512 or encryptors from others authentication tools as :clearance_sha1, diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index 2b6311e4fd7..abf66f2356d 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -1,6 +1,35 @@ require 'spec_helper' feature 'Password reset', feature: true do + describe 'with two-factor authentication' do + let(:user) { create(:user, :two_factor) } + + it 'requires login after password reset' do + visit root_path + + forgot_password + reset_password + + expect(page).to have_content("Your password was changed successfully.") + expect(page).not_to have_content("You are now signed in.") + expect(current_path).to eq new_user_session_path + end + end + + describe 'without two-factor authentication' do + let(:user) { create(:user) } + + it 'requires login after password reset' do + visit root_path + + forgot_password + reset_password + + expect(page).to have_content("Your password was changed successfully.") + expect(current_path).to eq new_user_session_path + end + end + def forgot_password click_on 'Forgot your password?' fill_in 'Email', with: user.email @@ -21,33 +50,4 @@ feature 'Password reset', feature: true do fill_in 'Confirm new password', with: password click_button 'Change your password' end - - describe 'with two-factor authentication' do - let(:user) { create(:user, :two_factor) } - - it 'requires login after password reset' do - visit root_path - - forgot_password - reset_password - - expect(page).to have_content("Your password was changed successfully.") - expect(page).not_to have_content("You are now signed in.") - expect(current_path).to eq new_user_session_path - end - end - - describe 'without two-factor authentication' do - let(:user) { create(:user) } - - it 'automatically logs in after password reset' do - visit root_path - - forgot_password - reset_password - - expect(current_path).to eq root_path - expect(page).to have_content("Your password was changed successfully. You are now signed in.") - end - end end