From 292bca0546c59b9816c696371cd9bbf04ba19fb2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 30 Sep 2015 15:38:21 -0400 Subject: Only allow password reset emails once per minute Addresses internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2611 --- app/controllers/passwords_controller.rb | 22 +++++++++++------ spec/features/password_reset_spec.rb | 43 +++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index edf43935f3c..a2d152addc9 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -2,18 +2,19 @@ class PasswordsController < Devise::PasswordsController def create email = resource_params[:email] - resource_found = resource_class.find_by_email(email) - if resource_found && resource_found.ldap_user? + self.resource = resource_class.find_by_email(email) + + if resource && resource.ldap_user? flash[:alert] = "Cannot reset password for LDAP user." respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) and return end - self.resource = resource_class.send_reset_password_instructions(resource_params) - if successfully_sent?(resource) - respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) - else - respond_with(resource) + unless can_send_reset_email? + flash[:alert] = "Instructions about how to reset your password have already been sent recently. Please wait a few minutes to try again." + respond_with({}, location: new_password_path(resource_name)) and return end + + super end def edit @@ -35,4 +36,11 @@ class PasswordsController < Devise::PasswordsController end end end + + private + + def can_send_reset_email? + resource && (resource.reset_password_sent_at.blank? || + resource.reset_password_sent_at < 1.minute.ago) + end end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index abf66f2356d..ce7a66a0da9 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -1,13 +1,44 @@ require 'spec_helper' feature 'Password reset', feature: true do - describe 'with two-factor authentication' do - let(:user) { create(:user, :two_factor) } + describe 'throttling' do + it 'sends reset instructions when not previously sent' do + visit root_path + forgot_password(create(:user)) + + expect(page).to have_content(I18n.t('devise.passwords.send_instructions')) + expect(current_path).to eq new_user_session_path + end + it 'sends reset instructions when previously sent more than a minute ago' do + user = create(:user) + user.send_reset_password_instructions + user.update_attribute(:reset_password_sent_at, 5.minutes.ago) + + visit root_path + forgot_password(user) + + expect(page).to have_content(I18n.t('devise.passwords.send_instructions')) + expect(current_path).to eq new_user_session_path + end + + it "throttles multiple resets in a short timespan" do + user = create(:user) + user.send_reset_password_instructions + + visit root_path + forgot_password(user) + + expect(page).to have_content("Instructions about how to reset your password have already been sent recently. Please wait a few minutes to try again.") + expect(current_path).to eq new_user_password_path + end + end + + describe 'with two-factor authentication' do it 'requires login after password reset' do visit root_path - forgot_password + forgot_password(create(:user, :two_factor)) reset_password expect(page).to have_content("Your password was changed successfully.") @@ -17,12 +48,10 @@ feature 'Password reset', feature: true do end describe 'without two-factor authentication' do - let(:user) { create(:user) } - it 'requires login after password reset' do visit root_path - forgot_password + forgot_password(create(:user)) reset_password expect(page).to have_content("Your password was changed successfully.") @@ -30,7 +59,7 @@ feature 'Password reset', feature: true do end end - def forgot_password + def forgot_password(user) click_on 'Forgot your password?' fill_in 'Email', with: user.email click_button 'Reset password' -- cgit v1.2.1