diff --git a/app/__init__.py b/app/__init__.py index 9b415aae3..87863e235 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,5 +1,5 @@ import os - +import re from flask import request, url_for from flask import Flask, _request_ctx_stack from flask.ext.sqlalchemy import SQLAlchemy @@ -92,3 +92,10 @@ def get_db_version(): return full_name.split('_')[0] except: return 'n/a' + + +def email_safe(string): + return "".join([ + character.lower() if character.isalnum() or character == "." else "" + for character in re.sub("\s+", ".", string.strip()) + ]) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index cdf7ca781..2ab34b3b9 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -33,6 +33,36 @@ def send_sms(service_id, notification_id, encrypted_notification): current_app.logger.debug(e) +@notify_celery.task(name="send-email") +def send_email(service_id, notification_id, subject, from_address, encrypted_notification): + notification = encryption.decrypt(encrypted_notification) + template = get_model_templates(notification['template']) + + try: + notification_db_object = Notification( + id=notification_id, + template_id=notification['template'], + to=notification['to'], + service_id=service_id, + status='sent' + ) + save_notification(notification_db_object) + + try: + aws_ses_client.send_email( + from_address, + notification['to'], + subject, + template.content + ) + except AwsSesClientException as e: + current_app.logger.debug(e) + save_notification(notification_db_object, {"status": "failed"}) + + except SQLAlchemyError as e: + current_app.logger.debug(e) + + @notify_celery.task(name='send-sms-code') def send_sms_code(encrypted_verification): verification_message = encryption.decrypt(encrypted_verification) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 1d4a04770..2b90355a2 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -3,23 +3,30 @@ import uuid from flask import ( Blueprint, jsonify, - request + request, + current_app ) from app import api_user, encryption from app.aws_sqs import add_notification_to_queue -from app.dao import (templates_dao, notifications_dao) +from app.dao import ( + templates_dao, + users_dao, + services_dao, + notifications_dao +) from app.schemas import ( email_notification_schema, sms_template_notification_schema, notification_status_schema ) -from app.celery.tasks import send_sms +from app.celery.tasks import send_sms, send_email from sqlalchemy.orm.exc import NoResultFound notifications = Blueprint('notifications', __name__) from app.errors import register_errors + register_errors(notifications) @@ -47,6 +54,12 @@ def create_sms_notification(): except NoResultFound: return jsonify(result="error", message={'template': ['Template not found']}), 400 + service = services_dao.dao_fetch_service_by_id(api_user['client']) + + if service.restricted: + if notification['to'] not in [user.email_address for user in service.users]: + return jsonify(result="error", message={'to': ['Invalid phone number for restricted service']}), 400 + notification_id = create_notification_id() send_sms.apply_async(( @@ -59,17 +72,38 @@ def create_sms_notification(): @notifications.route('/email', methods=['POST']) def create_email_notification(): - resp_json = request.get_json() - notification, errors = email_notification_schema.load(resp_json) + notification, errors = email_notification_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 - notification_id = add_notification_to_queue(api_user['client'], "admin", 'email', notification) + + template = templates_dao.dao_get_template_by_id_and_service_id( + template_id=notification['template'], + service_id=api_user['client'] + ) + + if not template: + return jsonify(result="error", message={'template': ['Template not found']}), 400 + + service = services_dao.dao_fetch_service_by_id(api_user['client']) + + if service.restricted: + if notification['to'] not in [user.email_address for user in service.users]: + return jsonify(result="error", message={'to': ['Email address not permitted for restricted service']}), 400 + + notification_id = create_notification_id() + + send_email.apply_async(( + api_user['client'], + notification_id, + template.subject, + "{}@{}".format(service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + encryption.encrypt(notification)), + queue='email') return jsonify({'notification_id': notification_id}), 201 @notifications.route('/sms/service/', methods=['POST']) def create_sms_for_service(service_id): - resp_json = request.get_json() notification, errors = sms_template_notification_schema.load(resp_json) diff --git a/app/schemas.py b/app/schemas.py index 0d8724e00..c024c2be1 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -7,6 +7,8 @@ from marshmallow import (post_load, ValidationError, validates, validates_schema mobile_regex = re.compile("^\\+44[\\d]{10}$") +email_regex = re.compile("(^[^@^\\s]+@[^@^\\.^\\s]+(\\.[^@^\\.^\\s]*)*\.(.+))") + # TODO I think marshmallow provides a better integration and error handling. # Would be better to replace functionality in dao with the marshmallow supported @@ -78,9 +80,6 @@ class RequestVerifyCodeSchema(ma.Schema): to = fields.Str(required=False) -# TODO main purpose to be added later -# when processing templates, template will be -# common for all notifications. class NotificationSchema(ma.Schema): pass @@ -94,50 +93,25 @@ class SmsNotificationSchema(NotificationSchema): raise ValidationError('Invalid phone number, must be of format +441234123123') +class EmailNotificationSchema(NotificationSchema): + to = fields.Str(required=True) + template = fields.Int(required=True) + + @validates('to') + def validate_to(self, value): + if not email_regex.match(value): + raise ValidationError('Invalid email') + + class SmsTemplateNotificationSchema(SmsNotificationSchema): template = fields.Int(required=True) job = fields.String() - @validates_schema - def validate_schema(self, data): - """ - Validate the to field is valid for this notification - """ - from app import api_user - template_id = data.get('template', None) - template = models.Template.query.filter_by(id=template_id).first() - if template: - service = template.service - # Validate restricted service, - # restricted services can only send to one of its users. - if service.restricted: - valid = False - for usr in service.users: - if data['to'] == usr.mobile_number: - valid = True - break - if not valid: - raise ValidationError('Invalid phone number for restricted service', 'restricted') - # Assert the template is valid for the service which made the request. - service = api_user['client'] - admin_users = [current_app.config.get('ADMIN_CLIENT_USER_NAME'), - current_app.config.get('DELIVERY_CLIENT_USER_NAME')] - if (service not in admin_users and - template.service != models.Service.query.filter_by(id=service).first()): - raise ValidationError('Invalid template', 'restricted') - class SmsAdminNotificationSchema(SmsNotificationSchema): content = fields.Str(required=True) -class EmailNotificationSchema(NotificationSchema): - to_address = fields.Str(load_from="to", dump_to='to', required=True) - from_address = fields.Str(load_from="from", dump_to='from', required=True) - subject = fields.Str(required=True) - body = fields.Str(load_from="message", dump_to='message', required=True) - - class NotificationStatusSchema(BaseSchema): class Meta: diff --git a/app/service/rest.py b/app/service/rest.py index f12989543..51eae44cf 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,4 +1,3 @@ -import re from datetime import datetime from flask import ( @@ -30,6 +29,7 @@ from app.schemas import ( service_schema, api_keys_schema ) +from app import email_safe from flask import Blueprint @@ -76,7 +76,7 @@ def create_service(): data.pop('user_id', None) if 'name' in data: - data['email_from'] = _email_safe(data.get('name', None)) + data['email_from'] = email_safe(data.get('name', None)) valid_service, errors = service_schema.load(request.get_json()) @@ -87,13 +87,6 @@ def create_service(): return jsonify(data=service_schema.dump(valid_service).data), 201 -def _email_safe(string): - return "".join([ - character.lower() if character.isalnum() or character == "." else "" - for character in re.sub("\s+", ".", string.strip()) - ]) - - @service.route('/', methods=['POST']) def update_service(service_id): fetched_service = dao_fetch_service_by_id(service_id) diff --git a/config.py b/config.py index 8700d71a3..000112235 100644 --- a/config.py +++ b/config.py @@ -19,6 +19,7 @@ class Config(object): SQLALCHEMY_DATABASE_URI = os.environ['SQLALCHEMY_DATABASE_URI'] SQLALCHEMY_RECORD_QUERIES = True VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['VERIFY_CODE_FROM_EMAIL_ADDRESS'] + NOTIFY_EMAIL_DOMAIN = os.environ['NOTIFY_EMAIL_DOMAIN'] BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { diff --git a/environment_test.sh b/environment_test.sh index c4ce96bcf..815a6281b 100644 --- a/environment_test.sh +++ b/environment_test.sh @@ -15,4 +15,5 @@ export TWILIO_ACCOUNT_SID="test" export TWILIO_AUTH_TOKEN="test" export TWILIO_NUMBER="test" export FIRETEXT_API_KEY="Firetext" -export FIRETEXT_NUMBER="Firetext" \ No newline at end of file +export FIRETEXT_NUMBER="Firetext" +export NOTIFY_EMAIL_DOMAIN="test.notify.com" diff --git a/scripts/run_celery.sh b/scripts/run_celery.sh index ef67982d3..023b54920 100755 --- a/scripts/run_celery.sh +++ b/scripts/run_celery.sh @@ -3,4 +3,4 @@ set -e source environment.sh -celery -A run_celery.notify_celery worker --loglevel=INFO --logfile=/var/log/notify/application.log --concurrency=4 -Q sms,sms-code,email-code +celery -A run_celery.notify_celery worker --loglevel=INFO --logfile=/var/log/notify/application.log --concurrency=4 -Q sms,sms-code,email-code,email diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 37817c162..e252cfd09 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,8 +1,9 @@ import uuid import pytest from flask import current_app -from app.celery.tasks import (send_sms, send_sms_code, send_email_code) +from app.celery.tasks import (send_sms, send_sms_code, send_email_code, send_email) from app import (firetext_client, aws_ses_client, encryption) +from app.clients.email.aws_ses import AwsSesClientException from app.clients.sms.firetext import FiretextClientException from app.dao import notifications_dao from sqlalchemy.exc import SQLAlchemyError @@ -32,6 +33,36 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat assert persisted_notification.status == 'sent' +def test_should_send_template_to_email_provider_and_persist(sample_email_template, mocker): + notification = { + "template": sample_email_template.id, + "to": "my_email@my_email.com" + } + mocker.patch('app.encryption.decrypt', return_value=notification) + mocker.patch('app.aws_ses_client.send_email') + + notification_id = uuid.uuid4() + + send_email( + sample_email_template.service_id, + notification_id, + 'subject', + 'email_from', + "encrypted-in-reality") + + aws_ses_client.send_email.assert_called_once_with( + "email_from", + "my_email@my_email.com", + "subject", + sample_email_template.content + ) + persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id) + assert persisted_notification.id == notification_id + assert persisted_notification.to == 'my_email@my_email.com' + assert persisted_notification.template_id == sample_email_template.id + assert persisted_notification.status == 'sent' + + def test_should_persist_notification_as_failed_if_sms_client_fails(sample_template, mocker): notification = { "template": sample_template.id, @@ -55,13 +86,43 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa assert persisted_notification.status == 'failed' +def test_should_persist_notification_as_failed_if_email_client_fails(sample_email_template, mocker): + notification = { + "template": sample_email_template.id, + "to": "my_email@my_email.com" + } + mocker.patch('app.encryption.decrypt', return_value=notification) + mocker.patch('app.aws_ses_client.send_email', side_effect=AwsSesClientException()) + + notification_id = uuid.uuid4() + + send_email( + sample_email_template.service_id, + notification_id, + 'subject', + 'email_from', + "encrypted-in-reality") + + aws_ses_client.send_email.assert_called_once_with( + "email_from", + "my_email@my_email.com", + "subject", + sample_email_template.content + ) + persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id) + assert persisted_notification.id == notification_id + assert persisted_notification.to == 'my_email@my_email.com' + assert persisted_notification.template_id == sample_email_template.id + assert persisted_notification.status == 'failed' + + def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker): notification = { "template": sample_template.id, "to": "+441234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException()) + mocker.patch('app.firetext_client.send_sms') mocker.patch('app.db.session.add', side_effect=SQLAlchemyError()) notification_id = uuid.uuid4() @@ -77,6 +138,30 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker): assert 'No row was found for one' in str(e.value) +def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mocker): + notification = { + "template": sample_email_template.id, + "to": "my_email@my_email.com" + } + mocker.patch('app.encryption.decrypt', return_value=notification) + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.db.session.add', side_effect=SQLAlchemyError()) + + notification_id = uuid.uuid4() + + send_email( + sample_email_template.service_id, + notification_id, + 'subject', + 'email_from', + "encrypted-in-reality") + + aws_ses_client.send_email.assert_not_called() + with pytest.raises(NoResultFound) as e: + notifications_dao.get_notification(sample_email_template.service_id, notification_id) + assert 'No row was found for one' in str(e.value) + + def test_should_send_sms_code(mocker): notification = {'to': '+441234123123', 'secret_code': '12345'} diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 3f12fc1a0..c9c64eed0 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,5 +1,6 @@ import pytest +from app import email_safe from app.models import (User, Service, Template, ApiKey, Job, Notification) from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) from app.dao.services_dao import dao_create_service @@ -13,11 +14,24 @@ import uuid @pytest.fixture(scope='function') def service_factory(notify_db, notify_db_session): class ServiceFactory(object): - def get(self, service_name, user=None): + def get(self, service_name, user=None, template_type=None): if not user: user = sample_user(notify_db, notify_db_session) service = sample_service(notify_db, notify_db_session, service_name, user) - sample_template(notify_db, notify_db_session, service=service) + if template_type == 'email': + sample_template( + notify_db, + notify_db_session, + template_type=template_type, + subject_line=email_safe(service_name), + service=service + ) + else: + sample_template( + notify_db, + notify_db_session, + service=service + ) return service return ServiceFactory() @@ -93,7 +107,7 @@ def sample_service(notify_db, 'limit': 1000, 'active': False, 'restricted': False, - 'email_from': service_name + 'email_from': email_safe(service_name) } service = Service.query.filter_by(name=service_name).first() if not service: @@ -119,6 +133,33 @@ def sample_template(notify_db, 'content': content, 'service': service } + if template_type == 'email': + data.update({ + 'subject': subject_line + }) + template = Template(**data) + save_model_template(template) + return template + + +@pytest.fixture(scope='function') +def sample_email_template( + notify_db, + notify_db_session, + template_name="Email Template Name", + template_type="email", + content="This is a template", + subject_line='Email Subject', + service=None): + if service is None: + service = sample_service(notify_db, notify_db_session) + sample_api_key(notify_db, notify_db_session, service=service) + data = { + 'name': template_name, + 'template_type': template_type, + 'content': content, + 'service': service + } if subject_line: data.update({ 'subject': subject_line diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index d9f2d7f9c..575243e76 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -5,6 +5,7 @@ from tests import create_authorization_header from flask import json from app.models import Service from app.dao.templates_dao import get_model_templates +from app.dao.services_dao import dao_update_service def test_get_notification_by_id(notify_api, sample_notification): @@ -158,21 +159,20 @@ def test_prevents_sending_to_any_mobile_on_restricted_service(notify_api, sample app.celery.tasks.send_sms.apply_async.assert_not_called() assert response.status_code == 400 - assert 'Invalid phone number for restricted service' in json_resp['message']['restricted'] + assert 'Invalid phone number for restricted service' in json_resp['message']['to'] -def test_should_not_allow_template_from_another_service(notify_api, service_factory, mocker): +def test_should_not_allow_template_from_another_service(notify_api, service_factory, sample_user, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.tasks.send_sms.apply_async') - service_1 = service_factory.get('service 1') - service_2 = service_factory.get('service 2') + service_1 = service_factory.get('service 1', user=sample_user) + service_2 = service_factory.get('service 2', user=sample_user) service_2_templates = get_model_templates(service_id=service_2.id) - data = { - 'to': '+441234123123', + 'to': sample_user.mobile_number, 'template': service_2_templates[0].id } @@ -191,7 +191,7 @@ def test_should_not_allow_template_from_another_service(notify_api, service_fact app.celery.tasks.send_sms.apply_async.assert_not_called() assert response.status_code == 400 - assert 'Invalid template' in json_resp['message']['restricted'] + assert 'Template not found' in json_resp['message']['template'] def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker): @@ -228,27 +228,14 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker assert notification_id -@moto.mock_sqs -def test_send_email_valid_data(notify_api, - notify_db, - notify_db_session, - sample_service, - sample_admin_service_id, - sqs_client_conn, - mocker): +def test_create_email_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - to_address = "to@notify.com" - from_address = "from@notify.com" - subject = "This is the subject" - message = "This is the message" - data = { - 'to': to_address, - 'from': from_address, - 'subject': subject, - 'message': message - } + mocker.patch('app.celery.tasks.send_email.apply_async') + + data = {} auth_header = create_authorization_header( + service_id=sample_api_key.service_id, request_body=json.dumps(data), path='/notifications/email', method='POST') @@ -258,8 +245,166 @@ def test_send_email_valid_data(notify_api, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) + json_resp = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_email.apply_async.assert_not_called() + assert json_resp['result'] == 'error' + assert 'Missing data for required field.' in json_resp['message']['to'][0] + assert 'Missing data for required field.' in json_resp['message']['template'][0] + assert response.status_code == 400 + + +def test_should_reject_email_notification_with_bad_email(notify_api, sample_email_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + to_address = "bad-email" + data = { + 'to': to_address, + 'template': sample_email_template.service.id + } + auth_header = create_authorization_header( + service_id=sample_email_template.service.id, + request_body=json.dumps(data), + path='/notifications/email', + method='POST') + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + data = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_email.apply_async.assert_not_called() + assert response.status_code == 400 + assert data['result'] == 'error' + assert data['message']['to'][0] == 'Invalid email' + + +def test_should_reject_email_notification_with_template_id_that_cant_be_found( + notify_api, sample_email_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + data = { + 'to': 'ok@ok.com', + 'template': 1234 + } + auth_header = create_authorization_header( + service_id=sample_email_template.service.id, + request_body=json.dumps(data), + path='/notifications/email', + method='POST') + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + data = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_email.apply_async.assert_not_called() + assert response.status_code == 400 + assert data['result'] == 'error' + assert data['message']['template'][0] == 'Template not found' + + +def test_should_not_allow_email_template_from_another_service(notify_api, service_factory, sample_user, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + + service_1 = service_factory.get('service 1', template_type='email', user=sample_user) + service_2 = service_factory.get('service 2', template_type='email', user=sample_user) + + service_2_templates = get_model_templates(service_id=service_2.id) + + data = { + 'to': sample_user.email_address, + 'template': service_2_templates[0].id + } + + auth_header = create_authorization_header( + service_id=service_1.id, + request_body=json.dumps(data), + path='/notifications/email', + method='POST') + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_email.apply_async.assert_not_called() + + assert response.status_code == 400 + assert 'Template not found' in json_resp['message']['template'] + + +def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api, sample_email_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + + sample_email_template.service.restricted = True + dao_update_service(sample_email_template) + + data = { + 'to': "not-someone-we-trust@email-address.com", + 'template': sample_email_template.id + } + + auth_header = create_authorization_header( + service_id=sample_email_template.service.id, + request_body=json.dumps(data), + path='/notifications/email', + method='POST') + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_email.apply_async.assert_not_called() + + assert response.status_code == 400 + assert 'Email address not permitted for restricted service' in json_resp['message']['to'] + + +def test_should_allow_valid_email_notification(notify_api, sample_email_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + + data = { + 'to': 'ok@ok.com', + 'template': sample_email_template.id + } + + auth_header = create_authorization_header( + request_body=json.dumps(data), + path='/notifications/email', + method='POST', + service_id=sample_email_template.service_id + ) + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + notification_id = json.loads(response.data)['notification_id'] + app.celery.tasks.send_email.apply_async.assert_called_once_with( + (str(sample_email_template.service_id), + notification_id, + "Email Subject", + "sample.service@test.notify.com", + "something_encrypted"), + queue="email" + ) assert response.status_code == 201 - assert json.loads(response.data)['notification_id'] is not None + assert notification_id @moto.mock_sqs