diff --git a/app/controllers/users/reset_passwords_controller.rb b/app/controllers/users/reset_passwords_controller.rb index 1a327f05a41..348989fe823 100644 --- a/app/controllers/users/reset_passwords_controller.rb +++ b/app/controllers/users/reset_passwords_controller.rb @@ -1,6 +1,8 @@ +# rubocop:disable Metrics/ClassLength module Users class ResetPasswordsController < Devise::PasswordsController include RecaptchaConcern + before_action :prevent_token_leakage, only: %i[edit] def new @password_reset_email_form = PasswordResetEmailForm.new('') @@ -34,7 +36,7 @@ def edit # PUT /resource/password def update - self.resource = user_matching_token(user_params[:reset_password_token]) + self.resource = user_matching_token(session[:reset_password_token]) @reset_password_form = ResetPasswordForm.new(resource) @@ -94,7 +96,14 @@ def user_matching_token(token) end def token_user - @_token_user ||= User.with_reset_password_token(params[:reset_password_token]) + @_token_user ||= User.with_reset_password_token(session[:reset_password_token]) + end + + def validated_token_from_url + reset_password_token = params[:reset_password_token] + return if reset_password_token.blank? + user = User.with_reset_password_token(reset_password_token) + user ? reset_password_token : nil end def build_user @@ -110,12 +119,14 @@ def handle_successful_password_reset redirect_to new_user_session_url EmailNotifier.new(resource).send_password_changed_email + session.delete(:reset_password_token) end def handle_unsuccessful_password_reset(result) if result.errors[:reset_password_token].present? flash[:error] = t('devise.passwords.token_expired') redirect_to new_user_password_url + session.delete(:reset_password_token) return end @@ -136,5 +147,20 @@ def user_params params.require(:reset_password_form). permit(:password, :reset_password_token) end + + def redirect_without_token_url(token) + session[:reset_password_token] = token + redirect_to url_for + end + + def prevent_token_leakage + token = validated_token_from_url + redirect_without_token_url(token) if token + end + + def assert_reset_token_passed + # remove devise's default behavior + end end end +# rubocop:enable Metrics/ClassLength diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index b12736df20c..d31353be89d 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -33,6 +33,7 @@ allow(user).to receive(:reset_password_period_valid?).and_return(false) get :edit, params: { reset_password_token: 'foo' } + get :edit analytics_hash = { success: false, @@ -65,6 +66,9 @@ get :edit, params: { reset_password_token: 'foo' } + expect(response).to redirect_to edit_user_password_url + + get :edit expect(response).to render_template :edit expect(flash.keys).to be_empty expect(response.body).to match('') @@ -87,8 +91,9 @@ reset_password_token: db_confirmation_token ) - params = { password: 'short', reset_password_token: raw_reset_token } + params = { password: 'short' } + get :edit, params: { reset_password_token: raw_reset_token } put :update, params: { reset_password_form: params } analytics_hash = { @@ -122,7 +127,7 @@ reset_password_token: db_confirmation_token, reset_password_sent_at: Time.zone.now ) - form_params = { password: 'short', reset_password_token: raw_reset_token } + form_params = { password: 'short' } analytics_hash = { success: false, errors: { password: ['is too short (minimum is 9 characters)'] }, @@ -134,6 +139,7 @@ expect(@analytics).to receive(:track_event). with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + get :edit, params: { reset_password_token: raw_reset_token } put :update, params: { reset_password_form: form_params } expect(response).to render_template(:edit) @@ -161,8 +167,9 @@ stub_email_notifier(user) password = 'a really long passw0rd' - params = { password: password, reset_password_token: raw_reset_token } + params = { password: password } + get :edit, params: { reset_password_token: raw_reset_token } put :update, params: { reset_password_form: params } analytics_hash = { @@ -199,8 +206,9 @@ stub_email_notifier(user) + get :edit, params: { reset_password_token: raw_reset_token } password = 'a really long passw0rd' - params = { password: password, reset_password_token: raw_reset_token } + params = { password: password } put :update, params: { reset_password_form: params } @@ -239,8 +247,9 @@ stub_email_notifier(user) password = 'a really long passw0rd' - params = { password: password, reset_password_token: raw_reset_token } + params = { password: password } + get :edit, params: { reset_password_token: raw_reset_token } put :update, params: { reset_password_form: params } analytics_hash = {