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

Fix/return path #276

Merged
merged 13 commits into from
Apr 24, 2023
9 changes: 7 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -479,9 +479,14 @@ Full List of Settings
http://readthedocs.org/docs/boto/en/latest/ref/ses.html#boto.ses.regions
http://docs.aws.amazon.com/general/latest/gr/rande.html

``AWS_SES_FROM_EMAIL``
Optional. The email address to be used as the "From" address for the email. The address that you specify has to be verified.
For more information please refer to https://boto3.amazonaws.com/v1/documentation/api/1.26.31/reference/services/sesv2.html#SESV2.Client.send_email
``AWS_SES_RETURN_PATH``
Instruct Amazon SES to forward bounced emails and complaints to this email.
For more information please refer to http://aws.amazon.com/ses/faqs/#38
Optional. Use `AWS_SES_RETURN_PATH` to receive complaint notifications
You must use the v2 client by setting `USE_SES_V2=True` for this setting to work, otherwise it is ignored.
https://docs.aws.amazon.com/ses/latest/APIReference-V2/API_SendEmail.html#API_SendEmail_RequestSyntax


``AWS_SES_CONFIGURATION_SET``
Optional. Use this to mark your e-mails as from being from a particular SES
Expand Down
12 changes: 7 additions & 5 deletions django_ses/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def send_messages(self, email_messages):
return

num_sent = 0
source = settings.AWS_SES_RETURN_PATH
source = settings.AWS_SES_FROM_EMAIL
email_feedback = settings.AWS_SES_RETURN_PATH
for message in email_messages:
# SES Configuration sets. If the AWS_SES_CONFIGURATION_SET setting
# is not None, append the appropriate header to the message so that
Expand Down Expand Up @@ -159,7 +160,7 @@ def send_messages(self, email_messages):
if self._throttle:
self._update_throttling()

kwargs = self._get_send_email_parameters(message, source)
kwargs = self._get_send_email_parameters(message, source, email_feedback)

try:
response = (self.connection.send_email(**kwargs)
Expand Down Expand Up @@ -238,17 +239,18 @@ def _update_throttling(self):
recent_send_times.append(now)
# end of throttling

def _get_send_email_parameters(self, message, source):
return (self._get_v2_parameters(message, source)
def _get_send_email_parameters(self, message, source, email_feedack):
return (self._get_v2_parameters(message, source, email_feedack)
if self._use_ses_v2
else self._get_v1_parameters(message, source))

def _get_v2_parameters(self, message, source):
def _get_v2_parameters(self, message, source, email_feedback):
"""V2-Style raw payload for `send_email`.

https://boto3.amazonaws.com/v1/documentation/api/1.26.31/reference/services/sesv2.html#SESV2.Client.send_email
"""
params = dict(
FeedbackForwardingEmailAddress=email_feedback,
FromEmailAddress=source or message.from_email,
Destination={
'ToAddresses': message.recipients()
Expand Down
1 change: 1 addition & 0 deletions django_ses/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
AWS_SES_RETURN_PATH_ARN = getattr(settings, 'AWS_SES_RETURN_PATH_ARN', None)

USE_SES_V2 = getattr(settings, 'USE_SES_V2', False)
AWS_SES_FROM_EMAIL = getattr(settings, 'AWS_SES_FROM_EMAIL', None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a breaking change, as it changes what AWS_SES_RETURN_PATH is used for right?

What happens to an existing project if they upgrade to this new version but don't declare AWS_SES_FROM_EMAIL? Will something break?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

It will cause a breaking change on the following scenery

AWS_SES_RETURN_PATH = "my_default_from_email@email.com"

SESBackend().send_messages(
    [
        {
            "from": "ignored_from_email@email.com",
            "to": "to_email@email.com",
            "subject": "some subject",
            "message": "some message"
        }
    ]
)

Before this change, this message would be sent with the FromEmailAddress=my_default_from_email@email.com.

After this change, if the attribute AWS_SES_FROM_EMAIL is not set with AWS_SES_FROM_EMAIL=my_default_from_email@email.com, the email will be sent with the from email of the EmailMessage, which would be FromEmailAddress=ignored_from_email@email.com.

The correct usage AFTER THE CHANGE would be

AWS_SES_FROM_EMAIL = "my_default_from_email@email.com"

SESBackend().send_messages(
    [
        {
            "from": "ignored_from_email@email.com",
            "to": "to_email@email.com",
            "subject": "some subject",
            "message": "some message"
        }
    ]
)

And this scenery could be a test itself! And it could act as a regression test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind, please add this example to the PR description and update the changelog, that will make it easier to merge this PR!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!


TIME_ZONE = settings.TIME_ZONE

Expand Down
23 changes: 21 additions & 2 deletions tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def test_configuration_set_callable_send_mail(self):
self.assertEqual(config_set_callable.dkim_selector, 'ses')
self.assertEqual(config_set_callable.dkim_headers, ('From', 'To', 'Cc', 'Subject'))


class SESV2BackendTest(TestCase):
def setUp(self):
django_settings.EMAIL_BACKEND = 'tests.test_backend.FakeSESBackend'
Expand Down Expand Up @@ -259,9 +260,18 @@ def test_return_path(self):
"""Ensure that the 'Source' argument sent into send_raw_email uses FromEmailAddress.
"""
settings.AWS_SES_RETURN_PATH = None
settings.AWS_SES_FROM_EMAIL = 'from@example.com'
send_mail('subject', 'body', 'from@example.com', ['to@example.com'])
self.assertEqual(self.outbox.pop()['FromEmailAddress'], 'from@example.com')

def test_feedback_forwarding(self):
"""
Ensure that the notification address argument uses FeedbackForwardingEmailAddress.
"""
settings.AWS_SES_RETURN_PATH = 'reply@example.com'
send_mail('subject', 'body', 'from@example.com', ['to@example.com'])
self.assertEqual(self.outbox.pop()['FeedbackForwardingEmailAddress'], 'reply@example.com')

def test_source_arn_is_NOT_set(self):
"""
Ensure that the helpers for Identity Owner for SES Sending Authorization are not present, if nothing has been
Expand Down Expand Up @@ -324,8 +334,17 @@ def setUp(self):
def tearDown(self):
# Empty outbox everytime test finishes
FakeSESConnection.outbox = []


def test_from_email(self):
settings.AWS_SES_FROM_EMAIL = "my_default_from@example.com"
send_mail('subject', 'body', 'ignored_from@example.com', ['to@example.com'])
self.assertEqual(self.outbox.pop()['Source'], 'my_default_from@example.com')

def test_return_path(self):
settings.USE_SES_V2 = True
settings.AWS_SES_RETURN_PATH = "return@example.com"
send_mail('subject', 'body', 'from@example.com', ['to@example.com'])
self.assertEqual(self.outbox.pop()['Source'], 'return@example.com')
message = self.outbox.pop()

self.assertEqual(message['FromEmailAddress'], 'my_default_from@example.com')
self.assertEqual(message['FeedbackForwardingEmailAddress'], 'return@example.com')