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.

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.
ce16bdc0 · Drew Blessing · Robert Speicher · 7b50965e · ce16bdc0 · ce16bdc0
--- 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


--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -40,7 +40,9 @@ def prevent_ldap_reset
  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
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -60,7 +60,7 @@
  # 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

--- 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."

--- a/spec/features/password_reset_spec.rb
+++ b/spec/features/password_reset_spec.rb
@@ -3,11 +3,12 @@
 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,24 @@
      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

-      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.to_s(:iso8601) }
+      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'