Merge pull request #2878 from alphagov/uk-prefix

Make sure people without international sms permission can send to crown dependencies

app/notifications/rest.py

@@ -15,7 +15,7 @@ from app.errors import (
     InvalidRequest
 )
 from app.models import (
-    EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE,
+    EMAIL_TYPE, SMS_TYPE,
     KEY_TYPE_TEAM, PRIORITY,
     LETTER_TYPE)
 from app.notifications.process_notifications import (
@@ -24,9 +24,10 @@ from app.notifications.process_notifications import (
     simulated_recipient
 )
 from app.notifications.validators import (
+    check_if_service_can_send_to_number,
     check_rate_limiting,
     service_has_permission,
-    validate_template
+    validate_template,
 )
 from app.schemas import (
     email_notification_schema,
@@ -38,7 +39,6 @@ from app.service.utils import service_allowed_to_send_to
 from app.utils import pagination_links, get_public_notify_type_text
 
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
-from notifications_utils.recipients import get_international_phone_info
 
 notifications = Blueprint('notifications', __name__)
 
@@ -115,7 +115,7 @@ def send_notification(notification_type):
         )
 
     if notification_type == SMS_TYPE:
-        _service_can_send_internationally(authenticated_service, notification_form['to'])
+        check_if_service_can_send_to_number(authenticated_service, notification_form['to'])
     # Do not persist or send notification to the queue if it is a simulated recipient
     simulated = simulated_recipient(notification_form['to'], notification_type)
     notification_model = persist_notification(template_id=template.id,
@@ -160,17 +160,6 @@ def get_notification_return_data(notification_id, notification, template):
     return output
 
 
-def _service_can_send_internationally(service, number):
-    international_phone_info = get_international_phone_info(number)
-
-    if international_phone_info.international and \
-            INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]:
-        raise InvalidRequest(
-            {'to': ["Cannot send to international mobile numbers"]},
-            status_code=400
-        )
-
-
 def _service_allowed_to_send_to(notification, service):
     if not service_allowed_to_send_to(notification['to'], service, api_user.key_type):
         if api_user.key_type == KEY_TYPE_TEAM:

app/notifications/validators.py

@@ -120,11 +120,7 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type,
     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)
+        international_phone_info = check_if_service_can_send_to_number(service, send_to)
-
-        if international_phone_info.international and \
-                INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]:
-            raise BadRequestError(message="Cannot send to international mobile numbers")
 
         return validate_and_format_phone_number(
             number=send_to,
@@ -134,6 +130,18 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type,
         return validate_and_format_email_address(email_address=send_to)
 
 
+def check_if_service_can_send_to_number(service, number):
+    international_phone_info = get_international_phone_info(number)
+
+    if (
+        # if number is international and not a crown dependency
+        international_phone_info.international and international_phone_info.country_prefix != '44'
+    ) and INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]:
+        raise BadRequestError(message="Cannot send to international mobile numbers")
+    else:
+        return international_phone_info
+
+
 def check_content_char_count(template_with_content):
     if template_with_content.is_message_too_long():
         message = f"Text messages cannot be longer than {SMS_CHAR_COUNT_LIMIT} characters. " \

requirements-app.txt

@@ -26,7 +26,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@39.6.0#egg=notifications-utils==39.6.0
+git+https://github.com/alphagov/notifications-utils.git@39.7.0#egg=notifications-utils==39.7.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1

requirements.txt

@@ -28,7 +28,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@39.6.0#egg=notifications-utils==39.6.0
+git+https://github.com/alphagov/notifications-utils.git@39.7.0#egg=notifications-utils==39.7.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1
@@ -39,14 +39,14 @@ alembic==1.4.2
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.82
+awscli==1.18.83
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.4
 blinker==1.4
 boto==2.49.0
 boto3==1.10.38
-botocore==1.17.5
+botocore==1.17.6
 cachetools==4.1.0
 certifi==2020.4.5.2
 chardet==3.0.4

tests/app/notifications/rest/test_send_notification.py

@@ -1051,7 +1051,9 @@ def test_should_allow_store_original_number_on_sms_notification(client, sample_t
     assert '+(44) 7700-900 855' == notifications[0].to
 
 
-def test_should_not_allow_international_number_on_sms_notification(client, sample_template, mocker):
+def test_should_not_allow_sending_to_international_number_without_international_permission(
+    client, sample_template, mocker
+):
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     data = {
@@ -1070,10 +1072,35 @@ def test_should_not_allow_international_number_on_sms_notification(client, sampl
     assert response.status_code == 400
     error_json = json.loads(response.get_data(as_text=True))
     assert error_json['result'] == 'error'
-    assert error_json['message']['to'][0] == 'Cannot send to international mobile numbers'
+    assert error_json['message'] == 'Cannot send to international mobile numbers'
 
 
-def test_should_allow_international_number_on_sms_notification(client, sample_service_full_permissions, mocker):
+def test_should_allow_sending_to_crown_dependency_number_without_international_permission(
+    client, mocker, notify_db_session
+):
+    service = create_service()
+
+    mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+    template = create_template(service)
+
+    data = {
+        'to': '07700-900-123',
+        'template': str(template.id)
+    }
+
+    auth_header = create_authorization_header(service_id=service.id)
+
+    response = client.post(
+        path='/notifications/sms',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 201
+
+
+def test_should_allow_sending_to_international_number_with_international_permission(
+    client, sample_service_full_permissions, mocker
+):
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
     template = create_template(sample_service_full_permissions)
 

tests/app/notifications/test_process_notification.py

@@ -356,7 +356,7 @@ def test_simulated_recipient(notify_api, to_address, notification_type, expected
 @pytest.mark.parametrize('recipient, expected_international, expected_prefix, expected_units', [
     ('7900900123', False, '44', 1),  # UK
     ('+447900900123', False, '44', 1),  # UK
-    ('07700900222', False, '44', 1),  # UK
+    ('07700900222', True, '44', 1),  # UK (Jersey)
     ('73122345678', True, '7', 1),  # Russia
     ('360623400400', True, '36', 3)]  # Hungary
 )