Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds an option to strictly enforce single recipients for emails #5680

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 43 additions & 1 deletion lib/devise/mailers/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ module Mailers
module Helpers
extend ActiveSupport::Concern

MultipleRecipientsError = Class.new(ArgumentError)

included do
include Devise::Controllers::ScopedViews
end
Expand All @@ -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)
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/controllers/internal_helpers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
class MyController < DeviseController
end

class HelpersTest < Devise::ControllerTestCase
class InternalHelpersTest < Devise::ControllerTestCase
tests MyController

def setup
Expand Down
2 changes: 1 addition & 1 deletion test/devise_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class DeviseTest < ActiveSupport::TestCase

test 'Devise.email_regexp should match valid email addresses' do
valid_emails = ["[email protected]", "[email protected]", "[email protected]", "[email protected]", "test@tt", "[email protected]"]
non_valid_emails = ["rex", "test [email protected]", "test_user@example server.com"]
non_valid_emails = ["rex", "test [email protected]", "test_user@example server.com", "test_user@example,[email protected]", "test_user@example;[email protected]"]

valid_emails.each do |email|
assert_match Devise.email_regexp, email
Expand Down
86 changes: 86 additions & 0 deletions test/mailers/helpers/helpers_test.rb
Original file line number Diff line number Diff line change
@@ -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: "[email protected]"})

assert_equal ["[email protected]"], mail.to
end

test "multiple recipients are permitted when setting configured to empty array" do
mail = send_mail({
to: ["[email protected]", "[email protected]"],
bcc: ["[email protected]"]
})

assert_equal ["[email protected]", "[email protected]"], mail.to
assert_equal ["[email protected]"], 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: ["[email protected]", "[email protected]"],
bcc: ["[email protected]"]
})

assert_equal ["[email protected]", "[email protected]"], mail.to
assert_equal ["[email protected]"], 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: '[email protected]', 'to' => '[email protected]' },
{ to: '[email protected],[email protected]' },
{ to: '[email protected];[email protected]' },
{ to: ['[email protected]', '[email protected]'] }
].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: ["[email protected]"]})
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: ["[email protected]"]})
end
end

end