diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index fb5b1ccd7..9cfd80bbd 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -111,7 +111,6 @@ def remove_emails_from_bounce(bounce_dict): def handle_complaint(ses_message): remove_emails_from_complaint(ses_message) current_app.logger.info("Complaint from SES: \n{}".format(ses_message)) - # It is possible that the we get a key error, let this fail so we can investigate. try: reference = ses_message['mail']['messageId'] except KeyError as e: @@ -132,6 +131,7 @@ def handle_complaint(ses_message): def remove_emails_from_complaint(complaint_dict): complaint_dict['complaint'].pop('complainedRecipients') + complaint_dict['mail'].pop('destination') def _check_and_queue_callback_task(notification): diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index 73bdbf147..99eb02c2b 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -50,4 +50,4 @@ def test_process_ses_results_in_complaint(sample_email_template, mocker): def test_remove_emails_from_complaint(): test_json = json.loads(ses_complaint_callback()['Message']) remove_emails_from_complaint(test_json) - assert "recipient1@example.com" not in test_json + assert "recipient1@example.com" not in json.dumps(test_json) diff --git a/tests/app/db.py b/tests/app/db.py index 334e4d0a9..5970165b1 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,4 +1,3 @@ -import json from datetime import datetime, date import uuid @@ -592,10 +591,11 @@ def ses_complaint_callback_malformed_message_id(): 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com', 'TopicArn': 'arn:ses_notifications', 'Type': 'Notification', 'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None, - 'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","badMessageId":"ref1","destination":["someone@hotmail.com"]}}', # noqa + 'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","badMessageId":"ref1","destination":["recipient1@example.com"]}}', # noqa 'SigningCertUrl': 'https://sns.pem' } + def ses_complaint_callback_with_missing_complaint_type(): """ https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object @@ -606,10 +606,11 @@ def ses_complaint_callback_with_missing_complaint_type(): 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com', 'TopicArn': 'arn:ses_notifications', 'Type': 'Notification', 'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None, - 'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["someone@hotmail.com"]}}', # noqa + 'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["recipient1@example.com"]}}', # noqa 'SigningCertUrl': 'https://sns.pem' } + def ses_complaint_callback(): """ https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object @@ -620,7 +621,7 @@ def ses_complaint_callback(): 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com', 'TopicArn': 'arn:ses_notifications', 'Type': 'Notification', 'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None, - 'Message': '{"notificationType":"Complaint","complaint":{"complaintFeedbackType": "abuse", "complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["someone@hotmail.com"]}}', # noqa + 'Message': '{"notificationType":"Complaint","complaint":{"complaintFeedbackType": "abuse", "complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" ","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["recipient1@example.com"]}}', # noqa 'SigningCertUrl': 'https://sns.pem' } diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 2e5cbe8a6..82c30e547 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -1,7 +1,9 @@ from datetime import datetime +import pytest from flask import json from freezegun import freeze_time +from sqlalchemy.exc import SQLAlchemyError from app import statsd_client from app.dao.notifications_dao import get_notification_by_id @@ -201,7 +203,7 @@ def test_remove_emails_from_bounce(): def test_process_ses_results_in_complaint(sample_email_template): notification = create_notification(template=sample_email_template, reference='ref1') - handle_complaint(ses_complaint_callback()) + handle_complaint(json.loads(ses_complaint_callback()['Message'])) complaints = Complaint.query.all() assert len(complaints) == 1 assert complaints[0].notification_id == notification.id @@ -213,6 +215,12 @@ def test_handle_complaint_does_not_raise_exception_if_reference_is_missing(notif assert len(Complaint.query.all()) == 0 +def test_handle_complaint_does_raise_exception_if_notification_not_found(notify_api): + response = json.loads(ses_complaint_callback()['Message']) + with pytest.raises(expected_exception=SQLAlchemyError): + handle_complaint(response) + + def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_type(notify_api, sample_email_template): notification = create_notification(template=sample_email_template, reference='ref1') msg = json.loads(ses_complaint_callback_with_missing_complaint_type()['Message'])