Merge branch 'master' of https://github.com/alphagov/notifications-api into vb-free-sms-limit-history

app/celery/tasks.py

@@ -63,6 +63,7 @@ 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
@@ -547,3 +548,12 @@ 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):
+    errors = process_ses_response(response)
+    if errors:
+        current_app.logger.error(errors)
+        self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error")

app/notifications/notifications_ses_callback.py

@@ -15,7 +15,6 @@ from app.dao import (
 )
 from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks
 from app.notifications.process_client_response import validate_callback_data
-from app.notifications.utils import autoconfirm_subscription
 
 ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__)
 
@@ -27,26 +26,33 @@ register_errors(ses_callback_blueprint)
 
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
-def process_ses_response():
+def sns_callback_handler():
+    errors = process_ses_response(json.loads(request.data))
+    if errors:
+        current_app.logger.error(errors)
+        raise InvalidRequest(errors, 400)
+
+    return jsonify(
+        result="success", message="SES callback succeeded"
+    ), 200
+
+
+def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        ses_request = json.loads(request.data)
 
-        subscribed_topic = autoconfirm_subscription(ses_request)
+        # TODO: remove this check once the sns_callback_handler is removed
-        if subscribed_topic:
+        if not isinstance(ses_request, dict):
-            current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
+            ses_request = json.loads(ses_request)
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -58,7 +64,7 @@ def process_ses_response():
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            raise InvalidRequest(error, status_code=400)
+            return error
 
         notification_status = aws_response_dict['notification_status']
 
@@ -71,7 +77,7 @@ def process_ses_response():
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                raise InvalidRequest(error, status_code=404)
+                return error
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -96,14 +102,12 @@ def process_ses_response():
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return jsonify(
+            return
-                result="success", message="SES callback succeeded"
-            ), 200
 
         except KeyError:
-            message = "SES callback failed: messageId missing"
+            error = "SES callback failed: messageId missing"
-            raise InvalidRequest(message, status_code=400)
+            return error
 
-    except ValueError as ex:
+    except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        raise InvalidRequest(error, status_code=400)
+        return error

tests/app/celery/test_tasks.py

@@ -1,3 +1,4 @@
+import codecs
 import json
 import uuid
 from datetime import datetime, timedelta
@@ -26,8 +27,8 @@ from app.celery.tasks import (
     process_incomplete_jobs,
     get_template_class,
     s3,
-    send_inbound_sms_to_service
+    send_inbound_sms_to_service,
-)
+    process_ses_results)
 from app.config import QueueNames
 from app.dao import jobs_dao, services_dao
 from app.models import (
@@ -1381,3 +1382,50 @@ 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_retry_called(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 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}'

tests/app/notifications/test_notifications_ses_callback.py

@@ -3,11 +3,10 @@ from unittest.mock import call
 
 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
+from app.notifications.notifications_ses_callback import process_ses_response
 from tests.app.conftest import sample_notification as create_sample_notification
 
 
@@ -17,75 +16,15 @@ def test_ses_callback_should_not_need_auth(client):
         data=ses_notification_callback(),
         headers=[('Content-Type', 'text/plain; charset=UTF-8')]
     )
-    assert response.status_code == 404
+    assert response.status_code == 400
 
 
 def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
+    errors = process_ses_response('nonsense')
-    response = client.post(
+    assert errors == 'SES callback failed: invalid json'
-        path='/notifications/email/ses',
-        data="nonsense",
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: invalid json'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_should_autoconfirm_subscriptions(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    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'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_autoconfirm_raises_exception_if_not_200(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    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
     stats_mock.assert_not_called()
 
 
@@ -94,15 +33,8 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
+    errors = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
-        path='/notifications/email/ses',
+    assert errors == 'SES callback failed: status Unknown not found'
-        data=ses_invalid_notification_type_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
 
@@ -111,15 +43,8 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
+    errors = process_ses_response(json.loads(ses_missing_notification_id_callback()))
-        path='/notifications/email/ses',
+    assert errors == 'SES callback failed: messageId missing'
-        data=ses_missing_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
 
@@ -128,15 +53,8 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
+    errors = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
-        path='/notifications/email/ses',
+    assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
-        data=ses_invalid_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
 
@@ -164,15 +82,8 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        response = client.post(
+        errors = process_ses_response(json.loads(ses_notification_callback()))
-            path='/notifications/email/ses',
+        assert errors is None
-            data=ses_notification_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-        json_resp = json.loads(response.get_data(as_text=True))
-        assert response.status_code == 200
-        assert json_resp['result'] == 'success'
-        assert json_resp['message'] == 'SES callback succeeded'
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -216,25 +127,10 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = client.post(
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref1'))) is None
-        path='/notifications/email/ses',
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref2'))) is None
-        data=ses_notification_callback(ref='ref1'),
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref3'))) is None
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp2 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref2'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp3 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref3'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
 
-    assert resp1.status_code == 200
-    assert resp2.status_code == 200
-    assert resp3.status_code == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -261,16 +157,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
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -294,16 +181,8 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
     )
 
     assert get_notification_by_id(notification.id).status == 'delivered'
-
+    error = process_ses_response(json.loads(ses_soft_bounce_callback()))
-    response = client.post(
+    assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
 
@@ -327,16 +206,7 @@ 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
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_hard_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)