darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-01-30 14:31:57 -05:00
Code Issues Packages Projects Releases Wiki Activity

Use the same validation in the endpoint and the task to validate the phone number is ok to send to.

Browse Source 
Format the phone number before sending it to the sms provider.
This commit is contained in:
Rebecca Law
2016-03-16 13:33:12 +00:00
parent 977ac4f566
commit 4268f8453b
5 changed files with 112 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
app/celery/tasks.py
View File

@@ -25,7 +25,8 @@ from sqlalchemy.exc import SQLAlchemyError
from app.aws import s3
from datetime import datetime
from utils.template import Template
from utils.recipients import RecipientCSV, validate_phone_number, format_phone_number
from utils.recipients import RecipientCSV, format_phone_number, validate_phone_number
from app.validation import (allowed_send_to_email, allowed_send_to_number)
@notify_celery.task(name="delete-verify-codes")
@@ -204,7 +205,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
)
client.send_sms(
to=notification['to'],
to=format_phone_number(validate_phone_number(notification['to'])),
content=template.replaced,
reference=str(notification_id)
)
@@ -223,20 +224,6 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
current_app.logger.debug(e)
def allowed_send_to_number(service, to):
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 True
def allowed_send_to_email(service, to):
if service.restricted and to not in [user.email_address for user in service.users]:
return False
return True
@notify_celery.task(name="send-email")
def send_email(service_id, notification_id, subject, from_address, encrypted_notification, created_at):
notification = encryption.decrypt(encrypted_notification)
@@ -300,7 +287,9 @@ def send_sms_code(encrypted_verification):
verification_message = encryption.decrypt(encrypted_verification)
try:
firetext_client.send_sms(
verification_message['to'], verification_message['secret_code'], 'send-sms-code'
format_phone_number(validate_phone_number(verification_message['to'])),
verification_message['secret_code'],
'send-sms-code'
)
except FiretextClientException as e:
current_app.logger.exception(e)

5
app/notifications/rest.py
View File

@@ -26,6 +26,7 @@ from app.schemas import (
notification_status_schema
)
from app.celery.tasks import send_sms, send_email
from app.validation import allowed_send_to_number, allowed_send_to_email
notifications = Blueprint('notifications', __name__)
@@ -320,7 +321,7 @@ def send_notification(notification_type):
notification_id = create_uuid()
if notification_type == 'sms':
if service.restricted and notification['to'] not in [user.mobile_number for user in service.users]:
if not allowed_send_to_number(service, notification['to']):
return jsonify(
result="error", message={'to': ['Invalid phone number for restricted service']}), 400
send_sms.apply_async((
@@ -330,7 +331,7 @@ def send_notification(notification_type):
datetime.utcnow().strftime(DATETIME_FORMAT)
), queue='sms')
else:
if service.restricted and notification['to'] not in [user.email_address for user in service.users]:
if not allowed_send_to_email(service, notification['to']):
return jsonify(
result="error", message={'to': ['Email address not permitted for restricted service']}), 400
send_email.apply_async((

15
app/validation.py Normal file
View File

@@ -0,0 +1,15 @@
from utils.recipients import format_phone_number, validate_phone_number
def allowed_send_to_number(service, to):
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 True
def allowed_send_to_email(service, to):
if service.restricted and to not in [user.email_address for user in service.users]:
return False
return True

40
tests/app/celery/test_tasks.py
View File

@@ -1,6 +1,8 @@
import uuid
import pytest
from flask import current_app
from utils.recipients import validate_phone_number, format_phone_number
from app.celery.tasks import (
send_sms,
send_sms_code,
@@ -248,7 +250,7 @@ def test_should_process_all_sms_job(sample_job, sample_job_with_placeholdered_te
def test_should_send_template_to_correct_sms_provider_and_persist(sample_template_with_placeholders, mocker):
notification = {
"template": sample_template_with_placeholders.id,
"to": "+441234123123",
"to": "+447234123123",
"personalisation": {"name": "Jo"}
}
mocker.patch('app.encryption.decrypt', return_value=notification)
@@ -265,7 +267,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
)
firetext_client.send_sms.assert_called_once_with(
to="+441234123123",
to=format_phone_number(validate_phone_number("+447234123123")),
content="Sample service: Hello Jo",
reference=str(notification_id)
)
@@ -273,7 +275,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
sample_template_with_placeholders.service_id, notification_id
)
assert persisted_notification.id == notification_id
assert persisted_notification.to == '+441234123123'
assert persisted_notification.to == '+447234123123'
assert persisted_notification.template_id == sample_template_with_placeholders.id
assert persisted_notification.status == 'sent'
assert persisted_notification.created_at == now
@@ -285,7 +287,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
def test_should_send_sms_without_personalisation(sample_template, mocker):
notification = {
"template": sample_template.id,
"to": "+441234123123"
"to": "+447234123123"
}
mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms')
@@ -301,7 +303,7 @@ def test_should_send_sms_without_personalisation(sample_template, mocker):
)
firetext_client.send_sms.assert_called_once_with(
to="+441234123123",
to=format_phone_number(validate_phone_number("+447234123123")),
content="Sample service: This is a template",
reference=str(notification_id)
)
@@ -330,7 +332,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
)
firetext_client.send_sms.assert_called_once_with(
to="+447700900890",
to=format_phone_number(validate_phone_number("+447700900890")),
content="Sample service: This is a template",
reference=str(notification_id)
)
@@ -396,7 +398,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa
notification = {
"template": sample_job.template.id,
"job": sample_job.id,
"to": "+441234123123"
"to": "+447234123123"
}
mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms')
@@ -411,13 +413,13 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa
now.strftime(DATETIME_FORMAT)
)
firetext_client.send_sms.assert_called_once_with(
to="+441234123123",
to=format_phone_number(validate_phone_number("+447234123123")),
content="Sample service: This is a template",
reference=str(notification_id)
)
persisted_notification = notifications_dao.get_notification(sample_job.template.service_id, notification_id)
assert persisted_notification.id == notification_id
assert persisted_notification.to == '+441234123123'
assert persisted_notification.to == '+447234123123'
assert persisted_notification.job_id == sample_job.id
assert persisted_notification.template_id == sample_job.template.id
assert persisted_notification.status == 'sent'
@@ -520,7 +522,7 @@ def test_should_use_email_template_and_persist_without_personalisation(
def test_should_persist_notification_as_failed_if_sms_client_fails(sample_template, mocker):
notification = {
"template": sample_template.id,
"to": "+441234123123"
"to": "+447234123123"
}
mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error()))
@@ -536,13 +538,13 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa
now.strftime(DATETIME_FORMAT)
)
firetext_client.send_sms.assert_called_once_with(
to="+441234123123",
to=format_phone_number(validate_phone_number("+447234123123")),
content="Sample service: This is a template",
reference=str(notification_id)
)
persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
assert persisted_notification.id == notification_id
assert persisted_notification.to == '+441234123123'
assert persisted_notification.to == '+447234123123'
assert persisted_notification.template_id == sample_template.id
assert persisted_notification.status == 'failed'
assert persisted_notification.created_at == now
@@ -590,7 +592,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai
def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
notification = {
"template": sample_template.id,
"to": "+441234123123"
"to": "+447234123123"
}
mocker.patch('app.encryption.decrypt', return_value=notification)
mocker.patch('app.firetext_client.send_sms')
@@ -638,24 +640,28 @@ def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mo
def test_should_send_sms_code(mocker):
notification = {'to': '+441234123123',
notification = {'to': '+447234123123',
'secret_code': '12345'}
encrypted_notification = encryption.encrypt(notification)
mocker.patch('app.firetext_client.send_sms')
send_sms_code(encrypted_notification)
firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code')
firetext_client.send_sms.assert_called_once_with(format_phone_number(validate_phone_number(notification['to'])),
notification['secret_code'],
'send-sms-code')
def test_should_throw_firetext_client_exception(mocker):
notification = {'to': '+441234123123',
notification = {'to': '+447234123123',
'secret_code': '12345'}
encrypted_notification = encryption.encrypt(notification)
mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error()))
send_sms_code(encrypted_notification)
firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code')
firetext_client.send_sms.assert_called_once_with(format_phone_number(validate_phone_number(notification['to'])),
notification['secret_code'],
'send-sms-code')
def test_should_send_email_code(mocker):

65
tests/app/test_validation.py Normal file
View File

@@ -0,0 +1,65 @@
from app.models import User, Service
from app.validation import allowed_send_to_number, allowed_send_to_email
def test_allowed_send_to_number_returns_true_for_restricted_service_with_same_number():
mobile_number = '07524609792'
service = _create_service_data(mobile_number)
assert allowed_send_to_number(service, mobile_number)
def test_allowed_send_to_number_returns_false_for_restricted_service_with_different_number():
mobile_number = '00447524609792'
service = _create_service_data(mobile_number)
assert not allowed_send_to_number(service, '+447344609793')
def test_allowed_send_to_number_returns_true_for_unrestricted_service_with_different_number():
mobile_number = '+447524609792'
service = _create_service_data(mobile_number, False)
assert allowed_send_to_number(service, '+447344609793')
def test_allowed_send_to_email__returns_true_for_restricted_service_with_same_email():
email = 'testing@it.gov.uk'
service = _create_service_data(email_address=email)
assert allowed_send_to_email(service, email)
def test_allowed_send_to_email__returns_false_for_restricted_service_with_different_email():
email = 'testing@it.gov.uk'
service = _create_service_data(email_address=email)
assert not allowed_send_to_email(service, 'another@it.gov.uk')
def test_allowed_send_to_email__returns_false_for_restricted_service_with_different_email():
email = 'testing@it.gov.uk'
service = _create_service_data(email_address=email)
assert not allowed_send_to_email(service, 'another@it.gov.uk')
def test_allowed_send_to_email__returns_true_for_unrestricted_service_with_different_email():
email = 'testing@it.gov.uk'
service = _create_service_data(email_address=email, restricted=False)
assert allowed_send_to_number(service, 'another@it.gov.uk')
def _create_service_data(mobile_number='+447524609792', restricted=True, email_address='test_user@it.gov.uk'):
usr = {
'name': 'Test User',
'email_address': email_address,
'password': 'password',
'mobile_number': mobile_number,
'state': 'active'
}
user = User(**usr)
data = {
'name': 'Test service',
'limit': 10,
'active': False,
'restricted': restricted,
'email_from': 'test_service@it.gov.uk'
}
service = Service(**data)
service.users = [user]
return service