Merge pull request #923 from alphagov/validate-int-numbers-on-api

Validate International phone numbers
This commit is contained in:
minglis
2017-04-27 13:21:04 +01:00
committed by GitHub
13 changed files with 262 additions and 49 deletions

View File

@@ -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
else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'])
if notification_type == SMS_TYPE:
formatted_simulated_numbers = [
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']

View File

@@ -2,8 +2,7 @@ from flask import (
Blueprint,
jsonify,
request,
current_app,
json
current_app
)
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'],
service_id=service.id)
template = templates_dao.dao_get_template_by_id_and_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:

View File

@@ -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)

View File

@@ -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)

View File

@@ -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

View File

@@ -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,

View File

@@ -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.3#egg=notifications-utils==16.1.3
git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

View File

@@ -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:

View File

@@ -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

View File

@@ -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,75 @@ 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(client, sample_template, mocker):
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(client, sample_template, mocker):
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(client, notify_db, notify_db_session, mocker):
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

View File

@@ -9,10 +9,13 @@ from collections import namedtuple
from app.models import Template, Notification, NotificationHistory
from app.notifications import SendNotificationToQueueError
from app.notifications.process_notifications import (create_content_for_notification,
persist_notification,
send_notification_to_queue,
simulated_recipient)
from app.notifications.process_notifications import (
create_content_for_notification,
persist_notification,
send_notification_to_queue,
simulated_recipient
)
from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
from app.utils import cache_key_for_service_template_counter
from app.v2.errors import BadRequestError
from tests.app.conftest import sample_api_key as create_api_key
@@ -261,22 +264,36 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(sample
assert NotificationHistory.query.count() == 0
@pytest.mark.parametrize("to_address, notification_type, expected",
[("+447700900000", "sms", True),
("+447700900111", "sms", True),
("+447700900222", "sms", True),
("simulate-delivered@notifications.service.gov.uk", "email", True),
("simulate-delivered-2@notifications.service.gov.uk", "email", True),
("simulate-delivered-3@notifications.service.gov.uk", "email", True),
("07515896969", "sms", False),
("valid_email@test.com", "email", False)])
@pytest.mark.parametrize("to_address, notification_type, expected", [
("+447700900000", "sms", True),
("+447700900111", "sms", True),
("+447700900222", "sms", True),
("07700900000", "sms", True),
("7700900111", "sms", True),
("simulate-delivered@notifications.service.gov.uk", "email", True),
("simulate-delivered-2@notifications.service.gov.uk", "email", True),
("simulate-delivered-3@notifications.service.gov.uk", "email", True),
("07515896969", "sms", False),
("valid_email@test.com", "email", False)
])
def test_simulated_recipient(notify_api, to_address, notification_type, expected):
# The values where the expected = 'research-mode' are listed in the config['SIMULATED_EMAIL_ADDRESSES']
# and config['SIMULATED_SMS_NUMBERS']. These values should result in using the research mode queue.
# SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk',
# 'simulate-delivered-2@notifications.service.gov.uk',
# 'simulate-delivered-2@notifications.service.gov.uk')
# SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222')
"""
The values where the expected = 'research-mode' are listed in the config['SIMULATED_EMAIL_ADDRESSES']
and config['SIMULATED_SMS_NUMBERS']. These values should result in using the research mode queue.
SIMULATED_EMAIL_ADDRESSES = (
'simulate-delivered@notifications.service.gov.uk',
'simulate-delivered-2@notifications.service.gov.uk',
'simulate-delivered-2@notifications.service.gov.uk'
)
SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222')
"""
formatted_address = None
actual = simulated_recipient(to_address, notification_type)
assert actual == expected
if notification_type == 'email':
formatted_address = validate_and_format_email_address(to_address)
else:
formatted_address = validate_and_format_phone_number(to_address)
is_simulated_address = simulated_recipient(formatted_address, notification_type)
assert is_simulated_address == expected

View File

@@ -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'

View File

@@ -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"])
@@ -200,6 +200,9 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
notification_type,
key_send_to,
send_to):
mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
sample = create_sample_template(
notify_db,
notify_db_session,
@@ -224,3 +227,74 @@ 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, mocker):
service = sample_service(notify_db, notify_db_session, can_send_international_sms=True)
template = create_sample_template(notify_db, notify_db_session, service=service)
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
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])
print(json.loads(response.get_data(as_text=True)))
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