Don’t respect the whitelist for one off sending

The whitelist was built to help developers and designers making
prototypes to do realistic usability testing of them, without having to
go through the whole go live process.

These users are sending messages using the API. The whitelist wasn’t
made available to users uploading spreadsheets. The users sending one
off messages are similar to those uploading spreadsheets, not those
using the API. Therefore they shouldn’t be able to use the whitelist to
expand the range of recipients they can send to.

Passing the argument through three methods doesn’t feel that great, but
can’t think of a better way without major refactoring…

app/notifications/validators.py

@@ -67,8 +67,8 @@ def check_template_is_active(template):
                               message="Template has been deleted")
 
 
-def service_can_send_to_recipient(send_to, key_type, service):
+def service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_recipients=True):
-    if not service_allowed_to_send_to(send_to, service, key_type):
+    if not service_allowed_to_send_to(send_to, service, key_type, allow_whitelisted_recipients):
         if key_type == KEY_TYPE_TEAM:
             message = 'Can’t send to this recipient using a team-only API key'
         else:
@@ -97,11 +97,11 @@ def check_service_can_schedule_notification(permissions, scheduled_for):
             raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)")
 
 
-def validate_and_format_recipient(send_to, key_type, service, notification_type):
+def validate_and_format_recipient(send_to, key_type, service, notification_type, allow_whitelisted_recipients=True):
     if send_to is None:
         raise BadRequestError(message="Recipient can't be empty")
 
-    service_can_send_to_recipient(send_to, key_type, service)
+    service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_recipients)
 
     if notification_type == SMS_TYPE:
         international_phone_info = get_international_phone_info(send_to)

app/service/send_notification.py

@@ -49,7 +49,8 @@ def send_one_off_notification(service_id, post_data):
         send_to=post_data['to'],
         key_type=KEY_TYPE_NORMAL,
         service=service,
-        notification_type=template.template_type
+        notification_type=template.template_type,
+        allow_whitelisted_recipients=False,
     )
 
     validate_created_by(service, post_data['created_by'])

app/service/utils.py

@@ -26,7 +26,7 @@ def get_whitelist_objects(service_id, request_json):
     ]
 
 
-def service_allowed_to_send_to(recipient, service, key_type):
+def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_recipients=True):
     if key_type == KEY_TYPE_TEST:
         return True
 
@@ -38,6 +38,7 @@ def service_allowed_to_send_to(recipient, service, key_type):
     )
     whitelist_members = [
         member.recipient for member in service.whitelist
+        if allow_whitelisted_recipients
     ]
 
     if (

tests/app/notifications/test_validators.py

@@ -213,6 +213,29 @@ def test_service_can_send_to_recipient_passes_for_whitelisted_recipient_passes(n
                                          sample_service) is None
 
 
+@pytest.mark.parametrize('recipient', [
+    {"email_address": "some_other_email@test.com"},
+    {"mobile_number": "07513332413"},
+])
+def test_service_can_send_to_recipient_fails_when_ignoring_whitelist(
+    notify_db,
+    notify_db_session,
+    sample_service,
+    recipient,
+):
+    sample_service_whitelist(notify_db, notify_db_session, **recipient)
+    with pytest.raises(BadRequestError) as exec_info:
+        service_can_send_to_recipient(
+            next(iter(recipient.values())),
+            'team',
+            sample_service,
+            allow_whitelisted_recipients=False,
+        )
+    assert exec_info.value.status_code == 400
+    assert exec_info.value.message == 'Can’t send to this recipient using a team-only API key'
+    assert exec_info.value.fields == []
+
+
 @pytest.mark.parametrize('recipient', ['07513332413', 'some_other_email@test.com'])
 @pytest.mark.parametrize('key_type, error_message',
                          [('team', 'Can’t send to this recipient using a team-only API key'),

tests/app/service/test_send_one_off_notification.py

@@ -7,12 +7,15 @@ from sqlalchemy.exc import SQLAlchemyError
 
 from app.v2.errors import BadRequestError, TooManyRequestsError
 from app.config import QueueNames
+from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts
 from app.service.send_notification import send_one_off_notification
 from app.models import (
     KEY_TYPE_NORMAL,
+    MOBILE_TYPE,
     PRIORITY,
     SMS_TYPE,
-    Notification
+    Notification,
+    ServiceWhitelist,
 )
 
 from tests.app.db import (
@@ -137,13 +140,24 @@ def test_send_one_off_notification_raises_if_invalid_recipient(notify_db_session
         send_one_off_notification(service.id, post_data)
 
 
-def test_send_one_off_notification_raises_if_cant_send_to_recipient(notify_db_session):
+@pytest.mark.parametrize('recipient', [
+    '07700 900 001',  # not in team or whitelist
+    '07700900123',  # in whitelist
+    '+447700-900-123',  # in whitelist in different format
+])
+def test_send_one_off_notification_raises_if_cant_send_to_recipient(
+    notify_db_session,
+    recipient,
+):
     service = create_service(restricted=True)
     template = create_template(service=service)
+    dao_add_and_commit_whitelisted_contacts([
+        ServiceWhitelist.from_string(service.id, MOBILE_TYPE, '07700900123'),
+    ])
 
     post_data = {
         'template_id': str(template.id),
-        'to': '07700 900 001',
+        'to': recipient,
         'created_by': str(service.created_by_id)
     }