diff --git a/lib/devise.rb b/lib/devise.rb index 3847e190c..2c3e09ed8 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -308,6 +308,17 @@ module Test mattr_accessor :sign_in_after_change_password @@sign_in_after_change_password = true + # When sending an email to an action set below, raise an error if + # there is more than one recipient. For example: + # @@strict_single_recipient_emails = [ + # :confirmation_instructions, + # :reset_password_instructions, + # :unlock_instructions + # ] + # By default any action can be sent to multiple recipients. + mattr_accessor :strict_single_recipient_emails + @@strict_single_recipient_emails = [] + # Default way to set up Devise. Run rails generate devise_install to create # a fresh initializer with all configuration values. def self.setup diff --git a/lib/devise/mailers/helpers.rb b/lib/devise/mailers/helpers.rb index 29a491970..e7bcaa6a9 100644 --- a/lib/devise/mailers/helpers.rb +++ b/lib/devise/mailers/helpers.rb @@ -5,6 +5,8 @@ module Mailers module Helpers extend ActiveSupport::Concern + MultipleRecipientsError = Class.new(ArgumentError) + included do include Devise::Controllers::ScopedViews end @@ -16,7 +18,11 @@ module Helpers # Configure default email options def devise_mail(record, action, opts = {}, &block) initialize_from_record(record) - mail headers_for(action, opts), &block + + headers = headers_for(action, opts) + validate_single_recipient_in_headers!(headers) if Devise.strict_single_recipient_emails.include?(action) + + mail headers, &block end def initialize_from_record(record) @@ -82,6 +88,42 @@ def subject_for(key) I18n.t(:"#{devise_mapping.name}_subject", scope: [:devise, :mailer, key], default: [:subject, key.to_s.humanize]) end + + # It is possible to send email to one or more recipients in one + # email by setting a list of emails in the :to key, or by :cc or + # :bcc-ing recipients. + # https://guides.rubyonrails.org/action_mailer_basics.html#sending-email-to-multiple-recipients + # + # This method ensures the headers contain a single recipient. + def validate_single_recipient_in_headers!(headers) + return unless headers + + symbolized_headers = headers.symbolize_keys + + if headers.keys.length != symbolized_headers.keys.length + raise MultipleRecipientsError, "headers has colliding key names" + end + + if symbolized_headers[:cc] || symbolized_headers[:bcc] + raise MultipleRecipientsError, 'headers[:cc] and headers[:bcc] are not allowed' + end + + if symbolized_headers[:to] && !validate_single_recipient_in_email(symbolized_headers[:to]) + raise MultipleRecipientsError, 'headers[:to] must be a string not containing ; or ,' + end + + true + end + + # Returns true if email is a String not containing email + # separators: commas (RFC5322) or semicolons (RFC1485). + # + # Unlike Devise.email_regexp (which can be overridden), it does + # not validate that the email is valid. + def validate_single_recipient_in_email(email) + email.is_a?(String) && !email.match(/[;,]/) + end + end end end diff --git a/test/controllers/internal_helpers_test.rb b/test/controllers/internal_helpers_test.rb index 124c8df06..e71ea0f34 100644 --- a/test/controllers/internal_helpers_test.rb +++ b/test/controllers/internal_helpers_test.rb @@ -5,7 +5,7 @@ class MyController < DeviseController end -class HelpersTest < Devise::ControllerTestCase +class InternalHelpersTest < Devise::ControllerTestCase tests MyController def setup diff --git a/test/devise_test.rb b/test/devise_test.rb index 532aa57dc..bb7f2b36b 100644 --- a/test/devise_test.rb +++ b/test/devise_test.rb @@ -97,7 +97,7 @@ class DeviseTest < ActiveSupport::TestCase test 'Devise.email_regexp should match valid email addresses' do valid_emails = ["test@example.com", "jo@jo.co", "f4$_m@you.com", "testing.example@example.com.ua", "test@tt", "test@valid---domain.com"] - non_valid_emails = ["rex", "test user@example.com", "test_user@example server.com"] + non_valid_emails = ["rex", "test user@example.com", "test_user@example server.com", "test_user@example,rex@server.com", "test_user@example;rex@server.com"] valid_emails.each do |email| assert_match Devise.email_regexp, email diff --git a/test/mailers/helpers/helpers_test.rb b/test/mailers/helpers/helpers_test.rb new file mode 100644 index 000000000..f791f3b6a --- /dev/null +++ b/test/mailers/helpers/helpers_test.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'test_helper' + +class HelpersTest < ActiveSupport::TestCase + def setup + Devise.strict_single_recipient_emails = [] + end + + def user + @user ||= create_user + end + + def send_mail(opts={}) + Devise.mailer.reset_password_instructions( + user, + 'fake-token', + opts, + ).deliver + end + + test "an email passed in opts is used instead of the record's email" do + mail = send_mail({to: "test2@example.com"}) + + assert_equal ["test2@example.com"], mail.to + end + + test "multiple recipients are permitted when setting configured to empty array" do + mail = send_mail({ + to: ["test1@example.com", "test2@example.com"], + bcc: ["admin@example.com"] + }) + + assert_equal ["test1@example.com", "test2@example.com"], mail.to + assert_equal ["admin@example.com"], mail.bcc + end + + test "multiple recipients are permitted setting configured to another action" do + Devise.strict_single_recipient_emails = [:unlock_instructions] + mail = send_mail({ + to: ["test1@example.com", "test2@example.com"], + bcc: ["admin@example.com"] + }) + + assert_equal ["test1@example.com", "test2@example.com"], mail.to + assert_equal ["admin@example.com"], mail.bcc + end + + test "single to recipient does not raises an error when action has strict enforcement" do + Devise.strict_single_recipient_emails = [:reset_password_instructions] + mail = send_mail + + assert_equal [user.email], mail.to + end + + test "multiple to recipients raises an error when action has strict enforcement" do + + Devise.strict_single_recipient_emails = [:reset_password_instructions] + + [ + { to: 'test1@example.com', 'to' => 'test2@example.com' }, + { to: 'test1@example.com,test2@example.com' }, + { to: 'test1@example.com;test2@example.com' }, + { to: ['test1@example.com', 'test2@example.com'] } + ].each do |headers| + assert_raises(Devise::Mailers::Helpers::MultipleRecipientsError) do + send_mail(headers) + end + end + end + + test "bcc raises an error when action has strict enforcement" do + Devise.strict_single_recipient_emails = [:reset_password_instructions] + assert_raises(Devise::Mailers::Helpers::MultipleRecipientsError) do + send_mail({bcc: ["admin@example.com"]}) + end + end + + test "cc raises an error when action has strict enforcement" do + Devise.strict_single_recipient_emails = [:reset_password_instructions] + assert_raises(Devise::Mailers::Helpers::MultipleRecipientsError) do + send_mail({cc: ["admin@example.com"]}) + end + end + +end \ No newline at end of file