Merge pull request #1318 from alphagov/email-validation

ensure emails are formatted before sending

app/delivery/send_to_providers.py

@@ -3,7 +3,8 @@ from datetime import datetime
 
 from flask import current_app
 from notifications_utils.recipients import (
-    validate_and_format_phone_number
+    validate_and_format_phone_number,
+    validate_and_format_email_address
 )
 from notifications_utils.template import HTMLEmailTemplate, PlainTextEmailTemplate, SMSMessageTemplate
 
@@ -130,11 +131,11 @@ def send_email_to_provider(notification):
 
             reference = provider.send_email(
                 from_address,
-                notification.to,
+                validate_and_format_email_address(notification.to),
                 plain_text_email.subject,
                 body=str(plain_text_email),
                 html_body=str(html_email),
-                reply_to_address=email_reply_to,
+                reply_to_address=validate_and_format_email_address(email_reply_to) if email_reply_to else None,
             )
             notification.reference = reference
             update_notification(notification, provider)

requirements.txt

@@ -26,6 +26,6 @@ notifications-python-client==4.4.0
 awscli>=1.11,<1.12
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@21.3.1#egg=notifications-utils==21.3.1
+git+https://github.com/alphagov/notifications-utils.git@21.4.1#egg=notifications-utils==21.4.1
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

tests/app/db.py

@@ -109,7 +109,7 @@ def create_notification(
     template,
     job=None,
     job_row_number=None,
-    to_field='+447700900855',
+    to_field=None,
     status='created',
     reference=None,
     created_at=None,
@@ -131,6 +131,9 @@ def create_notification(
     if created_at is None:
         created_at = datetime.utcnow()
 
+    if to_field is None:
+        to_field = '+447700900855' if template.template_type == SMS_TYPE else 'test@example.com'
+
     if status != 'created':
         sent_at = sent_at or datetime.utcnow()
         updated_at = updated_at or datetime.utcnow()

tests/app/delivery/test_send_to_providers.py

@@ -781,3 +781,61 @@ def test_send_email_to_provider_get_linked_email_reply_to_create_service_email_a
         html_body=ANY,
         reply_to_address=reply_to.email_address
     )
+
+
+def test_send_email_to_provider_should_format_reply_to_email_address(
+        sample_service,
+        sample_email_template,
+        mocker):
+    mocker.patch('app.aws_ses_client.send_email', return_value='reference')
+    mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
+
+    db_notification = create_notification(template=sample_email_template)
+
+    reply_to = create_reply_to_email_for_notification(
+        db_notification.id,
+        sample_service,
+        "test@test.com\t"
+    )
+
+    send_to_providers.send_email_to_provider(
+        db_notification,
+    )
+
+    app.aws_ses_client.send_email.assert_called_once_with(
+        ANY,
+        ANY,
+        ANY,
+        body=ANY,
+        html_body=ANY,
+        reply_to_address="test@test.com"
+    )
+
+
+def test_send_sms_to_provider_should_format_phone_number(sample_notification, mocker):
+    sample_notification.to = '+44 (7123) 123-123'
+    send_mock = mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
+
+    send_to_providers.send_sms_to_provider(sample_notification)
+
+    assert send_mock.call_args[1]['to'] == '447123123123'
+
+
+def test_send_email_to_provider_should_format_email_address(sample_email_notification, mocker):
+    sample_email_notification.to = 'test@example.com\t'
+    send_mock = mocker.patch('app.aws_ses_client.send_email', return_value='reference')
+    mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
+
+    send_to_providers.send_email_to_provider(sample_email_notification)
+
+    # to_addresses
+    send_mock.assert_called_once_with(
+        ANY,
+        # to_addresses
+        'test@example.com',
+        ANY,
+        body=ANY,
+        html_body=ANY,
+        reply_to_address=ANY,
+    )