summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDrew Blessing <drew@gitlab.com>2015-12-09 11:45:26 -0600
committerDrew Blessing <drew@gitlab.com>2015-12-09 18:40:37 -0600
commitf4ec906e90b2f8dbf18b359b773e3b31f5da89ff (patch)
treefd9d5a760e8100e643e49b6e26fe9d007d004b98
parent7b50965e9990bcb88f56b771d47514cbeb5316e5 (diff)
downloadgitlab-ce-f4ec906e90b2f8dbf18b359b773e3b31f5da89ff.tar.gz
Use devise paranoid mode and ensure the same message is returned every time
Skipped CI because it has already passed. Had to rebase due to CHANGELOG.
-rw-r--r--CHANGELOG3
-rw-r--r--app/controllers/passwords_controller.rb6
-rw-r--r--config/initializers/devise.rb2
-rw-r--r--config/locales/devise.en.yml1
-rw-r--r--spec/features/password_reset_spec.rb26
5 files changed, 21 insertions, 17 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 01f4c386e1a..03574b1e771 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,9 @@ v 8.2.3
- Fix Error 500s when creating global milestones with Unicode characters (Stan Hu)
- Update documentation for "Guest" permissions
- Properly convert Emoji-only comments into Award Emojis
+ - Enable devise paranoid mode to prevent user enumeration attack
+
+v 8.2.3
- Webhook payload has an added, modified and removed properties for each commit
- Fix 500 error when creating a merge request that removes a submodule
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 2025158d065..f74daff3bd0 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -40,7 +40,9 @@ class PasswordsController < Devise::PasswordsController
def throttle_reset
return unless resource && resource.recently_sent_password_reset?
- redirect_to new_password_path(resource_name),
- alert: I18n.t('devise.passwords.recently_reset')
+ # Throttle reset attempts, but return a normal message to
+ # avoid user enumeration attack.
+ redirect_to new_user_session_path,
+ notice: I18n.t('devise.passwords.send_paranoid_instructions')
end
end
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 29506970af2..5fb43a86e13 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -60,7 +60,7 @@ Devise.setup do |config|
# It will change confirmation, password recovery and other workflows
# to behave the same regardless if the e-mail provided was right or wrong.
# Does not affect registerable.
- # config.paranoid = true
+ config.paranoid = true
# ==> Configuration for :database_authenticatable
# For bcrypt, this is the cost for hashing the password and defaults to 10. If
diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml
index 22070e37f07..bd4c3ebc69e 100644
--- a/config/locales/devise.en.yml
+++ b/config/locales/devise.en.yml
@@ -30,7 +30,6 @@ en:
success: "Successfully authenticated from %{kind} account."
passwords:
no_token: "You can't access this page without coming from a password reset email. If you do come from a password reset email, please make sure you used the full URL provided."
- recently_reset: "Instructions about how to reset your password have already been sent recently. Please wait a few minutes to try again."
send_instructions: "You will receive an email with instructions on how to reset your password in a few minutes."
send_paranoid_instructions: "If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes."
updated: "Your password has been changed successfully. You are now signed in."
diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb
index 85e70b4d47f..257d363438c 100644
--- a/spec/features/password_reset_spec.rb
+++ b/spec/features/password_reset_spec.rb
@@ -3,11 +3,12 @@ require 'spec_helper'
feature 'Password reset', feature: true do
describe 'throttling' do
it 'sends reset instructions when not previously sent' do
- visit root_path
- forgot_password(create(:user))
+ user = create(:user)
+ forgot_password(user)
- expect(page).to have_content(I18n.t('devise.passwords.send_instructions'))
+ expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions'))
expect(current_path).to eq new_user_session_path
+ expect(user.recently_sent_password_reset?).to be_truthy
end
it 'sends reset instructions when previously sent more than a minute ago' do
@@ -15,26 +16,25 @@ feature 'Password reset', feature: true do
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{ forgot_password(user) }.to change{ user.reset_password_sent_at }
+ expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions'))
expect(current_path).to eq new_user_session_path
end
- it "throttles multiple resets in a short timespan" do
+ it 'throttles multiple resets in a short timespan' do
user = create(:user)
user.send_reset_password_instructions
+ # Reload because PG handles datetime less precisely than Ruby/Rails
+ user.reload
- visit root_path
- forgot_password(user)
-
- expect(page).to have_content(I18n.t('devise.passwords.recently_reset'))
- expect(current_path).to eq new_user_password_path
+ expect{ forgot_password(user) }.not_to change{ user.reset_password_sent_at }
+ expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions'))
+ expect(current_path).to eq new_user_session_path
end
end
def forgot_password(user)
+ visit root_path
click_on 'Forgot your password?'
fill_in 'Email', with: user.email
click_button 'Reset password'