diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ed1c4a871..692c960fd 100644 --- a/app/notifications/rest.py +++ b/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: diff --git a/app/notifications/validators.py b/app/notifications/validators.py index fb5880064..32aaa3b9c 100644 --- a/app/notifications/validators.py +++ b/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) - - 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") + international_phone_info = check_if_service_can_send_to_number(service, send_to) 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. " \ diff --git a/requirements-app.txt b/requirements-app.txt index d5834cb4b..f159a35ce 100644 --- a/requirements-app.txt +++ b/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 diff --git a/requirements.txt b/requirements.txt index 9d8a5bb82..1fc092b09 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index d5be60713..92c31f555 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/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) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 52f5fbfea..5fe5a513a 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/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 )