diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index b03d8ee69..ed45d4005 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -9,7 +9,7 @@ from notifications_utils.s3 import s3upload from app import notify_celery from app.aws.s3 import file_exists -from app.models import SMS_TYPE +from app.models import EXAMPLE_DOMAINS, OFCOM_PHONE_NUMBER_RANGE, SMS_TYPE from app.config import QueueNames from app.celery.process_ses_receipts_tasks import process_ses_results @@ -48,6 +48,8 @@ def send_email_response(reference, to): body = ses_hard_bounce_callback(reference) elif to == temp_fail_email: body = ses_soft_bounce_callback(reference) + elif to.lower().endswith(EXAMPLE_DOMAINS): + body = ses_hard_bounce_callback(reference) else: body = ses_notification_callback(reference) @@ -88,10 +90,14 @@ def mmg_callback(notification_id, to): status: 5 - rejected (perm failure) """ - if to.strip().endswith(temp_fail): + if to.strip().endswith(delivered): + status = "3" + elif to.strip().endswith(temp_fail): status = "4" elif to.strip().endswith(perm_fail): status = "5" + elif to.startswith(OFCOM_PHONE_NUMBER_RANGE): + status = "5" else: status = "3" @@ -107,10 +113,14 @@ def firetext_callback(notification_id, to): status: 0 - delivered status: 1 - perm failure """ - if to.strip().endswith(perm_fail): + if to.strip().endswith(delivered): + status = "0" + elif to.strip().endswith(perm_fail): status = "1" elif to.strip().endswith(temp_fail): status = "2" + elif to.startswith(OFCOM_PHONE_NUMBER_RANGE): + status = "1" else: status = "0" return { diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 8f8a09764..b598cd29e 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -21,7 +21,6 @@ from app.dao.templates_dao import dao_get_template_by_id from app.exceptions import NotificationTechnicalFailureException from app.models import ( SMS_TYPE, - KEY_TYPE_TEST, BRANDING_BOTH, BRANDING_ORG_BANNER, EMAIL_TYPE, @@ -50,7 +49,7 @@ def send_sms_to_provider(notification): show_prefix=service.prefix_sms, ) - if service.research_mode or notification.key_type == KEY_TYPE_TEST: + if service.research_mode or notification.dont_send_to_provider: update_notification_to_sending(notification, provider) send_sms_response(provider.get_name(), str(notification.id), notification.to) @@ -96,7 +95,7 @@ def send_email_to_provider(notification): values=notification.personalisation ) - if service.research_mode or notification.key_type == KEY_TYPE_TEST: + if service.research_mode or notification.dont_send_to_provider: notification.reference = str(create_uuid()) update_notification_to_sending(notification, provider) send_email_response(notification.reference, notification.to) diff --git a/app/models.py b/app/models.py index c98cce026..4914d760c 100644 --- a/app/models.py +++ b/app/models.py @@ -61,6 +61,13 @@ DELIVERY_STATUS_CALLBACK_TYPE = 'delivery_status' COMPLAINT_CALLBACK_TYPE = 'complaint' SERVICE_CALLBACK_TYPES = [DELIVERY_STATUS_CALLBACK_TYPE, COMPLAINT_CALLBACK_TYPE] +EXAMPLE_DOMAINS = ( + '@example.com', '@example.net', '@example.org', + '.example.com', '.example.net', '.example.org' +) + +OFCOM_PHONE_NUMBER_RANGE = '447700900' + def filter_null_value_fields(obj): return dict( @@ -1499,6 +1506,16 @@ class Notification(db.Model): template_object = get_template_instance(self.template.__dict__, self.personalisation) return template_object.subject + @property + def dont_send_to_provider(self): + if self.key_type == KEY_TYPE_TEST: + return True + if self.notification_type == EMAIL_TYPE: + return self.normalised_to.endswith(EXAMPLE_DOMAINS) + if self.notification_type == SMS_TYPE: + return self.normalised_to.startswith(OFCOM_PHONE_NUMBER_RANGE) + return False + @property def formatted_status(self): return { diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index 7a00f7163..853d2d568 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -75,7 +75,15 @@ def test_delivered_mmg_callback(phone_number): assert data['CID'] == "1234" -@pytest.mark.parametrize("phone_number", ["07700900002", "+447700900002", "7700900002", "+44 7700900002"]) +@pytest.mark.parametrize("phone_number", [ + "07700900002", + "+447700900002", + "7700900002", + "+44 7700900002", + "07700900001", + "07700900456", + "07700900999", +]) def test_perm_failure_mmg_callback(phone_number): data = json.loads(mmg_callback("1234", phone_number)) assert data['MSISDN'] == phone_number @@ -105,7 +113,7 @@ def test_delivered_firetext_callback(phone_number): @pytest.mark.parametrize("phone_number", ["07700900002", "+447700900002", "7700900002", "+44 7700900002"]) -def test_failure_firetext_callback(phone_number): +def test_temp_failure_firetext_callback(phone_number): assert firetext_callback('1234', phone_number) == { 'mobile': phone_number, 'status': '1', @@ -114,6 +122,25 @@ def test_failure_firetext_callback(phone_number): } +@pytest.mark.parametrize("phone_number", [ + "07700900002", + "+447700900002", + "7700900002", + "+44 7700900002", + "07700900 000", + "07700900 001", + "07700900 456", + "07700900 999", +]) +def test_perm_failure_firetext_callback(phone_number): + assert firetext_callback('1234', phone_number) == { + 'mobile': phone_number, + 'status': '2', + 'time': '2016-03-10 14:17:00', + 'reference': '1234' + } + + @freeze_time("2018-01-25 14:00:30") def test_create_fake_letter_response_file_uploads_response_file_s3( notify_api, mocker): diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 92c6f8fed..0cb17838f 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -5,7 +5,10 @@ from unittest.mock import ANY import pytest from flask import current_app -from notifications_utils.recipients import validate_and_format_phone_number +from notifications_utils.recipients import ( + format_email_address, + validate_and_format_phone_number, +) from requests import HTTPError import app @@ -229,6 +232,88 @@ def test_should_call_send_sms_response_task_if_research_mode( assert not persisted_notification.personalisation +@pytest.mark.parametrize('recipient', [ + '+44 07700-900-000', + '07700 900 000', + '07700 900 001', + '07700 900 555', + '07700 900 999', + pytest.param('07700 901 000', marks=pytest.mark.xfail), +]) +def test_should_not_send_to_known_fake_phone_numbers( + notify_db, + sample_template, + sample_user, + mocker, + recipient, +): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.delivery.send_to_providers.send_sms_response') + + notification = create_notification( + to_field=recipient, + normalised_to=validate_and_format_phone_number(recipient), + template=sample_template, + status='created', + billable_units=0, + ) + + send_to_providers.send_sms_to_provider(notification) + + assert notification.key_type == 'normal' + assert mmg_client.send_sms.called is False + + app.delivery.send_to_providers.send_sms_response.assert_called_once_with( + 'mmg', str(notification.id), notification.to + ) + + persisted_notification = notifications_dao.get_notification_by_id(notification.id) + assert persisted_notification.to == recipient + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_by == 'mmg' + assert persisted_notification.billable_units == 0 + + +@pytest.mark.parametrize('recipient', [ + 'test@example.com', + 'test@test.example.net', + 'test@eXaMPlE.OrG', + pytest.param('test@some-example.org', marks=pytest.mark.xfail), +]) +def test_should_not_send_to_known_fake_email_domains( + notify_db, + sample_email_template, + sample_user, + mocker, + recipient, +): + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.delivery.send_to_providers.send_email_response') + reference = uuid.uuid4() + mocker.patch('app.uuid.uuid4', return_value=reference) + + notification = create_notification( + to_field=recipient, + normalised_to=format_email_address(recipient), + template=sample_email_template, + status='created', + ) + + send_to_providers.send_email_to_provider(notification) + + assert notification.key_type == 'normal' + assert app.aws_ses_client.send_email.called is False + + app.delivery.send_to_providers.send_email_response.assert_called_once_with( + str(reference), notification.to + ) + + persisted_notification = notifications_dao.get_notification_by_id(notification.id) + assert persisted_notification.to == recipient + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_by == 'ses' + + def test_should_have_sending_status_if_fake_callback_function_fails(sample_notification, mocker): mocker.patch('app.delivery.send_to_providers.send_sms_response', side_effect=HTTPError)