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.
This commit is contained in:
Andy Allan 2024-03-02 15:48:54 +00:00
parent 664d02982c
commit 4e237db390
3 changed files with 24 additions and 15 deletions

View file

@ -43,12 +43,10 @@ class PasswordsController < ApplicationController
if user if user
token = user.generate_token_for(:password_reset) token = user.generate_token_for(:password_reset)
UserMailer.lost_password(user, token).deliver_later 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 end
def update def update

View file

@ -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." 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: sent_message_summary:
destroy_button: "Delete" destroy_button: "Delete"
heading: heading:
my_inbox: "My Inbox" my_inbox: "My Inbox"
my_outbox: "My Outbox" my_outbox: "My Outbox"
muted_messages: "Muted messages" muted_messages: "Muted messages"
@ -1797,8 +1797,7 @@ en:
new password button: "Reset password" 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." 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: create:
notice email on way: "Sorry you lost it :-( but an email is on its way so you can reset it soon." 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."
notice email cannot find: "Could not find that email address, sorry."
edit: edit:
title: "Reset password" title: "Reset password"
heading: "Reset Password for %{user}" heading: "Reset Password for %{user}"

View file

@ -51,12 +51,23 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
end end
assert_response :redirect assert_response :redirect
assert_redirected_to login_path 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 email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count assert_equal 1, email.to.count
assert_equal user.email, email.to.first assert_equal user.email, email.to.first
ActionMailer::Base.deliveries.clear 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 # Test resetting using an address that matches a different user
# that has the same address in a different case # that has the same address in a different case
assert_difference "ActionMailer::Base.deliveries.size", 1 do assert_difference "ActionMailer::Base.deliveries.size", 1 do
@ -66,7 +77,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
end end
assert_response :redirect assert_response :redirect
assert_redirected_to login_path 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 email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count assert_equal 1, email.to.count
assert_equal uppercase_user.email, email.to.first 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 } post user_forgot_password_path, :params => { :email => user.email.titlecase }
end end
end end
assert_response :success # Be paranoid about revealing there was no match
assert_template :new assert_response :redirect
assert_select ".alert.alert-danger", /^Could not find that email address/ 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 # Test resetting using the address as recorded for a user that has an
# address which is case insensitively unique # address which is case insensitively unique
@ -93,7 +105,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
end end
assert_response :redirect assert_response :redirect
assert_redirected_to login_path 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 email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count assert_equal 1, email.to.count
assert_equal third_user.email, email.to.first assert_equal third_user.email, email.to.first
@ -108,7 +120,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
end end
assert_response :redirect assert_response :redirect
assert_redirected_to login_path 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 email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count assert_equal 1, email.to.count
assert_equal third_user.email, email.to.first assert_equal third_user.email, email.to.first