From ac0fa5b21175dc5cbc6ed6c39fdb6cae71719c98 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 25 Apr 2016 16:37:55 +0100 Subject: [PATCH] Send emails with a friendly from name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s nicer to have emails with a sender name, as well as the raw email address. Amazon SES can acheive this by using the format ``` "Sender name" ``` — http://docs.aws.amazon.com/ses/latest/DeveloperGuide/email-format.html We also have to remove all non-ASCII characters from the sender name, because SMTP only supports 7-bit ASCII: > A field name MUST be composed of printable US-ASCII characters (i.e., > characters that have values between 33 and 126, inclusive), except > colon. — http://www.ietf.org/rfc/rfc5322.txt We use the service name as the sender name when: - sending emails from the API - sending emails from a CSV file We use GOV.UK Notify as the sender name when: - sending invitation emails - sending password reset emails --- app/celery/tasks.py | 22 ++++++++++--- app/notifications/rest.py | 2 +- tests/app/celery/test_tasks.py | 46 ++++++++++++++++++---------- tests/app/notifications/test_rest.py | 2 +- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index eb2be882f..4e62dc33b 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -173,7 +173,11 @@ def process_job(job_id): send_email.apply_async(( str(job.service_id), str(create_uuid()), - "{}@{}".format(job.service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + '"{}" <{}@{}>'.format( + service.name, + service.email_from, + current_app.config['NOTIFY_EMAIL_DOMAIN'] + ).encode('ascii', 'ignore').decode('ascii'), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), queue='bulk-email') @@ -384,8 +388,10 @@ def email_invited_user(encrypted_invitation): url, invitation['expiry_date']) try: - email_from = "{}@{}".format(current_app.config['INVITATION_EMAIL_FROM'], - current_app.config['NOTIFY_EMAIL_DOMAIN']) + email_from = '"GOV.UK Notify" <{}@{}>'.format( + current_app.config['INVITATION_EMAIL_FROM'], + current_app.config['NOTIFY_EMAIL_DOMAIN'] + ) subject_line = invitation_subject_line(invitation['user_name'], invitation['service_name']) aws_ses_client.send_email(email_from, invitation['to'], @@ -411,7 +417,10 @@ def password_reset_message(name, url): def email_reset_password(encrypted_reset_password_message): reset_password_message = encryption.decrypt(encrypted_reset_password_message) try: - aws_ses_client.send_email(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], + email_from = '"GOV.UK Notify" <{}>'.format( + current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] + ) + aws_ses_client.send_email(email_from, reset_password_message['to'], "Reset your GOV.UK Notify password", password_reset_message(name=reset_password_message['name'], @@ -433,7 +442,10 @@ def registration_verification_template(name, url): def email_registration_verification(encrypted_verification_message): verification_message = encryption.decrypt(encrypted_verification_message) try: - aws_ses_client.send_email(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], + email_from = '"GOV.UK Notify" <{}>'.format( + current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] + ) + aws_ses_client.send_email(email_from, verification_message['to'], "Confirm GOV.UK Notify registration", registration_verification_template(name=verification_message['name'], diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ec261a42d..713d928f0 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -359,7 +359,7 @@ def send_notification(notification_type): send_email.apply_async(( service_id, notification_id, - "{}@{}".format(service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), encryption.encrypt(notification), datetime.utcnow().strftime(DATETIME_FORMAT) ), queue='email') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 06b9c6b6a..27ef514e2 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -180,10 +180,10 @@ def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, not @freeze_time("2016-01-01 11:09:00.061258") -def test_should_process_sms_job_if_exactly_on_send_limits(notify_db, - notify_db_session, - mocker, - mock_celery_remove_job): +def test_should_process_email_job_if_exactly_on_send_limits(notify_db, + notify_db_session, + mocker, + mock_celery_remove_job): service = sample_service(notify_db, notify_db_session, limit=10) template = sample_email_template(notify_db, notify_db_session, service=service) job = sample_job(notify_db, notify_db_session, service=service, template=template, notification_count=10) @@ -202,11 +202,17 @@ def test_should_process_sms_job_if_exactly_on_send_limits(notify_db, job = jobs_dao.dao_get_job_by_id(job.id) assert job.status == 'finished' tasks.send_email.apply_async.assert_called_with( - (str(job.service_id), - "uuid", - "{}@{}".format(job.service.email_from, "test.notify.com"), - "something_encrypted", - "2016-01-01T11:09:00.061258"), + ( + str(job.service_id), + "uuid", + "\"{}\" <{}@{}>".format( + service.name, + service.email_from, + "test.notify.com" + ), + "something_encrypted", + "2016-01-01T11:09:00.061258" + ), queue="bulk-email" ) mock_celery_remove_job.assert_called_once_with((str(job.id),), queue="remove-job") @@ -243,11 +249,17 @@ def test_should_process_email_job(sample_email_job, mocker, mock_celery_remove_j assert encryption.encrypt.call_args[0][0]['to'] == 'test@test.com' assert encryption.encrypt.call_args[0][0]['personalisation'] == {} tasks.send_email.apply_async.assert_called_once_with( - (str(sample_email_job.service_id), - "uuid", - "{}@{}".format(sample_email_job.service.email_from, "test.notify.com"), - "something_encrypted", - "2016-01-01T11:09:00.061258"), + ( + str(sample_email_job.service_id), + "uuid", + "\"{}\" <{}@{}>".format( + sample_email_job.service.name, + sample_email_job.service.email_from, + "test.notify.com" + ), + "something_encrypted", + "2016-01-01T11:09:00.061258" + ), queue="bulk-email" ) job = jobs_dao.dao_get_job_by_id(sample_email_job.id) @@ -799,8 +811,10 @@ def test_email_invited_user_should_send_email(notify_api, mocker): invitation['expiry_date']) email_invited_user(encryption.encrypt(invitation)) - email_from = "{}@{}".format(current_app.config['INVITATION_EMAIL_FROM'], - current_app.config['NOTIFY_EMAIL_DOMAIN']) + email_from = '"GOV.UK Notify" <{}@{}>'.format( + current_app.config['INVITATION_EMAIL_FROM'], + current_app.config['NOTIFY_EMAIL_DOMAIN'] + ) expected_subject = tasks.invitation_subject_line(invitation['user_name'], invitation['service_name']) aws_ses_client.send_email.assert_called_once_with(email_from, invitation['to'], diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index cfb88c911..b748fa622 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1012,7 +1012,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template app.celery.tasks.send_email.apply_async.assert_called_once_with( (str(sample_email_template.service_id), notification_id, - "sample.service@test.notify.com", + "\"Sample service\" ", "something_encrypted", "2016-01-01T11:09:00.061258"), queue="email"