From 04b003c152308465f3f1a02120a5a7b024371f32 Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Tue, 25 Apr 2017 14:38:38 +0100 Subject: [PATCH 1/5] Add utility function for subscription autoconfirm --- app/notifications/notifications_ses_callback.py | 3 +++ app/notifications/utils.py | 8 ++++++++ 2 files changed, 11 insertions(+) create mode 100644 app/notifications/utils.py diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 3980f5add..2f4ba4bf3 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -15,6 +15,7 @@ from app.dao import ( ) from app.notifications.process_client_response import validate_callback_data +from app.notifications.utils import confirm_subscription ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__) @@ -33,6 +34,8 @@ def process_ses_response(): if 'Type' in ses_request and ses_request['Type'] == 'SubscriptionConfirmation': current_app.logger.info("SNS subscription confirmation url: {}".format(ses_request['SubscribeURL'])) + subscribed_topic = confirm_subscription(ses_request) + current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic)) errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name) if errors: diff --git a/app/notifications/utils.py b/app/notifications/utils.py new file mode 100644 index 000000000..ca9bf131c --- /dev/null +++ b/app/notifications/utils.py @@ -0,0 +1,8 @@ +import requests + + +def confirm_subscription(confirmation_request): + url = confirmation_request['SubscribeURL'] + response = requests.get(url) + if response.code < 400: + return confirmation_request['TopicArn'] From 74433c93353a4100d39b4563425eef18ade1dc1a Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Tue, 25 Apr 2017 17:01:38 +0100 Subject: [PATCH 2/5] Add tests for autoconfirmation --- .../notifications_ses_callback.py | 3 ++ app/notifications/utils.py | 4 +- .../test_notifications_ses_callback.py | 47 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 2f4ba4bf3..a32f9a01d 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -36,6 +36,9 @@ def process_ses_response(): current_app.logger.info("SNS subscription confirmation url: {}".format(ses_request['SubscribeURL'])) subscribed_topic = confirm_subscription(ses_request) current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic)) + return jsonify( + result="success", message="SES callback succeeded" + ), 200 errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name) if errors: diff --git a/app/notifications/utils.py b/app/notifications/utils.py index ca9bf131c..71bcb3429 100644 --- a/app/notifications/utils.py +++ b/app/notifications/utils.py @@ -4,5 +4,5 @@ import requests def confirm_subscription(confirmation_request): url = confirmation_request['SubscribeURL'] response = requests.get(url) - if response.code < 400: - return confirmation_request['TopicArn'] + response.raise_for_status() + return confirmation_request['TopicArn'] diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 10b3e1000..0190b8e2c 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -2,6 +2,8 @@ from datetime import datetime from flask import json from freezegun import freeze_time +from requests import HTTPError +import pytest from app import statsd_client from app.dao.notifications_dao import get_notification_by_id @@ -29,6 +31,48 @@ def test_ses_callback_should_fail_if_invalid_json(client): assert json_resp['message'] == 'SES callback failed: invalid json' +def test_ses_callback_should_auto_confirm_subscriptions(client, rmock): + endpoint = json.loads(ses_confirmation_callback())['SubscribeURL'] + rmock.request( + "GET", + endpoint, + json={"status": "success"}, + status_code=200) + + response = client.post( + path='/notifications/email/ses', + data=ses_confirmation_callback(), + headers=[('Content-Type', 'text/plain; charset=UTF-8')] + ) + json_resp = json.loads(response.get_data(as_text=True)) + + assert rmock.called + assert rmock.request_history[0].url == endpoint + assert response.status_code == 200 + assert json_resp['result'] == 'success' + assert json_resp['message'] == 'SES callback succeeded' + + +def test_ses_callback_autoconfirmat_raises_exception_if_not_200(client, rmock): + endpoint = json.loads(ses_confirmation_callback())['SubscribeURL'] + rmock.request( + "GET", + endpoint, + json={"status": "not allowed"}, + status_code=405) + + with pytest.raises(HTTPError) as exc: + client.post( + path='/notifications/email/ses', + data=ses_confirmation_callback(), + headers=[('Content-Type', 'text/plain; charset=UTF-8')] + ) + + assert rmock.called + assert rmock.request_history[0].url == endpoint + assert exc.value.response.status_code == 405 + + def test_ses_callback_should_fail_if_invalid_notification_type(client): response = client.post( path='/notifications/email/ses', @@ -256,3 +300,6 @@ def ses_hard_bounce_callback(): 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 336462cdb292ebf46ebdfa4bb56f0e8d873ca186 Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Wed, 26 Apr 2017 10:37:50 +0100 Subject: [PATCH 3/5] Fix whitespaces --- tests/app/notifications/test_notifications_ses_callback.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 0190b8e2c..ba6eaac9d 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -301,5 +301,6 @@ def ses_hard_bounce_callback(): 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 + 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 cfed90e5025b56377d66b4b5cbdf0a11f3e4116c Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Fri, 28 Apr 2017 16:10:41 +0100 Subject: [PATCH 4/5] Make code a bit more defensive and add som logging --- app/notifications/notifications_ses_callback.py | 11 ++++++----- app/notifications/utils.py | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index a32f9a01d..ce8dec9ba 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -32,13 +32,14 @@ def process_ses_response(): try: ses_request = json.loads(request.data) - if 'Type' in ses_request and ses_request['Type'] == 'SubscriptionConfirmation': + if ses_request.get('Type') == 'SubscriptionConfirmation': current_app.logger.info("SNS subscription confirmation url: {}".format(ses_request['SubscribeURL'])) subscribed_topic = confirm_subscription(ses_request) - current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic)) - return jsonify( - result="success", message="SES callback succeeded" - ), 200 + if subscribed_topic: + current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic)) + return jsonify( + result="success", message="SES callback succeeded" + ), 200 errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name) if errors: diff --git a/app/notifications/utils.py b/app/notifications/utils.py index 71bcb3429..80346c475 100644 --- a/app/notifications/utils.py +++ b/app/notifications/utils.py @@ -1,8 +1,18 @@ +from flask import current_app import requests def confirm_subscription(confirmation_request): - url = confirmation_request['SubscribeURL'] + url = confirmation_request.get('SubscribeURL') + if not url: + current_app.logger.warning("SubscribeURL does not exist or empty") + return + response = requests.get(url) - response.raise_for_status() + try: + response.raise_for_status() + except Exception as e: + current_app.logger.warning("Response: {}".format(response.text)) + raise e + return confirmation_request['TopicArn'] From c99b65252fe0113edbd36c85554edc0747682b51 Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Fri, 28 Apr 2017 16:11:27 +0100 Subject: [PATCH 5/5] Fix some typos --- tests/app/notifications/test_notifications_ses_callback.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index ba6eaac9d..051050b05 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -31,7 +31,7 @@ def test_ses_callback_should_fail_if_invalid_json(client): assert json_resp['message'] == 'SES callback failed: invalid json' -def test_ses_callback_should_auto_confirm_subscriptions(client, rmock): +def test_ses_callback_should_autoconfirm_subscriptions(client, rmock): endpoint = json.loads(ses_confirmation_callback())['SubscribeURL'] rmock.request( "GET", @@ -53,7 +53,7 @@ def test_ses_callback_should_auto_confirm_subscriptions(client, rmock): assert json_resp['message'] == 'SES callback succeeded' -def test_ses_callback_autoconfirmat_raises_exception_if_not_200(client, rmock): +def test_ses_callback_autoconfirm_raises_exception_if_not_200(client, rmock): endpoint = json.loads(ses_confirmation_callback())['SubscribeURL'] rmock.request( "GET",