mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-03 01:41:05 -05:00
Catch sending to restricted recipients in Celery
The Celery `send_sms` and `send_email` tasks _could_ assume that all the
recipients it gets are safe, because they have been checked either:
- when the admin app processes the CSV
- in the `/notifications/sms|email` endpoint
*However*, it’s probably safer to make the check again when the Celery
task run and passes the message off to the third party.
This also means that changing a service _back_ to restricted would have
an effect on messages that were queued, as well as all subsequent
messages.
This commit:
- restores the test that was removed here:
e56aee5d1d (diff-e5627619e387fccc04783c32a23e6859L346)
- adds checks back into the Celery tasks for sending email and SMS,
using the `allowed_to_send_to` function that was introduced into utils
in https://github.com/alphagov/notifications-utils/pull/16
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import itertools
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
|
||||||
from flask import current_app
|
from flask import current_app
|
||||||
@@ -12,7 +13,8 @@ from utils.template import Template
|
|||||||
|
|
||||||
from utils.recipients import (
|
from utils.recipients import (
|
||||||
RecipientCSV,
|
RecipientCSV,
|
||||||
validate_and_format_phone_number
|
validate_and_format_phone_number,
|
||||||
|
allowed_to_send_to
|
||||||
)
|
)
|
||||||
|
|
||||||
from app import (
|
from app import (
|
||||||
@@ -188,9 +190,16 @@ def process_job(job_id):
|
|||||||
def send_sms(service_id, notification_id, encrypted_notification, created_at):
|
def send_sms(service_id, notification_id, encrypted_notification, created_at):
|
||||||
notification = encryption.decrypt(encrypted_notification)
|
notification = encryption.decrypt(encrypted_notification)
|
||||||
service = dao_fetch_service_by_id(service_id)
|
service = dao_fetch_service_by_id(service_id)
|
||||||
|
|
||||||
client = firetext_client
|
client = firetext_client
|
||||||
|
|
||||||
|
restricted = False
|
||||||
|
|
||||||
|
if not service_allowed_to_send_to(notification['to'], service):
|
||||||
|
current_app.logger.info(
|
||||||
|
"SMS {} failed as restricted service".format(notification_id)
|
||||||
|
)
|
||||||
|
restricted = True
|
||||||
|
|
||||||
try:
|
try:
|
||||||
sent_at = datetime.utcnow()
|
sent_at = datetime.utcnow()
|
||||||
notification_db_object = Notification(
|
notification_db_object = Notification(
|
||||||
@@ -199,7 +208,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
|
|||||||
to=notification['to'],
|
to=notification['to'],
|
||||||
service_id=service_id,
|
service_id=service_id,
|
||||||
job_id=notification.get('job', None),
|
job_id=notification.get('job', None),
|
||||||
status='sent',
|
status='failed' if restricted else 'sent',
|
||||||
created_at=datetime.strptime(created_at, DATETIME_FORMAT),
|
created_at=datetime.strptime(created_at, DATETIME_FORMAT),
|
||||||
sent_at=sent_at,
|
sent_at=sent_at,
|
||||||
sent_by=client.get_name()
|
sent_by=client.get_name()
|
||||||
@@ -207,6 +216,9 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
|
|||||||
|
|
||||||
dao_create_notification(notification_db_object, TEMPLATE_TYPE_SMS)
|
dao_create_notification(notification_db_object, TEMPLATE_TYPE_SMS)
|
||||||
|
|
||||||
|
if restricted:
|
||||||
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
template = Template(
|
template = Template(
|
||||||
dao_get_template_by_id(notification['template']).__dict__,
|
dao_get_template_by_id(notification['template']).__dict__,
|
||||||
@@ -238,6 +250,15 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
|
|||||||
def send_email(service_id, notification_id, subject, from_address, encrypted_notification, created_at):
|
def send_email(service_id, notification_id, subject, from_address, encrypted_notification, created_at):
|
||||||
notification = encryption.decrypt(encrypted_notification)
|
notification = encryption.decrypt(encrypted_notification)
|
||||||
client = aws_ses_client
|
client = aws_ses_client
|
||||||
|
service = dao_fetch_service_by_id(service_id)
|
||||||
|
|
||||||
|
restricted = False
|
||||||
|
|
||||||
|
if not service_allowed_to_send_to(notification['to'], service):
|
||||||
|
current_app.logger.info(
|
||||||
|
"Email {} failed as restricted service".format(notification_id)
|
||||||
|
)
|
||||||
|
restricted = True
|
||||||
|
|
||||||
try:
|
try:
|
||||||
sent_at = datetime.utcnow()
|
sent_at = datetime.utcnow()
|
||||||
@@ -247,13 +268,16 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
|
|||||||
to=notification['to'],
|
to=notification['to'],
|
||||||
service_id=service_id,
|
service_id=service_id,
|
||||||
job_id=notification.get('job', None),
|
job_id=notification.get('job', None),
|
||||||
status='sent',
|
status='failed' if restricted else 'sent',
|
||||||
created_at=datetime.strptime(created_at, DATETIME_FORMAT),
|
created_at=datetime.strptime(created_at, DATETIME_FORMAT),
|
||||||
sent_at=sent_at,
|
sent_at=sent_at,
|
||||||
sent_by=client.get_name()
|
sent_by=client.get_name()
|
||||||
)
|
)
|
||||||
dao_create_notification(notification_db_object, TEMPLATE_TYPE_EMAIL)
|
dao_create_notification(notification_db_object, TEMPLATE_TYPE_EMAIL)
|
||||||
|
|
||||||
|
if restricted:
|
||||||
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
template = Template(
|
template = Template(
|
||||||
dao_get_template_by_id(notification['template']).__dict__,
|
dao_get_template_by_id(notification['template']).__dict__,
|
||||||
@@ -388,3 +412,16 @@ def email_registration_verification(encrypted_verification_message):
|
|||||||
url=verification_message['url']))
|
url=verification_message['url']))
|
||||||
except AwsSesClientException as e:
|
except AwsSesClientException as e:
|
||||||
current_app.logger.exception(e)
|
current_app.logger.exception(e)
|
||||||
|
|
||||||
|
|
||||||
|
def service_allowed_to_send_to(recipient, service):
|
||||||
|
|
||||||
|
if not service.restricted:
|
||||||
|
return True
|
||||||
|
|
||||||
|
return allowed_to_send_to(
|
||||||
|
recipient,
|
||||||
|
itertools.chain.from_iterable(
|
||||||
|
[user.mobile_number, user.email_address] for user in service.users
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|||||||
@@ -343,6 +343,31 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
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="07700 900205")
|
||||||
|
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
|
||||||
|
template = sample_template(notify_db, notify_db_session, service=service)
|
||||||
|
|
||||||
|
notification = {
|
||||||
|
"template": template.id,
|
||||||
|
"to": "07700 900849"
|
||||||
|
}
|
||||||
|
mocker.patch('app.encryption.decrypt', return_value=notification)
|
||||||
|
mocker.patch('app.firetext_client.send_sms')
|
||||||
|
mocker.patch('app.firetext_client.get_name', return_value="firetext")
|
||||||
|
|
||||||
|
notification_id = uuid.uuid4()
|
||||||
|
now = datetime.utcnow()
|
||||||
|
send_sms(
|
||||||
|
service.id,
|
||||||
|
notification_id,
|
||||||
|
"encrypted-in-reality",
|
||||||
|
now.strftime(DATETIME_FORMAT)
|
||||||
|
)
|
||||||
|
|
||||||
|
firetext_client.send_sms.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
def test_should_send_email_if_restricted_service_and_valid_email(notify_db, notify_db_session, mocker):
|
def test_should_send_email_if_restricted_service_and_valid_email(notify_db, notify_db_session, mocker):
|
||||||
user = sample_user(notify_db, notify_db_session, email="test@restricted.com")
|
user = sample_user(notify_db, notify_db_session, email="test@restricted.com")
|
||||||
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
|
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
|
||||||
|
|||||||
Reference in New Issue
Block a user