Be paranoid when sending password reset emails

This implements what is known as "paranoid" password reset flash
messages (using the terminology from Devise). It avoids revealing
whether the supplied email address is already registered.

Added an explicit test for this situation, so that the test for
email non-existance is separate from the duplicate-case tests.

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 8025fd700977d35f82ba267c2dd9136dee2d80a4..25b2b96075bb2d24e79a8e21f4170a2e89756a65 100644 (file)
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -43,12 +43,10 @@ class PasswordsController < ApplicationController
     if user
       token = user.generate_token_for(:password_reset)
       UserMailer.lost_password(user, token).deliver_later
     if user
       token = user.generate_token_for(:password_reset)
       UserMailer.lost_password(user, token).deliver_later

-      flash[:notice] = t ".notice email on way"
-      redirect_to login_path
-    else
-      flash.now[:error] = t ".notice email cannot find"
-      render :new

     end
     end

+
+    flash[:notice] = t ".send_paranoid_instructions"
+    redirect_to login_path

   end
 
   def update
   end
 
   def update

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 429786fcc3c13994eca2ff66254a565a4bbf72c6..0cc35863b4fbbe77e2020ac440c4d5c70c56214d 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1777,7 +1777,7 @@ en:
       wrong_user: "You are logged in as `%{user}' but the message you have asked to read was not sent by or to that user. Please login as the correct user in order to read it."
     sent_message_summary:
       destroy_button: "Delete"
       wrong_user: "You are logged in as `%{user}' but the message you have asked to read was not sent by or to that user. Please login as the correct user in order to read it."
     sent_message_summary:
       destroy_button: "Delete"

-    heading: 
+    heading:

       my_inbox: "My Inbox"
       my_outbox: "My Outbox"
       muted_messages: "Muted messages"
       my_inbox: "My Inbox"
       my_outbox: "My Outbox"
       muted_messages: "Muted messages"


@@ -1797,8 +1797,7 @@ en:
       new password button: "Reset password"
       help_text: "Enter the email address you used to sign up, we will send a link to it that you can use to reset your password."
     create:
       new password button: "Reset password"
       help_text: "Enter the email address you used to sign up, we will send a link to it that you can use to reset your password."
     create:

-      notice email on way: "Sorry you lost it :-( but an email is on its way so you can reset it soon."
-      notice email cannot find: "Could not find that email address, sorry."
+      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."

     edit:
       title: "Reset password"
       heading: "Reset Password for %{user}"
     edit:
       title: "Reset password"
       heading: "Reset Password for %{user}"

diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb
index 25cfdd4e5068c87c6022533fc6c8bb4b45a0ce3a..24d3f18938f094442cc91b8d7446464ede237f1a 100644 (file)
--- a/test/controllers/passwords_controller_test.rb
+++ b/test/controllers/passwords_controller_test.rb
@@ -51,12 +51,23 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     end
     assert_response :redirect
     assert_redirected_to login_path
     end
     assert_response :redirect
     assert_redirected_to login_path

-    assert_match(/^Sorry you lost it/, flash[:notice])
+    assert_match(/^If your email address exists/, flash[:notice])

     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal user.email, email.to.first
     ActionMailer::Base.deliveries.clear
 
     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal user.email, email.to.first
     ActionMailer::Base.deliveries.clear
 

+    # Test resetting using an address that does not exist
+    assert_no_difference "ActionMailer::Base.deliveries.size" do
+      perform_enqueued_jobs do
+        post user_forgot_password_path, :params => { :email => "nobody@example.com" }
+      end
+    end
+    # Be paranoid about revealing there was no match
+    assert_response :redirect
+    assert_redirected_to login_path
+    assert_match(/^If your email address exists/, flash[:notice])
+

     # Test resetting using an address that matches a different user
     # that has the same address in a different case
     assert_difference "ActionMailer::Base.deliveries.size", 1 do
     # Test resetting using an address that matches a different user
     # that has the same address in a different case
     assert_difference "ActionMailer::Base.deliveries.size", 1 do


@@ -66,7 +77,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     end
     assert_response :redirect
     assert_redirected_to login_path
     end
     assert_response :redirect
     assert_redirected_to login_path

-    assert_match(/^Sorry you lost it/, flash[:notice])
+    assert_match(/^If your email address exists/, flash[:notice])

     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal uppercase_user.email, email.to.first
     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal uppercase_user.email, email.to.first


@@ -79,9 +90,10 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
         post user_forgot_password_path, :params => { :email => user.email.titlecase }
       end
     end
         post user_forgot_password_path, :params => { :email => user.email.titlecase }
       end
     end

-    assert_response :success
-    assert_template :new
-    assert_select ".alert.alert-danger", /^Could not find that email address/
+    # Be paranoid about revealing there was no match
+    assert_response :redirect
+    assert_redirected_to login_path
+    assert_match(/^If your email address exists/, flash[:notice])

 
     # Test resetting using the address as recorded for a user that has an
     # address which is case insensitively unique
 
     # Test resetting using the address as recorded for a user that has an
     # address which is case insensitively unique


@@ -93,7 +105,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     end
     assert_response :redirect
     assert_redirected_to login_path
     end
     assert_response :redirect
     assert_redirected_to login_path

-    assert_match(/^Sorry you lost it/, flash[:notice])
+    assert_match(/^If your email address exists/, flash[:notice])

     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal third_user.email, email.to.first
     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal third_user.email, email.to.first


@@ -108,7 +120,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     end
     assert_response :redirect
     assert_redirected_to login_path
     end
     assert_response :redirect
     assert_redirected_to login_path

-    assert_match(/^Sorry you lost it/, flash[:notice])
+    assert_match(/^If your email address exists/, flash[:notice])

     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal third_user.email, email.to.first
     email = ActionMailer::Base.deliveries.first
     assert_equal 1, email.to.count
     assert_equal third_user.email, email.to.first