Merge pull request #147 from alphagov/fix-phone-number-matching-restricted-service

Be agnostic about format when comparing phone numbers
This commit is contained in:
Chris Hill-Scott
2016-03-11 13:51:00 +00:00
2 changed files with 9 additions and 7 deletions

View File

@@ -24,7 +24,7 @@ from sqlalchemy.exc import SQLAlchemyError
from app.aws import s3 from app.aws import s3
from datetime import datetime from datetime import datetime
from utils.template import Template from utils.template import Template
from utils.recipients import RecipientCSV from utils.recipients import RecipientCSV, validate_phone_number, format_phone_number
@notify_celery.task(name="delete-verify-codes") @notify_celery.task(name="delete-verify-codes")
@@ -219,7 +219,9 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
def allowed_send_to_number(service, to): def allowed_send_to_number(service, to):
if service.restricted and to not in [user.mobile_number for user in service.users]: if service.restricted and format_phone_number(validate_phone_number(to)) not in [
format_phone_number(validate_phone_number(user.mobile_number)) for user in service.users
]:
return False return False
return True return True

View File

@@ -296,13 +296,13 @@ def test_should_send_sms_without_personalisation(sample_template, mocker):
def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker):
user = sample_user(notify_db, notify_db_session, mobile_numnber="+441234123123") user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900890")
service = sample_service(notify_db, notify_db_session, user=user, restricted=True) service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
template = sample_template(notify_db, notify_db_session, service=service) template = sample_template(notify_db, notify_db_session, service=service)
notification = { notification = {
"template": template.id, "template": template.id,
"to": "+441234123123" "to": "+447700900890" # The users own number, but in a different format
} }
mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms') mocker.patch('app.firetext_client.send_sms')
@@ -317,17 +317,17 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
now.strftime(DATETIME_FORMAT) now.strftime(DATETIME_FORMAT)
) )
firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template") firetext_client.send_sms.assert_called_once_with("+447700900890", "Sample service: This is a template")
def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker):
user = sample_user(notify_db, notify_db_session, mobile_numnber="+441234123123") user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205")
service = sample_service(notify_db, notify_db_session, user=user, restricted=True) service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
template = sample_template(notify_db, notify_db_session, service=service) template = sample_template(notify_db, notify_db_session, service=service)
notification = { notification = {
"template": template.id, "template": template.id,
"to": "+440000000000" "to": "07700 900849"
} }
mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms') mocker.patch('app.firetext_client.send_sms')