From d45283d0f4f26b5be0dcaaa0f80204c244508b5a Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sat, 11 Nov 2017 19:18:01 +0000 Subject: [PATCH 1/8] Update sqlalchemy-utils from 0.32.19 to 0.32.21 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index a66ed696c..ff9647162 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,7 @@ marshmallow==2.14.0 monotonic==1.4 psycopg2==2.7.3.2 PyJWT==1.5.3 -SQLAlchemy-Utils==0.32.19 +SQLAlchemy-Utils==0.32.21 SQLAlchemy==1.1.15 statsd==3.2.1 From e8a94422e5658008560c52d3f8413aece351e94f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Nov 2017 16:44:33 +0000 Subject: [PATCH 2/8] Make SMS prefix setting non-nullable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have now: - defaulted new services to start with this column set to `true` - migrated all preexisting services[1] to have either `true` or `false` set for this column There is no way for a service to switch back from `true`/`false` to `null`. This means that we can safely enforce a non-nullable constraint on this column now. 1. There is a little gotcha: the GOV.UK Notify service still relies on the `sms_sender` column. It doesn’t have a row in the `service_sms_senders` table. This means the previous migration never changed this service’s value for `prefix_sms` from `null`. So this commit also changes its value to `False`, so that the rest of the migration, of the whole column, is possible. --- .../versions/0140_sms_prefix_non_nullable.py | 46 +++++++++++++++++++ tests/app/conftest.py | 2 +- tests/app/db.py | 2 +- tests/app/service/test_rest.py | 13 +++++- 4 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 migrations/versions/0140_sms_prefix_non_nullable.py diff --git a/migrations/versions/0140_sms_prefix_non_nullable.py b/migrations/versions/0140_sms_prefix_non_nullable.py new file mode 100644 index 000000000..f65e0a70e --- /dev/null +++ b/migrations/versions/0140_sms_prefix_non_nullable.py @@ -0,0 +1,46 @@ +""" + +Revision ID: 0140_sms_prefix_non_nullable +Revises: 0139_migrate_sms_allowance_data +Create Date: 2017-11-07 13:04:04.077142 + +""" +from alembic import op +from flask import current_app +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0140_sms_prefix_non_nullable' +down_revision = '0139_migrate_sms_allowance_data' + + +def upgrade(): + + op.execute(""" + update services + set prefix_sms = false + where id = '{}' + """.format(current_app.config['NOTIFY_SERVICE_ID'])) + + op.alter_column( + 'services', + 'prefix_sms', + existing_type=sa.BOOLEAN(), + nullable=False, + ) + + +def downgrade(): + + op.alter_column( + 'services', + 'prefix_sms', + existing_type=sa.BOOLEAN(), + nullable=True, + ) + + op.execute(""" + update services + set prefix_sms = null + where id = '{}' + """.format(current_app.config['NOTIFY_SERVICE_ID'])) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index cecfb5a27..948ba20cb 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -992,7 +992,7 @@ def notify_service(notify_db, notify_db_session): 'active': True, 'restricted': False, 'email_from': 'notify.service', - 'created_by': user + 'created_by': user, } service = Service(**data) db.session.add(service) diff --git a/tests/app/db.py b/tests/app/db.py index 8a22a748c..0457b3c0a 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -68,7 +68,7 @@ def create_service( research_mode=False, active=True, email_from=None, - prefix_sms=None, + prefix_sms=True, ): service = Service( name=service_name, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index f7655be72..5e3fdddb6 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1551,7 +1551,6 @@ def test_prefixing_messages_based_on_prefix_sms( @pytest.mark.parametrize('posted_value, stored_value, returned_value', [ (True, True, True), (False, False, False), - (None, None, True), ]) def test_set_sms_prefixing_for_service( client, @@ -1581,6 +1580,18 @@ def test_set_sms_prefixing_for_service( assert result['data']['sms_sender'] == current_app.config['FROM_NUMBER'] +def test_set_sms_prefixing_for_service_cant_be_none( + admin_request, + sample_service, +): + admin_request.post( + 'service.update_service', + service_id=sample_service.id, + _data={'prefix_sms': None}, + _expected_status=500, + ) + + @pytest.mark.parametrize('today_only,stats', [ ('False', {'requested': 2, 'delivered': 1, 'failed': 0}), ('True', {'requested': 1, 'delivered': 0, 'failed': 0}) From 4eaa482b2d5f1ea8c68f6c69570ad56e4e534efe Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 10 Nov 2017 08:42:00 +0000 Subject: [PATCH 3/8] Rewrite existing test to use admin_request fixture Makes it consistent with the other, related, test in this PR. --- tests/app/service/test_rest.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 5e3fdddb6..2df3ef0da 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1553,25 +1553,18 @@ def test_prefixing_messages_based_on_prefix_sms( (False, False, False), ]) def test_set_sms_prefixing_for_service( + admin_request, client, sample_service, posted_value, stored_value, returned_value, ): - data = { - 'prefix_sms': posted_value, - } - - auth_header = create_authorization_header() - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] + result = admin_request.post( + 'service.update_service', + service_id=sample_service.id, + _data={'prefix_sms': posted_value}, ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 assert result['data']['prefix_sms'] == stored_value # This derived value will go away eventually, once we’ve done a migration assert result['data']['prefix_sms_with_service_name'] == returned_value From 492462d4f9b132afee969285f328723ce83c72aa Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 10 Nov 2017 08:49:48 +0000 Subject: [PATCH 4/8] Make getting a service return `prefix_sms` column This gives us the confidence to remove references to the computed column in https://github.com/alphagov/notifications-admin/pull/1619 --- tests/app/service/test_rest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 2df3ef0da..1d4ffe233 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -159,6 +159,7 @@ def test_get_service_by_id(client, sample_service): assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] assert json_resp['data']['prefix_sms_with_service_name'] is True + assert json_resp['data']['prefix_sms'] is True def test_get_service_by_id_returns_free_sms_limit(client, sample_service): From d2154451e56f70b6792078d002dbd15f1cd34e67 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 17 Nov 2017 13:41:45 +0000 Subject: [PATCH 5/8] update research mode email callbacks to add process-ses-response task to queue this involved: * moving that task to callback_tasks to prevent circular imports * updating the dummy research mode callbacks (with actual SNS messages from the ses simulator emails) * refactoring tests --- app/celery/callback_tasks.py | 18 ++++++ app/celery/research_mode_tasks.py | 45 ++++++++++++-- app/celery/tasks.py | 13 ----- app/delivery/send_to_providers.py | 2 +- .../notifications_ses_callback.py | 7 ++- tests/app/celery/test_callback_tasks.py | 58 +++++++++++++++++++ tests/app/celery/test_provider_tasks.py | 5 -- tests/app/celery/test_research_mode_tasks.py | 33 ++++------- tests/app/celery/test_tasks.py | 58 +------------------ tests/app/delivery/test_send_to_providers.py | 2 +- .../test_notifications_ses_callback.py | 23 +++++++- 11 files changed, 158 insertions(+), 106 deletions(-) create mode 100644 app/celery/callback_tasks.py create mode 100644 tests/app/celery/test_callback_tasks.py diff --git a/app/celery/callback_tasks.py b/app/celery/callback_tasks.py new file mode 100644 index 000000000..3de4b746e --- /dev/null +++ b/app/celery/callback_tasks.py @@ -0,0 +1,18 @@ +from flask import current_app + +from app import notify_celery +from app.config import QueueNames +from app.statsd_decorators import statsd +from app.notifications.notifications_ses_callback import process_ses_response + + +@notify_celery.task(bind=True, name="process-ses-result", max_retries=5, default_retry_delay=300) +@statsd(namespace="tasks") +def process_ses_results(self, response): + try: + errors = process_ses_response(response) + if errors: + current_app.logger.error(errors) + except Exception: + current_app.logger.exception('Error processing SES results') + self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error") diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index ced35b1d4..1d35cb3cc 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -1,10 +1,11 @@ import json from flask import current_app -from app import notify_celery from requests import request, RequestException, HTTPError from app.models import SMS_TYPE +from app.config import QueueNames +from app.celery.callback_tasks import process_ses_results temp_fail = "7700900003" perm_fail = "7700900002" @@ -36,7 +37,7 @@ def send_sms_response(provider, reference, to): make_request(SMS_TYPE, provider, body, headers) -def send_email_response(provider, reference, to): +def send_email_response(reference, to): if to == perm_fail_email: body = ses_hard_bounce_callback(reference) elif to == temp_fail_email: @@ -44,7 +45,7 @@ def send_email_response(provider, reference, to): else: body = ses_notification_callback(reference) - make_request('email', provider, body, headers={"Content-type": "application/json"}) + process_ses_results.apply_async([body], queue=QueueNames.RESEARCH_MODE) def make_request(notification_type, provider, data, headers): @@ -115,12 +116,44 @@ def firetext_callback(notification_id, to): def ses_notification_callback(reference): - return '{ "Type" : "Notification", "MessageId" : "%s", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"%s\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' % (reference, reference) # noqa + return { + 'EventSource': 'aws:sns', + 'EventVersion': '1.0', + 'EventSubscriptionArn': 'arn:aws:sns:eu-west-1:302763885840:ses_notifications:27447d51-7008-4d9c-83f2-519983b60937', + 'Sns': { + 'Type': 'Notification', + 'MessageId': '8e83c020-3a50-5957-a2ca-92a8ee9baa0a', + 'TopicArn': 'arn:aws:sns:eu-west-1:302763885840:ses_notifications', + 'Subject': None, + 'Message': '''{"notificationType":"Delivery","mail":{"timestamp":"2017-11-17T12:14:01.643Z","source":"\\"sakis\\" ","sourceArn":"arn:aws:ses:eu-west-1:302763885840:identity/notify.works","sourceIp":"52.208.24.161","sendingAccountId":"302763885840","messageId":"0102015fc9e669ab-d1395dba-84c7-4311-9f25-405396a3f7aa-000000","destination":["success@simulator.amazonses.com"],"headersTruncated":false,"headers":[{"name":"From","value":"sakis "},{"name":"To","value":"success@simulator.amazonses.com"},{"name":"Subject","value":"lambda test"},{"name":"MIME-Version","value":"1.0"},{"name":"Content-Type","value":"multipart/alternative; boundary=\\"----=_Part_617203_1627511946.1510920841645\\""}],"commonHeaders":{"from":["sakis "],"to":["success@simulator.amazonses.com"],"subject":"lambda test"}},"delivery":{"timestamp":"2017-11-17T12:14:03.646Z","processingTimeMillis":2003,"recipients":["success@simulator.amazonses.com"],"smtpResponse":"250 2.6.0 Message received","remoteMtaIp":"207.171.163.188","reportingMTA":"a7-32.smtp-out.eu-west-1.amazonses.com"}}', 'Timestamp': '2017-11-17T12:14:03.710Z', 'SignatureVersion': '1', 'Signature': 'IxQPpK5kHVSiYhFqWjH35ElZSUOkE29hDcdCjFrA6Cx51Fw5ZNyFGsYJQsCskVEIXteTrn/9VU9zeW5oSf81dbGzv5GnFF4iq8hq+WISQ3etVGx9cOzRABudt82okoIPLU71dsENwj3scibVvsBSP8vD4NJsnOXVfVo1CczumbDT601dFomF45u1bRpg684zUOxvZBpStUfkFaBkDrWp9yt6j5SuDx+AqC0nuQdGPN0+LFbLXN20SZmUwqDiX89xm2JlPBaWimnm0/jBRAqLkSJcix7ssD6ELsCIebljzggibnKzQo3vQV1Frji6+713WlYC7lnziNhVT1VL1tCrhA==', 'SigningCertUrl': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-433026a4050d206028891664da859041.pem', 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:ses_notifications:27447d51-7008-4d9c-83f2-519983b60937''', # noqa + 'MessageAttributes': {} + } + } def ses_hard_bounce_callback(reference): - return '{ "Type" : "Notification", "MessageId" : "%s", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Permanent\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"%s\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' % (reference, reference) # noqa + return { + 'Message': '{"notificationType":"Bounce","bounce":{"bounceType":"Permanent","bounceSubType":"General"}, "mail":{"messageId":"%s","timestamp":"2016-03-14T12:35:25.909Z","source":"test@test-domain.com","sourceArn":"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify","sendingAccountId":"123456789012","destination":["testing@digital.cabinet-office.gov.uk"]},"delivery":{"timestamp":"2016-03-14T12:35:26.567Z","processingTimeMillis":658,"recipients":["testing@digital.cabinet-office.gov.uk"],"smtpResponse":"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp","reportingMTA":"a6-238.smtp-out.eu-west-1.amazonses.com"}}' % reference, # noqa + 'MessageId': reference, + 'Signature': 'X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==', # noqa + 'SignatureVersion': '1', + 'SigningCertURL': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem', # noqa + 'Timestamp': '2016-03-14T12:35:26.665Z', + 'TopicArn': 'arn:aws:sns:eu-west-1:123456789012:testing', + 'Type': 'Notification', + 'UnsubscribeURL': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da' # noqa + } def ses_soft_bounce_callback(reference): - return '{ "Type" : "Notification", "MessageId" : "%s", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Undetermined\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"%s\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' % (reference, reference) # noqa + return { + 'Message': '{"notificationType":"Bounce","bounce":{"bounceType":"Temporary","bounceSubType":"General"}, "mail":{"messageId":"%s","timestamp":"2016-03-14T12:35:25.909Z","source":"test@test-domain.com","sourceArn":"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify","sendingAccountId":"123456789012","destination":["testing@digital.cabinet-office.gov.uk"]},"delivery":{"timestamp":"2016-03-14T12:35:26.567Z","processingTimeMillis":658,"recipients":["testing@digital.cabinet-office.gov.uk"],"smtpResponse":"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp","reportingMTA":"a6-238.smtp-out.eu-west-1.amazonses.com"}}' % reference, # noqa + 'MessageId': reference, + 'Signature': 'X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==', # noqa + 'SignatureVersion': '1', + 'SigningCertURL': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem', # noqa + 'Timestamp': '2016-03-14T12:35:26.665Z', + 'TopicArn': 'arn:aws:sns:eu-west-1:123456789012:testing', + 'Type': 'Notification', + 'UnsubscribeURL': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da' # noqa + } diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9935ad609..436def14d 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -63,7 +63,6 @@ from app.models import ( SMS_TYPE, ) from app.notifications.process_notifications import persist_notification -from app.notifications.notifications_ses_callback import process_ses_response from app.service.utils import service_allowed_to_send_to from app.statsd_decorators import statsd from notifications_utils.s3 import s3upload @@ -562,15 +561,3 @@ def process_incomplete_job(job_id): process_row(row_number, recipient, personalisation, template, job, job.service) job_complete(job, job.service, template, resumed=True) - - -@notify_celery.task(bind=True, name="process-ses-result", max_retries=5, default_retry_delay=300) -@statsd(namespace="tasks") -def process_ses_results(self, response): - try: - errors = process_ses_response(response) - if errors: - current_app.logger.error(errors) - except Exception: - current_app.logger.exception('Error processing SES results') - self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error") diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index ab56a7e05..45d35e64d 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -126,7 +126,7 @@ def send_email_to_provider(notification): notification.billable_units = 0 notification.reference = reference update_notification(notification, provider) - send_email_response(provider.get_name(), reference, notification.to) + send_email_response(reference, notification.to) else: from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 0418a6329..b35704304 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -28,7 +28,7 @@ def process_ses_response(ses_request): notification_type = ses_message['notificationType'] if notification_type == 'Bounce': - current_app.logger.info('SES bounce dict: {}'.format(ses_message['bounce'])) + current_app.logger.info('SES bounce dict: {}'.format(remove_emails_from_bounce(ses_message['bounce']))) if ses_message['bounce']['bounceType'] == 'Permanent': notification_type = ses_message['bounce']['bounceType'] # permanent or not else: @@ -85,3 +85,8 @@ def process_ses_response(ses_request): except ValueError: error = "{} callback failed: invalid json".format(client_name) return error + + +def remove_emails_from_bounce(bounce_dict): + for recip in bounce_dict['bouncedRecipients']: + recip.pop('emailAddress') diff --git a/tests/app/celery/test_callback_tasks.py b/tests/app/celery/test_callback_tasks.py new file mode 100644 index 000000000..4b5ce4547 --- /dev/null +++ b/tests/app/celery/test_callback_tasks.py @@ -0,0 +1,58 @@ +import json +from datetime import datetime + +from app.celery.callback_tasks import process_ses_results + +from tests.app.db import create_notification + + +def test_process_ses_results(sample_email_template): + create_notification( + sample_email_template, + reference='ref1', + sent_at=datetime.utcnow(), + status='sending') + + response = json.loads(ses_notification_callback()) + assert process_ses_results(response=response) is None + + +def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): + mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + response = json.loads(ses_notification_callback()) + process_ses_results(response=response) + assert mocked.call_count == 0 + + +def test_process_ses_results_retry_called(notify_db, mocker): + mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED")) + mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + response = json.loads(ses_notification_callback()) + process_ses_results(response=response) + assert mocked.call_count != 0 + + +def ses_notification_callback(): + return '{\n "Type" : "Notification",\n "MessageId" : "ref1",' \ + '\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \ + '\n "Message" : "{\\"notificationType\\":\\"Delivery\\",' \ + '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \ + '\\"source\\":\\"test@test-domain.com\\",' \ + '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \ + '\\"sendingAccountId\\":\\"123456789012\\",' \ + '\\"messageId\\":\\"ref1\\",' \ + '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \ + '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \ + '\\"processingTimeMillis\\":658,' \ + '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \ + '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \ + '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \ + '\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",' \ + '\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \ + 'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \ + 'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \ + 'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \ + '\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \ + 'dd426d95ee9390147a5624348ee.pem",' \ + '\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \ + 'ubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index b5dbc999a..5b640a7ec 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -12,8 +12,6 @@ def test_should_have_decorated_tasks_functions(): def test_should_call_send_sms_to_provider_from_deliver_sms_task( - notify_db, - notify_db_session, sample_notification, mocker): mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') @@ -23,7 +21,6 @@ def test_should_call_send_sms_to_provider_from_deliver_sms_task( def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_sms_task( - notify_db, notify_db_session, mocker): mocker.patch('app.delivery.send_to_providers.send_sms_to_provider') @@ -37,8 +34,6 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_sms_task def test_should_call_send_email_to_provider_from_deliver_email_task( - notify_db, - notify_db_session, sample_notification, mocker): mocker.patch('app.delivery.send_to_providers.send_email_to_provider') diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index b56de3bb4..2c6bed80e 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -1,5 +1,10 @@ +import uuid +from unittest.mock import ANY + import pytest from flask import json + +from app.config import QueueNames from app.celery.research_mode_tasks import ( send_sms_response, send_email_response, @@ -42,16 +47,14 @@ def test_make_firetext_callback(notify_api, rmock, phone_number): assert 'mobile={}'.format(phone_number) in rmock.request_history[0].text -def test_make_ses_callback(notify_api, rmock): - endpoint = "http://localhost:6011/notifications/email/ses" - rmock.request( - "POST", - endpoint, - json={"status": "success"}, - status_code=200) - send_email_response("ses", "1234", "test@test.com") +def test_make_ses_callback(notify_api, mocker): + mock_task = mocker.patch('app.celery.research_mode_tasks.process_ses_results') + some_ref = str(uuid.uuid4()) - assert rmock.called + send_email_response(reference=some_ref, to="test@test.com") + + mock_task.apply_async.assert_called_once_with(ANY, queue=QueueNames.RESEARCH_MODE) + assert mock_task.apply_async.call_args[0][0][0] == ses_notification_callback(some_ref) @pytest.mark.parametrize("phone_number", ["07700900001", "+447700900001", "7700900001", "+44 7700900001", @@ -101,15 +104,3 @@ def test_failure_firetext_callback(phone_number): 'time': '2016-03-10 14:17:00', 'reference': '1234' } - - -def test_delivered_ses_callback(): - assert ses_notification_callback("my-reference") == '{ "Type" : "Notification", "MessageId" : "my-reference", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"my-reference\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' # noqa - - -def test_ses_hard_bounce_callback(): - assert ses_hard_bounce_callback("my-reference") == '{ "Type" : "Notification", "MessageId" : "my-reference", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Permanent\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"my-reference\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' # noqa - - -def ses_soft_bounce_callback(): - assert ses_soft_bounce_callback("my-reference") == '{ "Type" : "Notification", "MessageId" : "my-reference", "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing", "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Undetermined\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"%s\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}", "Timestamp" : "2016-03-14T12:35:26.665Z", "SignatureVersion" : "1", "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==", "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem", "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"}' # noqa diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 5267dd436..b3a046897 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -28,8 +28,7 @@ from app.celery.tasks import ( process_incomplete_jobs, get_template_class, s3, - send_inbound_sms_to_service, - process_ses_results) + send_inbound_sms_to_service) from app.config import QueueNames from app.dao import jobs_dao, services_dao from app.models import ( @@ -1403,58 +1402,3 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): assert completed_job.job_status == JOB_STATUS_FINISHED assert mock_letter_saver.call_count == 8 - - -def test_process_ses_results(notify_db, notify_db_session, sample_email_template): - - create_sample_notification( - notify_db, - notify_db_session, - template=sample_email_template, - reference='ref1', - sent_at=datetime.utcnow(), - status='sending') - - response = json.loads(ses_notification_callback()) - assert process_ses_results(response=response) is None - - -def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): - mocked = mocker.patch('app.celery.tasks.process_ses_results.retry') - response = json.loads(ses_notification_callback()) - process_ses_results(response=response) - assert mocked.call_count == 0 - - -def test_process_ses_results_retry_called(notify_db, mocker): - mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED")) - mocked = mocker.patch('app.celery.tasks.process_ses_results.retry') - response = json.loads(ses_notification_callback()) - process_ses_results(response=response) - assert mocked.call_count != 0 - - -def ses_notification_callback(): - return '{\n "Type" : "Notification",\n "MessageId" : "ref1",' \ - '\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \ - '\n "Message" : "{\\"notificationType\\":\\"Delivery\\",' \ - '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \ - '\\"source\\":\\"test@test-domain.com\\",' \ - '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \ - '\\"sendingAccountId\\":\\"123456789012\\",' \ - '\\"messageId\\":\\"ref1\\",' \ - '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \ - '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \ - '\\"processingTimeMillis\\":658,' \ - '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \ - '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \ - '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \ - '\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",' \ - '\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \ - 'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \ - 'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \ - 'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \ - '\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \ - 'dd426d95ee9390147a5624348ee.pem",' \ - '\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \ - 'ubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 64684bf5b..5779a55c3 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -373,7 +373,7 @@ def test_send_email_to_provider_should_call_research_mode_task_response_task_if_ assert not app.aws_ses_client.send_email.called stats_mock.assert_called_once_with(notification) - app.delivery.send_to_providers.send_email_response.assert_called_once_with('ses', str(reference), 'john@smith.com') + app.delivery.send_to_providers.send_email_response.assert_called_once_with(str(reference), 'john@smith.com') persisted_notification = Notification.query.filter_by(id=notification.id).one() assert persisted_notification.to == 'john@smith.com' assert persisted_notification.template_id == sample_email_template.id diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 925d9871e..dfcf6b6af 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -6,7 +6,7 @@ from freezegun import freeze_time from app import statsd_client from app.dao.notifications_dao import get_notification_by_id -from app.notifications.notifications_ses_callback import process_ses_response +from app.notifications.notifications_ses_callback import process_ses_response, remove_emails_from_bounce from tests.app.conftest import sample_notification as create_sample_notification @@ -180,6 +180,27 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, stats_mock.assert_called_once_with(notification) +def test_remove_emails_from_bounce(): + # an actual bouncedict example + message_dict = { + 'feedbackId': '0102015fc9acfc14-12341234-1234-1234-1234-123412341234-000000', + 'bounceType': 'Permanent', + 'reportingMTA': 'dsn; a1-23.smtp-out.eu-west-4.amazonses.com', + 'remoteMtaIp': '123.123.123.123', + 'bounceSubType': 'General', + 'bouncedRecipients': [{ + 'diagnosticCode': "smtp; 550-5.1.1 The email account that you tried to reach does not exist. Please try\n550-5.1.1 double-checking the recipient's email address for typos or\n550-5.1.1 unnecessary spaces. Learn more at\n550 5.1.1 https://support.google.com/mail/?p=NoSuchUser x33si3064453edx.66 - gsmtp", # noqa + 'action': 'failed', + 'status': '5.1.1', + 'emailAddress': 'not-real@gmail.com'}], + 'timestamp': '2017-11-17T11:11:18.098Z' + } + + remove_emails_from_bounce(message_dict['bounce']) + + assert 'not-real@gmail.com' not in json.dumps(message_dict) + + def ses_notification_callback(ref='ref'): return str.encode( '{\n "Type" : "Notification",\n "MessageId" : "%(ref)s",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"%(ref)s\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' % {'ref': ref} # noqa From 04e321808769778e7523fa4e95484409981836ee Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 17 Nov 2017 14:25:59 +0000 Subject: [PATCH 6/8] ensure sample SES callbacks and SNS subscription payloads are accurate in research mode and unit tests --- app/celery/research_mode_tasks.py | 163 ++++++++++++++---- .../app/notifications/rest/test_callbacks.py | 9 +- .../test_notifications_ses_callback.py | 78 +-------- 3 files changed, 146 insertions(+), 104 deletions(-) diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index 1d35cb3cc..b37062826 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -116,44 +116,143 @@ def firetext_callback(notification_id, to): def ses_notification_callback(reference): + ses_message_body = { + 'delivery': { + 'processingTimeMillis': 2003, + 'recipients': ['success@simulator.amazonses.com'], + 'remoteMtaIp': '123.123.123.123', + 'reportingMTA': 'a7-32.smtp-out.eu-west-1.amazonses.com', + 'smtpResponse': '250 2.6.0 Message received', + 'timestamp': '2017-11-17T12:14:03.646Z' + }, + 'mail': { + 'commonHeaders': { + 'from': ['TEST '], + 'subject': 'lambda test', + 'to': ['success@simulator.amazonses.com'] + }, + 'destination': ['success@simulator.amazonses.com'], + 'headers': [ + { + 'name': 'From', + 'value': 'TEST ' + }, + { + 'name': 'To', + 'value': 'success@simulator.amazonses.com' + }, + { + 'name': 'Subject', + 'value': 'lambda test' + }, + { + 'name': 'MIME-Version', + 'value': '1.0' + }, + { + 'name': 'Content-Type', + 'value': 'multipart/alternative; boundary="----=_Part_617203_1627511946.1510920841645"' + } + ], + 'headersTruncated': False, + 'messageId': reference, + 'sendingAccountId': '12341234', + 'source': '"TEST" ', + 'sourceArn': 'arn:aws:ses:eu-west-1:12341234:identity/notify.works', + 'sourceIp': '0.0.0.1', + 'timestamp': '2017-11-17T12:14:01.643Z' + }, + 'notificationType': 'Delivery' + } + return { - 'EventSource': 'aws:sns', - 'EventVersion': '1.0', - 'EventSubscriptionArn': 'arn:aws:sns:eu-west-1:302763885840:ses_notifications:27447d51-7008-4d9c-83f2-519983b60937', - 'Sns': { - 'Type': 'Notification', - 'MessageId': '8e83c020-3a50-5957-a2ca-92a8ee9baa0a', - 'TopicArn': 'arn:aws:sns:eu-west-1:302763885840:ses_notifications', - 'Subject': None, - 'Message': '''{"notificationType":"Delivery","mail":{"timestamp":"2017-11-17T12:14:01.643Z","source":"\\"sakis\\" ","sourceArn":"arn:aws:ses:eu-west-1:302763885840:identity/notify.works","sourceIp":"52.208.24.161","sendingAccountId":"302763885840","messageId":"0102015fc9e669ab-d1395dba-84c7-4311-9f25-405396a3f7aa-000000","destination":["success@simulator.amazonses.com"],"headersTruncated":false,"headers":[{"name":"From","value":"sakis "},{"name":"To","value":"success@simulator.amazonses.com"},{"name":"Subject","value":"lambda test"},{"name":"MIME-Version","value":"1.0"},{"name":"Content-Type","value":"multipart/alternative; boundary=\\"----=_Part_617203_1627511946.1510920841645\\""}],"commonHeaders":{"from":["sakis "],"to":["success@simulator.amazonses.com"],"subject":"lambda test"}},"delivery":{"timestamp":"2017-11-17T12:14:03.646Z","processingTimeMillis":2003,"recipients":["success@simulator.amazonses.com"],"smtpResponse":"250 2.6.0 Message received","remoteMtaIp":"207.171.163.188","reportingMTA":"a7-32.smtp-out.eu-west-1.amazonses.com"}}', 'Timestamp': '2017-11-17T12:14:03.710Z', 'SignatureVersion': '1', 'Signature': 'IxQPpK5kHVSiYhFqWjH35ElZSUOkE29hDcdCjFrA6Cx51Fw5ZNyFGsYJQsCskVEIXteTrn/9VU9zeW5oSf81dbGzv5GnFF4iq8hq+WISQ3etVGx9cOzRABudt82okoIPLU71dsENwj3scibVvsBSP8vD4NJsnOXVfVo1CczumbDT601dFomF45u1bRpg684zUOxvZBpStUfkFaBkDrWp9yt6j5SuDx+AqC0nuQdGPN0+LFbLXN20SZmUwqDiX89xm2JlPBaWimnm0/jBRAqLkSJcix7ssD6ELsCIebljzggibnKzQo3vQV1Frji6+713WlYC7lnziNhVT1VL1tCrhA==', 'SigningCertUrl': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-433026a4050d206028891664da859041.pem', 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:ses_notifications:27447d51-7008-4d9c-83f2-519983b60937''', # noqa - 'MessageAttributes': {} - } + 'Type': 'Notification', + 'MessageId': '8e83c020-1234-1234-1234-92a8ee9baa0a', + 'TopicArn': 'arn:aws:sns:eu-west-1:12341234:ses_notifications', + 'Subject': None, + 'Message': json.dumps(ses_message_body), + 'Timestamp': '2017-11-17T12:14:03.710Z', + 'SignatureVersion': '1', + 'Signature': '[REDACTED]', + 'SigningCertUrl': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-[REDACTED].pem', + 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=[REACTED]', + 'MessageAttributes': {} } def ses_hard_bounce_callback(reference): - return { - 'Message': '{"notificationType":"Bounce","bounce":{"bounceType":"Permanent","bounceSubType":"General"}, "mail":{"messageId":"%s","timestamp":"2016-03-14T12:35:25.909Z","source":"test@test-domain.com","sourceArn":"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify","sendingAccountId":"123456789012","destination":["testing@digital.cabinet-office.gov.uk"]},"delivery":{"timestamp":"2016-03-14T12:35:26.567Z","processingTimeMillis":658,"recipients":["testing@digital.cabinet-office.gov.uk"],"smtpResponse":"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp","reportingMTA":"a6-238.smtp-out.eu-west-1.amazonses.com"}}' % reference, # noqa - 'MessageId': reference, - 'Signature': 'X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==', # noqa - 'SignatureVersion': '1', - 'SigningCertURL': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem', # noqa - 'Timestamp': '2016-03-14T12:35:26.665Z', - 'TopicArn': 'arn:aws:sns:eu-west-1:123456789012:testing', - 'Type': 'Notification', - 'UnsubscribeURL': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da' # noqa - } + return _ses_bounce_callback(reference, 'Permanent') def ses_soft_bounce_callback(reference): - return { - 'Message': '{"notificationType":"Bounce","bounce":{"bounceType":"Temporary","bounceSubType":"General"}, "mail":{"messageId":"%s","timestamp":"2016-03-14T12:35:25.909Z","source":"test@test-domain.com","sourceArn":"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify","sendingAccountId":"123456789012","destination":["testing@digital.cabinet-office.gov.uk"]},"delivery":{"timestamp":"2016-03-14T12:35:26.567Z","processingTimeMillis":658,"recipients":["testing@digital.cabinet-office.gov.uk"],"smtpResponse":"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp","reportingMTA":"a6-238.smtp-out.eu-west-1.amazonses.com"}}' % reference, # noqa - 'MessageId': reference, - 'Signature': 'X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==', # noqa - 'SignatureVersion': '1', - 'SigningCertURL': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem', # noqa - 'Timestamp': '2016-03-14T12:35:26.665Z', - 'TopicArn': 'arn:aws:sns:eu-west-1:123456789012:testing', - 'Type': 'Notification', - 'UnsubscribeURL': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da' # noqa + return _ses_bounce_callback(reference, 'Temporary') + + +def _ses_bounce_callback(reference, bounce_type): + ses_message_body = { + 'bounce': { + 'bounceSubType': 'General', + 'bounceType': bounce_type, + 'bouncedRecipients': [{ + 'action': 'failed', + 'diagnosticCode': 'smtp; 550 5.1.1 user unknown', + 'emailAddress': 'bounce@simulator.amazonses.com', + 'status': '5.1.1' + }], + 'feedbackId': '0102015fc9e676fb-12341234-1234-1234-1234-9301e86a4fa8-000000', + 'remoteMtaIp': '123.123.123.123', + 'reportingMTA': 'dsn; a7-31.smtp-out.eu-west-1.amazonses.com', + 'timestamp': '2017-11-17T12:14:05.131Z' + }, + 'mail': { + 'commonHeaders': { + 'from': ['TEST '], + 'subject': 'ses callback test', + 'to': ['bounce@simulator.amazonses.com'] + }, + 'destination': ['bounce@simulator.amazonses.com'], + 'headers': [ + { + 'name': 'From', + 'value': 'TEST ' + }, + { + 'name': 'To', + 'value': 'bounce@simulator.amazonses.com' + }, + { + 'name': 'Subject', + 'value': 'lambda test' + }, + { + 'name': 'MIME-Version', + 'value': '1.0' + }, + { + 'name': 'Content-Type', + 'value': 'multipart/alternative; boundary="----=_Part_596529_2039165601.1510920843367"' + } + ], + 'headersTruncated': False, + 'messageId': reference, + 'sendingAccountId': '12341234', + 'source': '"TEST" ', + 'sourceArn': 'arn:aws:ses:eu-west-1:12341234:identity/notify.works', + 'sourceIp': '0.0.0.1', + 'timestamp': '2017-11-17T12:14:03.000Z' + }, + 'notificationType': 'Bounce' + } + return { + 'Type': 'Notification', + 'MessageId': '36e67c28-1234-1234-1234-2ea0172aa4a7', + 'TopicArn': 'arn:aws:sns:eu-west-1:12341234:ses_notifications', + 'Subject': None, + 'Message': json.dumps(ses_message_body), + 'Timestamp': '2017-11-17T12:14:05.149Z', + 'SignatureVersion': '1', + 'Signature': '[REDACTED]', # noqa + 'SigningCertUrl': 'https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-[REDACTED]].pem', + 'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=[REDACTED]]', + 'MessageAttributes': {} } diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index ff2cb417e..73d33c7a8 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -9,7 +9,6 @@ import app.celery.tasks from app.dao.notifications_dao import ( get_notification_by_id ) -from tests.app.notifications.test_notifications_ses_callback import ses_confirmation_callback from tests.app.conftest import sample_notification as create_sample_notification @@ -50,7 +49,7 @@ def test_dvla_callback_returns_400_with_invalid_request(client): def test_dvla_callback_autoconfirms_subscription(client, mocker): autoconfirm_mock = mocker.patch('app.notifications.notifications_letter_callback.autoconfirm_subscription') - data = ses_confirmation_callback() + data = _sns_confirmation_callback() response = dvla_post(client, data) assert response.status_code == 200 assert autoconfirm_mock.called @@ -61,7 +60,7 @@ def test_dvla_callback_autoconfirm_does_not_call_update_letter_notifications_tas update_task = \ mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async') - data = ses_confirmation_callback() + data = _sns_confirmation_callback() response = dvla_post(client, data) assert response.status_code == 200 @@ -443,3 +442,7 @@ def _sample_sns_s3_callback(): "TopicArn": "sample-topic-arn", "Message": '{"Records":[{"eventVersion":"2.0","eventSource":"aws:s3","awsRegion":"eu-west-1","eventTime":"2017-05-16T11:38:41.073Z","eventName":"ObjectCreated:Put","userIdentity":{"principalId":"some-p-id"},"requestParameters":{"sourceIPAddress":"8.8.8.8"},"responseElements":{"x-amz-request-id":"some-r-id","x-amz-id-2":"some-x-am-id"},"s3":{"s3SchemaVersion":"1.0","configurationId":"some-c-id","bucket":{"name":"some-bucket","ownerIdentity":{"principalId":"some-p-id"},"arn":"some-bucket-arn"},"object":{"key":"bar.txt","size":200,"eTag":"some-e-tag","versionId":"some-v-id","sequencer":"some-seq"}}}]}' # noqa }) + + +def _sns_confirmation_callback(): + return b'{\n "Type": "SubscriptionConfirmation",\n "MessageId": "165545c9-2a5c-472c-8df2-7ff2be2b3b1b",\n "Token": "2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n "TopicArn": "arn:aws:sns:us-west-2:123456789012:MyTopic",\n "Message": "You have chosen to subscribe to the topic arn:aws:sns:us-west-2:123456789012:MyTopic.\\nTo confirm the subscription, visit the SubscribeURL included in this message.",\n "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=arn:aws:sns:us-west-2:123456789012:MyTopic&Token=2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n "Timestamp": "2012-04-26T20:45:04.751Z",\n "SignatureVersion": "1",\n "Signature": "EXAMPLEpH+DcEwjAPg8O9mY8dReBSwksfg2S7WKQcikcNKWLQjwu6A4VbeS0QHVCkhRS7fUQvi2egU3N858fiTDN6bkkOxYDVrY0Ad8L10Hs3zH81mtnPk5uvvolIC1CXGu43obcgFxeL3khZl8IKvO61GWB6jI9b5+gLPoBc1Q=",\n "SigningCertURL": "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-f3ecfb7224c7233fe7bb5f59f96de52f.pem"\n}' # noqa diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index dfcf6b6af..330466234 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -7,29 +7,11 @@ from freezegun import freeze_time from app import statsd_client from app.dao.notifications_dao import get_notification_by_id from app.notifications.notifications_ses_callback import process_ses_response, remove_emails_from_bounce +from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback + from tests.app.conftest import sample_notification as create_sample_notification -def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker): - stats_mock = mocker.patch( - 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' - ) - - errors = process_ses_response(json.loads(ses_invalid_notification_type_callback())) - assert errors == 'SES callback failed: status Unknown not found' - stats_mock.assert_not_called() - - -def test_ses_callback_should_fail_if_missing_message_id(client, mocker): - stats_mock = mocker.patch( - 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' - ) - - errors = process_ses_response(json.loads(ses_missing_notification_id_callback())) - assert errors == 'SES callback failed: messageId missing' - stats_mock.assert_not_called() - - def test_ses_callback_should_update_notification_status( client, notify_db, @@ -54,7 +36,7 @@ def test_ses_callback_should_update_notification_status( assert get_notification_by_id(notification.id).status == 'sending' - errors = process_ses_response(json.loads(ses_notification_callback())) + errors = process_ses_response(ses_notification_callback(reference='ref')) assert errors is None assert get_notification_by_id(notification.id).status == 'delivered' statsd_client.timing_with_dates.assert_any_call( @@ -99,9 +81,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent( sent_at=datetime.utcnow(), status='sending') - assert process_ses_response(json.loads(ses_notification_callback(ref='ref1'))) is None - assert process_ses_response(json.loads(ses_notification_callback(ref='ref2'))) is None - assert process_ses_response(json.loads(ses_notification_callback(ref='ref3'))) is None + assert process_ses_response(ses_notification_callback(reference='ref1')) is None + assert process_ses_response(ses_notification_callback(reference='ref2')) is None + assert process_ses_response(ses_notification_callback(reference='ref3')) is None stats_mock.assert_has_calls([ call(notification1), @@ -129,7 +111,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, sent_at=datetime.utcnow() ) assert get_notification_by_id(notification.id).status == 'sending' - assert process_ses_response(json.loads(ses_soft_bounce_callback())) is None + assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'temporary-failure' stats_mock.assert_called_once_with(notification) @@ -175,57 +157,15 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, ) assert get_notification_by_id(notification.id).status == 'sending' - assert process_ses_response(json.loads(ses_hard_bounce_callback())) is None + assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'permanent-failure' stats_mock.assert_called_once_with(notification) def test_remove_emails_from_bounce(): # an actual bouncedict example - message_dict = { - 'feedbackId': '0102015fc9acfc14-12341234-1234-1234-1234-123412341234-000000', - 'bounceType': 'Permanent', - 'reportingMTA': 'dsn; a1-23.smtp-out.eu-west-4.amazonses.com', - 'remoteMtaIp': '123.123.123.123', - 'bounceSubType': 'General', - 'bouncedRecipients': [{ - 'diagnosticCode': "smtp; 550-5.1.1 The email account that you tried to reach does not exist. Please try\n550-5.1.1 double-checking the recipient's email address for typos or\n550-5.1.1 unnecessary spaces. Learn more at\n550 5.1.1 https://support.google.com/mail/?p=NoSuchUser x33si3064453edx.66 - gsmtp", # noqa - 'action': 'failed', - 'status': '5.1.1', - 'emailAddress': 'not-real@gmail.com'}], - 'timestamp': '2017-11-17T11:11:18.098Z' - } + message_dict = json.loads(ses_hard_bounce_callback(reference='ref')['Message']) remove_emails_from_bounce(message_dict['bounce']) assert 'not-real@gmail.com' not in json.dumps(message_dict) - - -def ses_notification_callback(ref='ref'): - return str.encode( - '{\n "Type" : "Notification",\n "MessageId" : "%(ref)s",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"%(ref)s\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' % {'ref': ref} # noqa - ) - - -def ses_invalid_notification_id_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "missing",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"missing\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_missing_notification_id_callback(): - return b'{\n "Type" : "Notification",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_invalid_notification_type_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Unknown\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_hard_bounce_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Permanent\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"ref\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_soft_bounce_callback(): - return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Undetermined\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"ref\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa - - -def ses_confirmation_callback(): - return b'{\n "Type": "SubscriptionConfirmation",\n "MessageId": "165545c9-2a5c-472c-8df2-7ff2be2b3b1b",\n "Token": "2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n "TopicArn": "arn:aws:sns:us-west-2:123456789012:MyTopic",\n "Message": "You have chosen to subscribe to the topic arn:aws:sns:us-west-2:123456789012:MyTopic.\\nTo confirm the subscription, visit the SubscribeURL included in this message.",\n "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=arn:aws:sns:us-west-2:123456789012:MyTopic&Token=2336412f37fb687f5d51e6e241d09c805a5a57b30d712f794cc5f6a988666d92768dd60a747ba6f3beb71854e285d6ad02428b09ceece29417f1f02d609c582afbacc99c583a916b9981dd2728f4ae6fdb82efd087cc3b7849e05798d2d2785c03b0879594eeac82c01f235d0e717736",\n "Timestamp": "2012-04-26T20:45:04.751Z",\n "SignatureVersion": "1",\n "Signature": "EXAMPLEpH+DcEwjAPg8O9mY8dReBSwksfg2S7WKQcikcNKWLQjwu6A4VbeS0QHVCkhRS7fUQvi2egU3N858fiTDN6bkkOxYDVrY0Ad8L10Hs3zH81mtnPk5uvvolIC1CXGu43obcgFxeL3khZl8IKvO61GWB6jI9b5+gLPoBc1Q=",\n "SigningCertURL": "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-f3ecfb7224c7233fe7bb5f59f96de52f.pem"\n}' # noqa From 12a99f9a7fa7d7287811e1f178d270d0b45b3eba Mon Sep 17 00:00:00 2001 From: venusbb Date: Fri, 17 Nov 2017 14:36:28 +0000 Subject: [PATCH 7/8] Verify authorisation header being send --- app/notifications/receive_notifications.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index fa9d11146..bc46a8ed1 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -56,6 +56,10 @@ def receive_mmg_sms(): def receive_firetext_sms(): post_data = request.form + # This is a pre-implementation test code to validate the provider is sending through what they claim. + auth = request.authorization + current_app.logger.info("Inbound sms username: {}".format(auth.username)) + inbound_number = strip_leading_forty_four(post_data['destination']) service = fetch_potential_service(inbound_number, 'firetext') From d70fdbb5c290c205fd0a6370f7a4842d6b572cac Mon Sep 17 00:00:00 2001 From: venusbb Date: Fri, 17 Nov 2017 15:53:00 +0000 Subject: [PATCH 8/8] no auth check --- app/notifications/receive_notifications.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index bc46a8ed1..4b6e2ebb8 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -56,9 +56,12 @@ def receive_mmg_sms(): def receive_firetext_sms(): post_data = request.form - # This is a pre-implementation test code to validate the provider is sending through what they claim. + # This is pre-implementation test code to validate the provider is basic auth headers. auth = request.authorization - current_app.logger.info("Inbound sms username: {}".format(auth.username)) + if auth: + current_app.logger.info("Inbound sms username: {}".format(auth.username)) + else: + current_app.logger.info("Inbound sms no auth header") inbound_number = strip_leading_forty_four(post_data['destination'])