Send Notify's 2FA codes via only the notify queue

This means that these codes won't be delayed by large jobs going through the
send-sms/email queues. send_user_sms_code now works much more like the
endpoints for sending notifications, by persisting the notification and only
using the deliver_sms task (instead of using send_sms as well).

The workers consuming the `notify` queue should be able to handle the deliver
task as well, so no change should be needed to the celery workers to support
this.

I think there's also a change in behaviour here: previously, if the Notify
service was in research mode, 2FA codes would not have been sent out, making
it impossible to log into the admin. Now, a call to this endpoint will always
send out the notification even if we've put the Notify service into research
mode, since we set the notification's key type to normal and ignore the
service's research mode setting when sending the notification to the queue.

app/user/rest.py

@@ -18,7 +18,11 @@ from app.dao.users_dao import (
 from app.dao.permissions_dao import permission_dao
 from app.dao.services_dao import dao_fetch_service_by_id
 from app.dao.templates_dao import dao_get_template_by_id
-from app.models import SMS_TYPE
+from app.models import SMS_TYPE, KEY_TYPE_NORMAL
+from app.notifications.process_notifications import (
+    persist_notification,
+    send_notification_to_queue
+)
 from app.schemas import (
     email_data_request_schema,
     user_schema,
@@ -143,20 +147,22 @@ def send_user_sms_code(user_id):
     mobile = user_to_send_to.mobile_number if verify_code.get('to', None) is None else verify_code.get('to')
     sms_code_template_id = current_app.config['SMS_CODE_TEMPLATE_ID']
     sms_code_template = dao_get_template_by_id(sms_code_template_id)
-    verification_message = encryption.encrypt({
-        'template': sms_code_template_id,
-        'template_version': sms_code_template.version,
-        'to': mobile,
-        'personalisation': {
-            'verify_code': secret_code
-        }
+    notify_service_id = current_app.config['NOTIFY_SERVICE_ID']
 
-    })
-    send_sms.apply_async([current_app.config['NOTIFY_SERVICE_ID'],
-                          str(uuid.uuid4()),
-                          verification_message,
-                          datetime.utcnow().strftime(DATETIME_FORMAT)
-                          ], queue='notify')
+    saved_notification = persist_notification(
+        template_id=sms_code_template_id,
+        template_version=sms_code_template.version,
+        recipient=mobile,
+        service_id=notify_service_id,
+        personalisation={'verify_code': secret_code},
+        notification_type=SMS_TYPE,
+        api_key_id=None,
+        key_type=KEY_TYPE_NORMAL
+    )
+    # Assume that we never want to observe the Notify service's research mode
+    # setting for this notification - we still need to be able to log into the
+    # admin even if we're doing user research using this service:
+    send_notification_to_queue(saved_notification, False, queue='notify')
 
     return jsonify({}), 204
 

tests/app/user/test_rest_verify.py

@@ -1,5 +1,6 @@
 import json
 import moto
+import pytest
 
 from datetime import (
     datetime,
@@ -7,9 +8,11 @@ from datetime import (
 )
 
 from flask import url_for, current_app
+from app.dao.services_dao import dao_update_service, dao_fetch_service_by_id
 from app.models import (
     VerifyCode,
-    User
+    User,
+    Notification
 )
 
 from app import db, encryption
@@ -214,41 +217,47 @@ def test_user_verify_password_missing_password(notify_api,
             assert 'Required field missing data' in json_resp['message']['password']
 
 
+@pytest.mark.parametrize('research_mode', [True, False])
 @freeze_time("2016-01-01 11:09:00.061258")
 def test_send_user_sms_code(notify_api,
                             sample_user,
                             sms_code_template,
-                            mocker):
+                            mocker,
+                            research_mode):
     """
     Tests POST endpoint /user/<user_id>/sms-code
     """
 
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
+            if research_mode:
+                notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID'])
+                notify_service.research_mode = True
+                dao_update_service(notify_service)
+
             data = json.dumps({})
             auth_header = create_authorization_header()
             mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111')
-            mocker.patch('app.celery.tasks.send_sms.apply_async')
-            mocker.patch('uuid.uuid4', return_value='some_uuid')  # for the notification id
+            mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+
             resp = client.post(
                 url_for('user.send_user_sms_code', user_id=sample_user.id),
                 data=data,
                 headers=[('Content-Type', 'application/json'), auth_header])
             assert resp.status_code == 204
+
             assert mocked.call_count == 1
-            encrypted = encryption.encrypt({
-                'template': current_app.config['SMS_CODE_TEMPLATE_ID'],
-                'template_version': 1,
-                'to': sample_user.mobile_number,
-                'personalisation': {
-                    'verify_code': '11111'
-                }
-            })
-            app.celery.tasks.send_sms.apply_async.assert_called_once_with(
-                ([current_app.config['NOTIFY_SERVICE_ID'],
-                  "some_uuid",
-                  encrypted,
-                  "2016-01-01T11:09:00.061258Z"]),
+            assert VerifyCode.query.count() == 1
+            assert VerifyCode.query.first().check_code('11111')
+
+            assert Notification.query.count() == 1
+            notification = Notification.query.first()
+            assert notification.personalisation == {'verify_code': '11111'}
+            assert notification.to == sample_user.mobile_number
+            assert str(notification.service_id) == current_app.config['NOTIFY_SERVICE_ID']
+
+            app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
+                ([str(notification.id)]),
                 queue="notify"
             )
 
@@ -257,38 +266,29 @@ def test_send_user_sms_code(notify_api,
 def test_send_user_code_for_sms_with_optional_to_field(notify_api,
                                                        sample_user,
                                                        sms_code_template,
-                                                       mock_encryption,
                                                        mocker):
     """
-    Tests POST endpoint '/<user_id>/code' successful sms with optional to field
+    Tests POST endpoint /user/<user_id>/sms-code with optional to field
     """
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
+            to_number = '+441119876757'
             mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111')
-            mocker.patch('uuid.uuid4', return_value='some_uuid')  # for the notification id
-            mocker.patch('app.celery.tasks.send_sms.apply_async')
-            data = json.dumps({'to': '+441119876757'})
+            mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            data = json.dumps({'to': to_number})
             auth_header = create_authorization_header()
+
             resp = client.post(
                 url_for('user.send_user_sms_code', user_id=sample_user.id),
                 data=data,
                 headers=[('Content-Type', 'application/json'), auth_header])
 
             assert resp.status_code == 204
-            encrypted = encryption.encrypt({
-                'template': current_app.config['SMS_CODE_TEMPLATE_ID'],
-                'template_version': 1,
-                'to': '+441119876757',
-                'personalisation': {
-                    'verify_code': '11111'
-                }
-            })
             assert mocked.call_count == 1
-            app.celery.tasks.send_sms.apply_async.assert_called_once_with(
-                ([current_app.config['NOTIFY_SERVICE_ID'],
-                  "some_uuid",
-                  encrypted,
-                  "2016-01-01T11:09:00.061258Z"]),
+            notification = Notification.query.first()
+            assert notification.to == to_number
+            app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
+                ([str(notification.id)]),
                 queue="notify"
             )