diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 85909c5a2..c25d75aea 100644 --- a/app/notifications/validators.py +++ b/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): - if not service_allowed_to_send_to(send_to, service, key_type): +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, 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) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 6f630d74b..dbb4bd4fc 100644 --- a/app/service/send_notification.py +++ b/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']) diff --git a/app/service/utils.py b/app/service/utils.py index d25c6b7de..16521fbbd 100644 --- a/app/service/utils.py +++ b/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 ( diff --git a/requirements.txt b/requirements.txt index a36d0099b..8a3e83605 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,6 +25,6 @@ notifications-python-client==4.7.1 awscli==1.14.25 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@23.5.0#egg=notifications-utils==23.5.0 +git+https://github.com/alphagov/notifications-utils.git@23.5.2#egg=notifications-utils==23.5.2 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 1eb712f2e..aead9c5f1 100644 --- a/tests/app/notifications/test_validators.py +++ b/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'), diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index cbbe8f7dd..5a88e107d 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/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) }