diff --git a/app/controllers/alter_passwords_controller.rb b/app/controllers/alter_passwords_controller.rb index f22c9ea11..114e64813 100644 --- a/app/controllers/alter_passwords_controller.rb +++ b/app/controllers/alter_passwords_controller.rb @@ -7,7 +7,8 @@ class AlterPasswordsController < ApplicationController def update @account.validate_current_password = true - if @account.update(account_params) + if update_password_and_rotate_token + clearance_session.sign_in(@account) redirect_to account_path, flash: { success: t('.password_changed') } else render :edit, status: :unprocessable_entity @@ -29,6 +30,20 @@ def must_own_account access_denied end + def update_password_and_rotate_token + success = false + ActiveRecord::Base.transaction do + raise ActiveRecord::Rollback unless @account.update(account_params) + + @account.reset_remember_token! + success = true + end + success + rescue ActiveRecord::ActiveRecordError => e + Airbrake.notify(e) + false + end + def account_params params.require(:account).permit(:current_password, :password) end diff --git a/app/models/concerns/account_validations.rb b/app/models/concerns/account_validations.rb index 27a2a71f8..2a80a373e 100644 --- a/app/models/concerns/account_validations.rb +++ b/app/models/concerns/account_validations.rb @@ -53,9 +53,9 @@ def about_raw_not_blocked_domain end def valid_current_password? - return false if current_password_matches_existing? && access.active_and_not_disabled? - - errors.add(:current_password) + valid = current_password_matches_existing? && access.active_and_not_disabled? + errors.add(:current_password) unless valid + valid end # Use _was since encrypted_password & salt have already changed in password=. diff --git a/test/controllers/alter_passwords_controller_test.rb b/test/controllers/alter_passwords_controller_test.rb index d1bea7b03..bef95ce78 100644 --- a/test/controllers/alter_passwords_controller_test.rb +++ b/test/controllers/alter_passwords_controller_test.rb @@ -62,6 +62,20 @@ class AlterPasswordsControllerTest < ActionController::TestCase _(flash[:success]).must_equal 'Password successfully changed.' end + it 'must rotate remember_token on successful password change' do + oldpassword = PasswordGenerator.generate + newpassword = PasswordGenerator.generate + account = create(:account, password: oldpassword) + original_token = account.remember_token + _(original_token).wont_be_nil + login_as account + put :update, + params: { id: account.login, account: { current_password: oldpassword, password: newpassword } } + account.reload + # Token should be rotated — old sessions are invalidated + _(account.remember_token).wont_equal original_token + end + describe 'authentication and authorization' do it 'must redirect to login when not authenticated' do account = create(:account)