Fix email enumeration vulnerabilities in password reset and registration flows

app/controllers/devise/passwords_controller.rb

@@ -15,11 +15,9 @@ def create
     self.resource = resource_class.send_reset_password_instructions(resource_params)
     yield resource if block_given?
 
-    if successfully_sent?(resource)
-      respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name))
-    else
-      respond_with(resource)
-    end
+    # Always show success message to prevent email enumeration
+    set_flash_message! :notice, :send_instructions
+    respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name))
   end
 
   # GET /resource/password/edit?reset_password_token=abcdef

app/controllers/devise/registrations_controller.rb

@@ -16,6 +16,32 @@ def new
   def create
     build_resource(sign_up_params)
 
+    # Check if email already exists when in paranoid mode
+    if Devise.paranoid
+      # Apply same transformations as the model would
+      email_key = resource_class.authentication_keys.first
+      email_value = resource.send(email_key)
+
+      # Apply case insensitive search if configured
+      if resource_class.case_insensitive_keys.include?(email_key)
+        existing_user = resource_class.where("lower(#{email_key}) = ?", email_value.downcase).first
+      else
+        existing_user = resource_class.find_by(email_key => email_value)
+      end
+
+      if existing_user
+        # Send "already registered" email instead of showing error
+        devise_mailer.email_already_registered(existing_user).deliver
+
+        # Show same success message as normal registration with confirmation
+        set_flash_message! :notice, :signed_up_but_unconfirmed
+        expire_data_after_sign_in!
+        redirect_to new_session_path(resource_name)
+        return
+      end
+    end
+
+    # Normal registration flow
     resource.save
     yield resource if block_given?
     if resource.persisted?
@@ -82,6 +108,10 @@ def cancel
 
   protected
 
+    def devise_mailer
+      Devise.mailer
+    end
+
   def update_needs_confirmation?(resource, previous)
     resource.respond_to?(:pending_reconfirmation?) &&
       resource.pending_reconfirmation? &&

app/mailers/devise/mailer.rb

@@ -26,5 +26,9 @@ def email_changed(record, opts = {})
     def password_change(record, opts = {})
       devise_mail(record, :password_change, opts)
     end
+
+    def email_already_registered(record, opts = {})
+      devise_mail(record, :email_already_registered, opts)
+    end
   end
 end

app/views/devise/mailer/email_already_registered.html.erb

@@ -0,0 +1,11 @@
+<p>Hello <%= @resource.email %>!</p>
+
+<p>Someone tried to create an account using your email address, but an account already exists.</p>
+
+<p>If this was you, you can:</p>
+<ul>
+  <li><%= link_to "Sign in to your account", new_session_url(@resource, @scope_name) %></li>
+  <li><%= link_to "Reset your password", new_password_url(@resource, @scope_name) %> if you've forgotten it</li>
+</ul>
+
+<p>If this wasn't you, you can safely ignore this email. Your account is secure.</p>

config/locales/en.yml

@@ -27,12 +27,14 @@ en:
         subject: "Email Changed"
       password_change:
         subject: "Password Changed"
+      email_already_registered:
+        subject: "Account already exists"
     omniauth_callbacks:
       failure: "Could not authenticate you from %{kind} because \"%{reason}\"."
       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."
-      send_instructions: "You will receive an email with instructions on how to reset your password in a few minutes."
+      send_instructions: "If your email address exists in our database, you will receive a password recovery link at your email address 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."
       updated_not_active: "Your password has been changed successfully."

lib/devise/models/recoverable.rb

@@ -118,11 +118,23 @@ def with_reset_password_token(token)
 
         # Attempt to find a user by its email. If a record is found, send new
         # password instructions to it. If user is not found, returns a new user
-        # with an email not found error.
+        # with no errors to prevent email enumeration attacks.
         # Attributes must contain the user's email
         def send_reset_password_instructions(attributes = {})
           recoverable = find_or_initialize_with_errors(reset_password_keys, attributes, :not_found)
-          recoverable.send_reset_password_instructions if recoverable.persisted?
+
+          if recoverable.persisted?
+            recoverable.send_reset_password_instructions
+          else
+            # Perform similar work to mitigate timing attacks
+            # Generate a token even though we won't use it
+            Devise.friendly_token(20)
+
+            # Always return a new user with no errors to prevent email enumeration
+            recoverable = new
+            recoverable.errors.clear
+          end
+
           recoverable
         end
 

mise.toml

@@ -0,0 +1,2 @@
+[tools]
+ruby = "3.1.6"

test/integration/recoverable_test.rb

@@ -52,7 +52,7 @@ def reset_password(options = {}, &block)
     end
 
     assert_current_url '/users/sign_in'
-    assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
+    assert_contain 'If your email address exists in our database'
   end
 
   test 'reset password with email should send an email from a custom mailer' do
@@ -68,18 +68,17 @@ def reset_password(options = {}, &block)
     assert_match edit_user_password_path(reset_password_token: 'abcdef'), mail.body.encoded
   end
 
-  test 'reset password with email of different case should fail when email is NOT the list of case insensitive keys' do
+  test 'reset password with email of different case should show success message when email is NOT the list of case insensitive keys' do
     swap Devise, case_insensitive_keys: [] do
       create_user(email: '[email protected]')
 
       request_forgot_password do
         fill_in 'email', with: '[email protected]'
       end
 
-      assert_response :success
-      assert_current_url '/users/password'
-      assert_have_selector "input[type=email][value='[email protected]']"
-      assert_contain 'not found'
+      # Now always shows success to prevent email enumeration
+      assert_current_url '/users/sign_in'
+      assert_contain 'If your email address exists in our database'
     end
   end
 
@@ -91,21 +90,20 @@ def reset_password(options = {}, &block)
     end
 
     assert_current_url '/users/sign_in'
-    assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
+    assert_contain 'If your email address exists in our database'
   end
 
-  test 'reset password with email with extra whitespace should fail when email is NOT the list of strip whitespace keys' do
+  test 'reset password with email with extra whitespace should show success message when email is NOT the list of strip whitespace keys' do
     swap Devise, strip_whitespace_keys: [] do
       create_user(email: '[email protected]')
 
       request_forgot_password do
         fill_in 'email', with: ' [email protected] '
       end
 
-      assert_response :success
-      assert_current_url '/users/password'
-      assert_have_selector "input[type=email][value=' [email protected] ']"
-      assert_contain 'not found'
+      # Now always shows success to prevent email enumeration
+      assert_current_url '/users/sign_in'
+      assert_contain 'If your email address exists in our database'
     end
   end
 
@@ -124,18 +122,17 @@ def reset_password(options = {}, &block)
     request_forgot_password
 
     assert_current_url '/users/sign_in'
-    assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
+    assert_contain 'If your email address exists in our database'
   end
 
-  test 'not authenticated user with invalid email should receive an error message' do
+  test 'not authenticated user with invalid email should still see success message' do
     request_forgot_password do
       fill_in 'email', with: '[email protected]'
     end
 
-    assert_response :success
-    assert_current_url '/users/password'
-    assert_have_selector "input[type=email][value='[email protected]']"
-    assert_contain 'not found'
+    # Now always shows success to prevent email enumeration
+    assert_current_url '/users/sign_in'
+    assert_contain 'If your email address exists in our database'
   end
 
   test 'authenticated user should not be able to visit edit password page' do
@@ -293,14 +290,15 @@ def reset_password(options = {}, &block)
     assert_equal({}.to_json, response.body)
   end
 
-  test 'reset password request with invalid e-mail in JSON format should return valid response' do
+  test 'reset password request with invalid e-mail in JSON format should return success response' do
     create_user
     post user_password_path(format: 'json'), params: { user: {email: "[email protected]"} }
-    assert_response :unprocessable_entity
-    assert_includes response.body, '{"errors":{'
+    # Now always returns success to prevent email enumeration
+    assert_response :success
+    assert_equal({}.to_json, response.body)
   end
 
-  test 'reset password request with invalid e-mail in JSON format should return empty and valid response in paranoid mode' do
+  test 'reset password request with invalid e-mail in JSON format also returns success in paranoid mode' do
     swap Devise, paranoid: true do
       create_user
       post user_password_path(format: 'json'), params: { user: {email: "[email protected]"} }

test/integration/registerable_test.rb

@@ -138,6 +138,31 @@ def user_sign_up
     assert_not warden.authenticated?(:user)
   end
 
+  test 'in paranoid mode, duplicate registration shows success message and sends email to existing user' do
+    swap Devise, paranoid: true do
+      create_user
+      get new_user_registration_path
+
+      fill_in 'email', with: '[email protected]'
+      fill_in 'password', with: '123456'
+      fill_in 'password confirmation', with: '123456'
+
+      assert_email_sent '[email protected]' do
+        click_button 'Sign up'
+      end
+
+      # Should redirect to sign in page with success message
+      assert_current_url '/users/sign_in'
+      assert_contain 'A message with a confirmation link has been sent to your email address'
+      assert_not warden.authenticated?(:user)
+
+      # Check that the email sent is the "already registered" email
+      mail = ActionMailer::Base.deliveries.last
+      assert_equal 'Account already exists', mail.subject
+      assert_match 'Someone tried to create an account using your email address', mail.body.encoded
+    end
+  end
+
   test 'a guest should not be able to change account' do
     get edit_user_registration_path
     assert_redirected_to new_user_session_path

test/models/recoverable_test.rb

@@ -119,10 +119,11 @@ def setup
     assert_equal user, reset_password_user
   end
 
-  test 'should return a new record with errors if user was not found by e-mail' do
+  test 'should return a new record with no errors if user was not found by e-mail' do
     reset_password_user = User.send_reset_password_instructions(email: "[email protected]")
     assert_not reset_password_user.persisted?
-    assert_equal "not found", reset_password_user.errors[:email].join
+    # No longer exposes email not found to prevent enumeration
+    assert reset_password_user.errors.empty?
   end
 
   test 'should find a user to send instructions by authentication_keys' do
@@ -138,7 +139,8 @@ def setup
       user = create_user
       reset_password_user = User.send_reset_password_instructions(email: user.email)
       assert_not reset_password_user.persisted?
-      assert reset_password_user.errors.added?(:username, :blank)
+      # No longer shows specific errors to prevent enumeration
+      assert reset_password_user.errors.empty?
     end
   end
 
@@ -260,4 +262,28 @@ def setup
     assert_nil User.with_reset_password_token('random-token')
   end
 
+  test 'should not expose email existence by default' do
+    user = User.send_reset_password_instructions(email: '[email protected]')
+    assert_not user.persisted?
+    assert user.errors.empty?
+  end
+
+  test 'should send reset instructions only for existing users' do
+    existing_user = create_user
+
+    # Should send email for existing user
+    assert_email_sent do
+      user = User.send_reset_password_instructions(email: existing_user.email)
+      assert user.persisted?
+      assert user.errors.empty?
+    end
+
+    # Should not send email for non-existing user
+    assert_email_not_sent do
+      user = User.send_reset_password_instructions(email: '[email protected]')
+      assert_not user.persisted?
+      assert user.errors.empty?
+    end
+  end
+
 end