Validate International phone numbers

- uses new utils methods to validate phone numbers
- defaults to International=True on validation. This ensures the validator works on all numbers
- Then check if the user can send this message to the number internationally if needed.

app/notifications/process_notifications.py

@@ -4,6 +4,7 @@ from flask import current_app
 
 from app import redis_store
 from app.celery import provider_tasks
+from notifications_utils.recipients import validate_and_format_phone_number
 from notifications_utils.clients import redis
 from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
 from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
@@ -98,6 +99,10 @@ def send_notification_to_queue(notification, research_mode, queue=None):
 
 
 def simulated_recipient(to_address, notification_type):
-    return (to_address in current_app.config['SIMULATED_SMS_NUMBERS']
+    if notification_type == SMS_TYPE:
-            if notification_type == SMS_TYPE
+        formatted_simulated_numbers = [
-            else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'])
+            validate_and_format_phone_number(number) for number in current_app.config['SIMULATED_SMS_NUMBERS']
+        ]
+        return to_address in formatted_simulated_numbers
+    else:
+        return to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES']

app/notifications/rest.py

@@ -2,8 +2,7 @@ from flask import (
     Blueprint,
     jsonify,
     request,
-    current_app,
+    current_app
-    json
 )
 
 from app import api_user
@@ -14,10 +13,6 @@ from app.dao import (
 )
 from app.models import KEY_TYPE_TEAM, PRIORITY
 from app.models import SMS_TYPE
-from app.notifications.process_client_response import (
-    validate_callback_data,
-    process_sms_client_response
-)
 from app.notifications.process_notifications import (persist_notification,
                                                      send_notification_to_queue,
                                                      simulated_recipient)
@@ -35,6 +30,8 @@ from app.schemas import (
 from app.service.utils import service_allowed_to_send_to
 from app.utils import pagination_links, get_template_instance
 
+from notifications_utils.recipients import get_international_phone_info
+
 notifications = Blueprint('notifications', __name__)
 
 from app.errors import (
@@ -104,13 +101,15 @@ def send_notification(notification_type):
     notification_form, errors = (
         sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema
     ).load(request.get_json())
+
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
     check_service_message_limit(api_user.key_type, service)
 
-    template = templates_dao.dao_get_template_by_id_and_service_id(template_id=notification_form['template'],
+    template = templates_dao.dao_get_template_by_id_and_service_id(
-                                                                   service_id=service.id)
+        template_id=notification_form['template'],
+        service_id=service.id)
 
     check_template_is_for_notification_type(notification_type, template.template_type)
     check_template_is_active(template)
@@ -118,12 +117,14 @@ def send_notification(notification_type):
     template_object = create_template_object_for_notification(template, notification_form.get('personalisation', {}))
 
     _service_allowed_to_send_to(notification_form, service)
+    if notification_type == SMS_TYPE:
+        _service_can_send_internationally(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,
                                               template_version=template.version,
-                                              recipient=notification_form['to'],
+                                              recipient=request.get_json()['to'],
                                               service=service,
                                               personalisation=notification_form.get('personalisation', None),
                                               notification_type=notification_type,
@@ -160,6 +161,16 @@ 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 not service.can_send_international_sms:
+        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

@@ -1,5 +1,9 @@
 from flask import current_app
-from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
+from notifications_utils.recipients import (
+    validate_and_format_phone_number,
+    validate_and_format_email_address,
+    get_international_phone_info
+)
 
 from app.dao import services_dao
 from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE
@@ -47,8 +51,17 @@ def service_can_send_to_recipient(send_to, key_type, service):
 
 def validate_and_format_recipient(send_to, key_type, service, notification_type):
     service_can_send_to_recipient(send_to, key_type, service)
+
     if notification_type == SMS_TYPE:
-        return validate_and_format_phone_number(number=send_to)
+        international_phone_info = get_international_phone_info(send_to)
+
+        if international_phone_info.international and not service.can_send_international_sms:
+            raise BadRequestError(message="Cannot send to international mobile numbers")
+
+        return validate_and_format_phone_number(
+            number=send_to,
+            international=international_phone_info.international
+        )
     else:
         return validate_and_format_email_address(email_address=send_to)
 

app/schema_validation/__init__.py

@@ -11,7 +11,7 @@ def validate(json_to_validate, schema):
     @format_checker.checks('phone_number', raises=InvalidPhoneError)
     def validate_schema_phone_number(instance):
         if instance is not None:
-            validate_phone_number(instance)
+            validate_phone_number(instance, international=True)
         return True
 
     @format_checker.checks('email_address', raises=InvalidEmailError)

app/schemas.py

@@ -324,13 +324,13 @@ class SmsNotificationSchema(NotificationSchema):
     @validates('to')
     def validate_to(self, value):
         try:
-            validate_phone_number(value)
+            validate_phone_number(value, international=True)
         except InvalidPhoneError as error:
             raise ValidationError('Invalid phone number: {}'.format(error))
 
     @post_load
     def format_phone_number(self, item):
-        item['to'] = validate_and_format_phone_number(item['to'])
+        item['to'] = validate_and_format_phone_number(item['to'], international=True)
         return item
 
 

app/v2/notifications/post_notifications.py

@@ -27,6 +27,7 @@ def post_notification(notification_type):
         form = validate(request.get_json(), post_email_request)
     else:
         form = validate(request.get_json(), post_sms_request)
+
     service = services_dao.dao_fetch_service_by_id(api_user.service_id)
     check_service_message_limit(api_user.key_type, service)
     form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address']
@@ -41,7 +42,7 @@ def post_notification(notification_type):
     simulated = simulated_recipient(send_to, notification_type)
     notification = persist_notification(template_id=template.id,
                                         template_version=template.version,
-                                        recipient=send_to,
+                                        recipient=form_send_to,
                                         service=service,
                                         personalisation=form.get('personalisation', None),
                                         notification_type=notification_type,

requirements.txt

@@ -29,6 +29,6 @@ notifications-python-client>=3.1,<3.2
 awscli>=1.11,<1.12
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@15.2.1#egg=notifications-utils==15.2.1
+git+https://github.com/alphagov/notifications-utils.git@16.1.0#egg=notifications-utils==16.1.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

tests/app/conftest.py

@@ -123,7 +123,8 @@ def sample_service(
     user=None,
     restricted=False,
     limit=1000,
-    email_from=None
+    email_from=None,
+    can_send_international_sms=False
 ):
     if user is None:
         user = create_user()
@@ -136,6 +137,7 @@ def sample_service(
         'email_from': email_from,
         'created_by': user,
         'letter_contact_block': 'London,\nSW1A 1AA',
+        'can_send_international_sms': can_send_international_sms
     }
     service = Service.query.filter_by(name=service_name).first()
     if not service:

tests/app/delivery/test_send_to_providers.py

@@ -4,7 +4,7 @@ from collections import namedtuple
 from unittest.mock import ANY
 
 import pytest
-from notifications_utils.recipients import validate_phone_number, format_phone_number
+from notifications_utils.recipients import validate_and_format_phone_number
 
 import app
 from app import mmg_client
@@ -69,7 +69,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
     )
 
     mmg_client.send_sms.assert_called_once_with(
-        to=format_phone_number(validate_phone_number("+447234123123")),
+        to=validate_and_format_phone_number("+447234123123"),
         content="Sample service: Hello Jo\nHere is <em>some HTML</em> & entities",
         reference=str(db_notification.id),
         sender=None
@@ -151,7 +151,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
     )
 
     mmg_client.send_sms.assert_called_once_with(
-        to=format_phone_number(validate_phone_number("+447234123123")),
+        to=validate_and_format_phone_number("+447234123123"),
         content="Sample service: This is a template:\nwith a newline",
         reference=str(db_notification.id),
         sender=None
@@ -254,7 +254,7 @@ def test_should_send_sms_sender_from_service_if_present(
     )
 
     mmg_client.send_sms.assert_called_once_with(
-        to=format_phone_number(validate_phone_number("+447234123123")),
+        to=validate_and_format_phone_number("+447234123123"),
         content="This is a template:\nwith a newline",
         reference=str(db_notification.id),
         sender=sample_service.sms_sender

tests/app/notifications/rest/test_send_notification.py

@@ -20,8 +20,8 @@ from tests.app.conftest import (
     sample_email_template as create_sample_email_template,
     sample_template as create_sample_template,
     sample_service_whitelist as create_sample_service_whitelist,
-    sample_api_key as create_sample_api_key
+    sample_api_key as create_sample_api_key,
-)
+    sample_service)
 
 from app.models import Template
 from app.errors import InvalidRequest
@@ -1046,3 +1046,81 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
 
     assert response.status_code == 201
     mocked.assert_called_once_with([notification_id], queue='priority')
+
+
+def test_should_allow_store_original_number_on_sms_notification(notify_api, sample_template, mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
+
+            data = {
+                'to': '+(44) 7700-900 855',
+                'template': str(sample_template.id)
+            }
+
+            auth_header = create_authorization_header(service_id=sample_template.service_id)
+
+            response = client.post(
+                path='/notifications/sms',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+
+            response_data = json.loads(response.data)['data']
+            notification_id = response_data['notification']['id']
+
+            mocked.assert_called_once_with([notification_id], queue='send-sms')
+            assert response.status_code == 201
+            assert notification_id
+            notifications = Notification.query.all()
+            assert len(notifications) == 1
+            assert '+(44) 7700-900 855' == notifications[0].to
+
+
+def test_should_not_allow_international_number_on_sms_notification(notify_api, sample_template, mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
+
+            data = {
+                'to': '20-12-1234-1234',
+                'template': str(sample_template.id)
+            }
+
+            auth_header = create_authorization_header(service_id=sample_template.service_id)
+
+            response = client.post(
+                path='/notifications/sms',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+
+            assert not mocked.called
+            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'
+
+
+def test_should_allow_international_number_on_sms_notification(notify_api, notify_db, notify_db_session, mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
+
+            service = sample_service(notify_db, notify_db_session, can_send_international_sms=True)
+            template = create_sample_template(notify_db, notify_db_session, service=service)
+
+            data = {
+                'to': '20-12-1234-1234',
+                '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

tests/app/notifications/test_validators.py

@@ -2,12 +2,14 @@ import pytest
 from freezegun import freeze_time
 
 import app
+from app.models import KEY_TYPE_NORMAL
 from app.notifications.validators import (
     check_service_message_limit,
     check_template_is_for_notification_type,
     check_template_is_active,
     service_can_send_to_recipient,
-    check_sms_content_char_count
+    check_sms_content_char_count,
+    validate_and_format_recipient
 )
 from app.v2.errors import (
     BadRequestError,
@@ -262,3 +264,19 @@ def test_check_sms_content_char_count_fails(char_count, notify_api):
     assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format(
         notify_api.config['SMS_CHAR_COUNT_LIMIT'])
     assert e.value.fields == []
+
+
+@pytest.mark.parametrize('key_type', ['test', 'normal'])
+def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type):
+    with pytest.raises(BadRequestError) as e:
+        validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, 'sms')
+    assert e.value.status_code == 400
+    assert e.value.message == 'Cannot send to international mobile numbers'
+    assert e.value.fields == []
+
+
+@pytest.mark.parametrize('key_type', ['test', 'normal'])
+def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms(sample_service, key_type):
+    sample_service.can_send_international_sms = True
+    result = validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, 'sms')
+    assert result == '201212341234'

tests/app/v2/notifications/test_post_notifications.py

@@ -3,7 +3,7 @@ import pytest
 from flask import json
 from app.models import Notification
 from tests import create_authorization_header
-from tests.app.conftest import sample_template as create_sample_template
+from tests.app.conftest import sample_template as create_sample_template, sample_service
 
 
 @pytest.mark.parametrize("reference", [None, "reference_from_client"])
@@ -224,3 +224,71 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
 
     assert response.status_code == 201
     mocked.assert_called_once_with([notification_id], queue='priority')
+
+
+def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client, sample_service, sample_template):
+    data = {
+        'phone_number': '20-12-1234-1234',
+        'template_id': sample_template.id
+    }
+    auth_header = create_authorization_header(service_id=sample_service.id)
+
+    response = client.post(
+        path='/v2/notifications/sms',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 400
+    assert response.headers['Content-type'] == 'application/json'
+
+    error_json = json.loads(response.get_data(as_text=True))
+    assert error_json['status_code'] == 400
+    assert error_json['errors'] == [
+        {"error": "BadRequestError", "message": 'Cannot send to international mobile numbers'}
+    ]
+
+
+def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client):
+
+    service = sample_service(notify_db, notify_db_session, can_send_international_sms=True)
+    template = create_sample_template(notify_db, notify_db_session, service=service)
+
+    data = {
+        'phone_number': '20-12-1234-1234',
+        'template_id': template.id
+    }
+    auth_header = create_authorization_header(service_id=service.id)
+
+    response = client.post(
+        path='/v2/notifications/sms',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 201
+    assert response.headers['Content-type'] == 'application/json'
+
+
+def test_post_sms_should_persist_supplied_sms_number(notify_api, sample_template_with_placeholders, mocker):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            data = {
+                'phone_number': '+(44) 77009-00855',
+                'template_id': str(sample_template_with_placeholders.id),
+                'personalisation': {' Name': 'Jo'}
+            }
+
+            auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id)
+
+            response = client.post(
+                path='/v2/notifications/sms',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert response.status_code == 201
+            resp_json = json.loads(response.get_data(as_text=True))
+            notifications = Notification.query.all()
+            assert len(notifications) == 1
+            notification_id = notifications[0].id
+            assert '+(44) 77009-00855' == notifications[0].to
+            assert resp_json['id'] == str(notification_id)
+            assert mocked.called