diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 6e2351fb4..fb5b1ccd7 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -38,7 +38,7 @@ def process_ses_response(ses_request): if notification_type == 'Bounce': notification_type = determine_notification_bounce_type(notification_type, ses_message) elif notification_type == 'Complaint': - handle_complaint(ses_request) + handle_complaint(ses_message) return try: @@ -108,13 +108,12 @@ def remove_emails_from_bounce(bounce_dict): recip.pop('emailAddress') -def handle_complaint(ses_request): - ses_message = json.loads(ses_request['Message']) +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_request['MessageId'] + reference = ses_message['mail']['messageId'] except KeyError as e: current_app.logger.exception("Complaint from SES failed to get reference from message", e) return diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index d8f97096b..73bdbf147 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -7,7 +7,7 @@ from app.notifications.notifications_ses_callback import remove_emails_from_comp from tests.app.db import ( create_notification, ses_complaint_callback, - ses_notification_callback + ses_notification_callback, ) @@ -40,8 +40,7 @@ def test_process_ses_results_retry_called(notify_db, mocker): def test_process_ses_results_in_complaint(sample_email_template, mocker): notification = create_notification(template=sample_email_template, reference='ref1') mocked = mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference") - response = json.loads(ses_complaint_callback()) - process_ses_results(response=response) + process_ses_results(response=ses_complaint_callback()) assert mocked.call_count == 0 complaints = Complaint.query.all() assert len(complaints) == 1 @@ -49,7 +48,6 @@ def test_process_ses_results_in_complaint(sample_email_template, mocker): def test_remove_emails_from_complaint(): - test_message = ses_complaint_callback() - test_json = json.loads(json.loads(test_message)['Message']) + test_json = json.loads(ses_complaint_callback()['Message']) remove_emails_from_complaint(test_json) assert "recipient1@example.com" not in test_json diff --git a/tests/app/complaint/test_complaint_rest.py b/tests/app/complaint/test_complaint_rest.py index d3b352fca..274c071d4 100644 --- a/tests/app/complaint/test_complaint_rest.py +++ b/tests/app/complaint/test_complaint_rest.py @@ -5,20 +5,20 @@ from tests.app.db import create_complaint, create_service, create_template, crea def test_get_all_complaints_returns_list_for_multiple_services_and_complaints(client, notify_db_session): - service=create_service(service_name='service1') + service = create_service(service_name='service1') template = create_template(service=service) notification = create_notification(template=template) complaint_1 = create_complaint() # default service complaint_2 = create_complaint(service=service, notification=notification) - response = client.get('/complaint', headers=[create_authorization_header()]) + response = client.get('/complaint', headers=[create_authorization_header()]) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == [complaint_2.serialize(), complaint_1.serialize()] def test_get_all_complaints_returns_empty_list(client): - response = client.get('/complaint', headers=[create_authorization_header()]) + response = client.get('/complaint', headers=[create_authorization_header()]) assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == [] diff --git a/tests/app/db.py b/tests/app/db.py index 93ff83579..334e4d0a9 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,3 +1,4 @@ +import json from datetime import datetime, date import uuid @@ -399,10 +400,10 @@ def create_monthly_billing_entry( def create_reply_to_email( - service, - email_address, - is_default=True, - archived=False + service, + email_address, + is_default=True, + archived=False ): data = { 'service': service, @@ -419,11 +420,11 @@ def create_reply_to_email( def create_service_sms_sender( - service, - sms_sender, - is_default=True, - inbound_number_id=None, - archived=False + service, + sms_sender, + is_default=True, + inbound_number_id=None, + archived=False ): data = { 'service_id': service.id, @@ -441,10 +442,10 @@ def create_service_sms_sender( def create_letter_contact( - service, - contact_block, - is_default=True, - archived=False + service, + contact_block, + is_default=True, + archived=False ): data = { 'service': service, @@ -567,63 +568,61 @@ def create_ft_billing(bst_date, def create_complaint(service=None, notification=None): - if not service: - service=create_service() + service = create_service() if not notification: template = create_template(service=service, template_type='email') notification = create_notification(template=template) - complaint = Complaint(notification_id=notification.id, - service_id=service.id, - ses_feedback_id=str(uuid.uuid4()), - complaint_type='abuse', - complaint_date=datetime.utcnow() - ) + service_id=service.id, + ses_feedback_id=str(uuid.uuid4()), + complaint_type='abuse', + complaint_date=datetime.utcnow() + ) db.session.add(complaint) db.session.commit() return complaint def ses_complaint_callback_malformed_message_id(): - return '{\n "Type" : "Notification",\n "msgId" : "ref1",' \ - '\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \ - '\n "Message" : "{\\"notificationType\\":\\"Complaint\\",' \ - '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \ - '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \ - '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \ - '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \ - '\\"feedbackId\\":\\"someSESID\\"}}"\n}' - + return { + 'Signature': 'bb', + 'SignatureVersion': '1', 'MessageAttributes': {}, 'MessageId': '98c6e927-af5d-5f3b-9522-bab736f2cbde', + '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 + '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 """ - return '{\n "Type" : "Notification",\n "MessageId" : "ref1",' \ - '\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \ - '\n "Message" : "{\\"notificationType\\":\\"Complaint\\",' \ - '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \ - '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \ - '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \ - '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \ - '\\"feedbackId\\":\\"someSESID\\"}}"\n}' - + return { + 'Signature': 'bb', + 'SignatureVersion': '1', 'MessageAttributes': {}, 'MessageId': '98c6e927-af5d-5f3b-9522-bab736f2cbde', + '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 + 'SigningCertUrl': 'https://sns.pem' + } def ses_complaint_callback(): """ https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object """ - return '{\n "Type" : "Notification",\n "MessageId" : "ref1",' \ - '\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \ - '\n "Message" : "{\\"notificationType\\":\\"Complaint\\",' \ - '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \ - '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \ - '\\"complaintFeedbackType\\":\\"abuse\\", ' \ - '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \ - '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \ - '\\"feedbackId\\":\\"someSESID\\"}}"\n}' + return { + 'Signature': 'bb', + 'SignatureVersion': '1', 'MessageAttributes': {}, 'MessageId': '98c6e927-af5d-5f3b-9522-bab736f2cbde', + '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 + 'SigningCertUrl': 'https://sns.pem' + } def ses_notification_callback(): diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index c508ff5b4..2e5cbe8a6 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -201,23 +201,22 @@ 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') - response = json.loads(ses_complaint_callback()) - handle_complaint(response) + handle_complaint(ses_complaint_callback()) complaints = Complaint.query.all() assert len(complaints) == 1 assert complaints[0].notification_id == notification.id def test_handle_complaint_does_not_raise_exception_if_reference_is_missing(notify_api): - response = json.loads(ses_complaint_callback_malformed_message_id()) + response = json.loads(ses_complaint_callback_malformed_message_id()['Message']) handle_complaint(response) assert len(Complaint.query.all()) == 0 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') - response = json.loads(ses_complaint_callback_with_missing_complaint_type()) - handle_complaint(response) + msg = json.loads(ses_complaint_callback_with_missing_complaint_type()['Message']) + handle_complaint(msg) complaints = Complaint.query.all() assert len(complaints) == 1 assert complaints[0].notification_id == notification.id