diff --git a/app/__init__.py b/app/__init__.py index f035df8ed..aa64ec5d6 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -17,7 +17,7 @@ from app.clients.sms.twilio import TwilioClient db = SQLAlchemy() ma = Marshmallow() notify_alpha_client = NotifyAPIClient() -celery = NotifyCelery() +notify_celery = NotifyCelery() twilio_client = TwilioClient() @@ -35,7 +35,7 @@ def create_app(config_name, config_overrides=None): ma.init_app(application) logging.init_app(application) twilio_client.init_app(application) - celery.init_app(application) + notify_celery.init_app(application) from app.service.rest import service as service_blueprint from app.user.rest import user as user_blueprint diff --git a/app/celery/tasks.py b/app/celery/tasks.py index e8d20d009..f17b844d7 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -1,12 +1,12 @@ from itsdangerous import URLSafeSerializer -from app import celery, twilio_client, db +from app import notify_celery, twilio_client, db from app.clients.sms.twilio import TwilioClientException from app.dao.templates_dao import get_model_templates from app.models import Notification from flask import current_app -@celery.task(name="send-sms", bind="True") +@notify_celery.task(name="send-sms", bind="True") def send_sms(service_id, notification_id, encrypted_notification, secret_key, salt): serializer = URLSafeSerializer(secret_key) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 6a9a888c2..51974437d 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -39,12 +39,15 @@ def get_notifications(notification_id): def create_sms_notification(): serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - resp_json = request.get_json() - - notification, errors = sms_template_notification_schema.load(resp_json) + notification, errors = sms_template_notification_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 + try: + templates_dao.get_model_templates(template_id=notification['template'], service_id=api_user['client']) + except NoResultFound: + return jsonify(result="error", message={'template': ['Template not found']}), 400 + notification_id = create_notification_id() encrypted_notification = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) diff --git a/app/schemas.py b/app/schemas.py index b8fbbd757..9810cad6d 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -93,13 +93,6 @@ class SmsTemplateNotificationSchema(SmsNotificationSchema): template = fields.Int(required=True) job = fields.String() - @validates('template') - def validate_template(self, value): - if not models.Template.query.filter_by(id=value).first(): - # TODO is this message consistent with what marshmallow - # would normally produce. - raise ValidationError('Template not found') - @validates_schema def validate_schema(self, data): """ @@ -141,7 +134,7 @@ class EmailNotificationSchema(NotificationSchema): class NotificationStatusSchema(BaseSchema): class Meta: - model = models.Noti∫~fication + model = models.Notification user_schema = UserSchema() diff --git a/run_celery.py b/run_celery.py index 269142d5b..46217adae 100644 --- a/run_celery.py +++ b/run_celery.py @@ -1,6 +1,6 @@ #!/usr/bin/env python import os -from app import celery, create_app +from app import notify_celery, create_app application = create_app(os.getenv('NOTIFY_API_ENVIRONMENT') or 'development') application.app_context().push() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 26d5179a9..906ce664d 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,7 +1,6 @@ import pytest -from flask import jsonify -from app.models import (User, Service, Template, ApiKey, Job, VerifyCode, Notification) +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 save_model_service from app.dao.templates_dao import save_model_template @@ -10,6 +9,17 @@ from app.dao.jobs_dao import save_job from app.dao.notifications_dao import save_notification import uuid +@pytest.fixture(scope='function') +def service_factory(notify_db, notify_db_session): + class ServiceFactory(object): + def get(self, service_name): + 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) + return service + + return ServiceFactory() + @pytest.fixture(scope='function') def sample_user(notify_db, diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index dbe8be2b6..63f175b43 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1,16 +1,14 @@ -import moto import uuid +import app.celery.tasks from tests import create_authorization_header -from flask import url_for, json +from flask import json from app.models import Service +from app.dao.templates_dao import get_model_templates +from mock import ANY -def test_get_notifications( - notify_api, notify_db, notify_db_session, sample_api_key, sample_notification): - """ - Tests GET endpoint '/' to retrieve entire service list. - """ +def test_get_notification_by_id(notify_api, sample_notification): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header( @@ -30,217 +28,208 @@ def test_get_notifications( assert response.status_code == 200 -def test_get_notifications_empty_result( - notify_api, notify_db, notify_db_session, sample_api_key, mocker): +def test_get_notifications_empty_result(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: - id = uuid.uuid4() - + missing_notification_id = uuid.uuid4() auth_header = create_authorization_header( service_id=sample_api_key.service_id, - path=url_for('notifications.get_notifications', notification_id=id), + path='/notifications/{}'.format(missing_notification_id), method='GET') response = client.get( - url_for('notifications.get_notifications', notification_id=id), + path='/notifications/{}'.format(missing_notification_id), headers=[auth_header]) + notification = json.loads(response.get_data(as_text=True)) + assert notification['result'] == "error" + assert notification['message'] == "not found" assert response.status_code == 404 -def test_create_sms_should_reject_if_no_phone_numbers( - notify_api, notify_db, notify_db_session, sample_api_key, mocker): - """ - Tests GET endpoint '/' to retrieve entire service list. - """ +def test_create_sms_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: - data = { - 'template': "my message" - } + mocker.patch('app.celery.tasks.send_sms.apply_async') + + data = {} auth_header = create_authorization_header( service_id=sample_api_key.service_id, request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), + path='/notifications/sms', method='POST') response = client.post( - url_for('notifications.create_sms_notification'), + path='/notifications/sms', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 400 + app.celery.tasks.send_sms.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_bad_phone_numbers( - notify_api, notify_db, notify_db_session, mocker): - """ - Tests GET endpoint '/' to retrieve entire service list. - """ +def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + data = { 'to': 'invalid', - 'template': "my message" + 'template': sample_template.id } auth_header = create_authorization_header( request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), + path='/notifications/sms', method='POST') response = client.post( - url_for('notifications.create_sms_notification'), + path='/notifications/sms', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 400 + app.celery.tasks.send_sms.apply_async.assert_not_called() + assert json_resp['result'] == 'error' + assert len(json_resp['message'].keys()) == 1 assert 'Invalid phone number, must be of format +441234123123' in json_resp['message']['to'] + assert response.status_code == 400 -def test_send_notification_restrict_mobile(notify_api, - notify_db, - notify_db_session, - sample_api_key, - sample_template, - sample_user, - mocker): - """ - Test POST endpoint '/sms' with service notification with mobile number - not in restricted list. - """ +def test_send_notification_invalid_template_id(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + + data = { + 'to': '+441234123123', + 'template': 9999 + } + auth_header = create_authorization_header( + service_id=sample_template.service.id, + request_body=json.dumps(data), + path='/notifications/sms', + method='POST') + + response = client.post( + path='/notifications/sms', + 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_sms.apply_async.assert_not_called() + + assert response.status_code == 400 + assert len(json_resp['message'].keys()) == 1 + assert 'Template not found' in json_resp['message']['template'] + + +def test_prevents_sending_to_any_mobile_on_restricted_service(notify_api, sample_template, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + Service.query.filter_by( - id=sample_template.service.id).update({'restricted': True}) + id=sample_template.service.id + ).update( + {'restricted': True} + ) invalid_mob = '+449999999999' data = { 'to': invalid_mob, 'template': sample_template.id } - assert invalid_mob != sample_user.mobile_number + auth_header = create_authorization_header( service_id=sample_template.service.id, request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), + path='/notifications/sms', method='POST') response = client.post( - url_for('notifications.create_sms_notification'), + path='/notifications/sms', 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_sms.apply_async.assert_not_called() + assert response.status_code == 400 assert 'Invalid phone number for restricted service' in json_resp['message']['restricted'] -def test_send_notification_invalid_template_id(notify_api, - notify_db, - notify_db_session, - sample_api_key, - sample_template, - sample_user, - mocker): - """ - Tests POST endpoint '/sms' with notifications-admin notification with invalid template id - """ +def test_should_not_allow_template_from_another_service(notify_api, service_factory, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') - Service.query.filter_by( - id=sample_template.service.id).update({'restricted': True}) - invalid_mob = '+449999999999' - data = { - 'to': invalid_mob, - 'template': 9999 - } - assert invalid_mob != sample_user.mobile_number - auth_header = create_authorization_header( - service_id=sample_template.service.id, - request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), - method='POST') + service_1 = service_factory.get('service 1') + service_2 = service_factory.get('service 2') - response = client.post( - url_for('notifications.create_sms_notification'), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) + service_2_templates = get_model_templates(service_id=service_2.id) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 400 - assert 'Template not found' in json_resp['message']['template'] - - -@moto.mock_sqs -def test_should_not_allow_template_from_other_service(notify_api, - notify_db, - notify_db_session, - sample_template, - sample_admin_service_id, - mocker): - """ - Tests POST endpoint '/sms' with notifications. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: data = { 'to': '+441234123123', - 'template': sample_template.id + 'template': service_2_templates[0].id } + auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=service_1.id, request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), + path='/notifications/sms', method='POST') response = client.post( - url_for('notifications.create_sms_notification'), + path='/notifications/sms', 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_sms.apply_async.assert_not_called() + assert response.status_code == 400 assert 'Invalid template' in json_resp['message']['restricted'] -@moto.mock_sqs -def test_should_allow_valid_message(notify_api, - notify_db, - notify_db_session, - sqs_client_conn, - sample_user, - sample_template, - mocker): - """ - Tests POST endpoint '/sms' with notifications-admin notification. - """ +def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + data = { 'to': '+441234123123', 'template': sample_template.id } + auth_header = create_authorization_header( request_body=json.dumps(data), - path=url_for('notifications.create_sms_notification'), - method='POST') + path='/notifications/sms', + method='POST', + service_id=sample_template.service_id + ) response = client.post( - url_for('notifications.create_sms_notification'), + path='/notifications/sms', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) + notification_id = json.loads(response.data)['notification_id'] + app.celery.tasks.send_sms.apply_async.assert_called_once_with( + (str(sample_template.service_id), + notification_id, + ANY, + notify_api.config['SECRET_KEY'], + notify_api.config['DANGEROUS_SALT']) + ) assert response.status_code == 201 - assert json.loads(response.data)['notification_id'] is not None + assert notification_id -@moto.mock_sqs def test_send_email_valid_data(notify_api, notify_db, notify_db_session, @@ -262,11 +251,11 @@ def test_send_email_valid_data(notify_api, } auth_header = create_authorization_header( request_body=json.dumps(data), - path=url_for('notifications.create_email_notification'), + path='/notifications/email', method='POST') response = client.post( - url_for('notifications.create_email_notification'), + path='/notifications/email', data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) @@ -274,7 +263,6 @@ def test_send_email_valid_data(notify_api, assert json.loads(response.data)['notification_id'] is not None -@moto.mock_sqs def test_valid_message_with_service_id(notify_api, notify_db, notify_db_session, @@ -286,7 +274,7 @@ def test_valid_message_with_service_id(notify_api, with notify_api.test_client() as client: job_id = uuid.uuid4() service_id = sample_template.service.id - url = url_for('notifications.create_sms_for_service', service_id=service_id) + url = '/notifications/sms/service/{}'.format(service_id) data = { 'to': '+441234123123', 'template': sample_template.id, @@ -306,7 +294,6 @@ def test_valid_message_with_service_id(notify_api, assert json.loads(response.data)['notification_id'] is not None -@moto.mock_sqs def test_message_with_incorrect_service_id_should_fail(notify_api, notify_db, notify_db_session, @@ -319,7 +306,7 @@ def test_message_with_incorrect_service_id_should_fail(notify_api, job_id = uuid.uuid4() invalid_service_id = uuid.uuid4() - url = url_for('notifications.create_sms_for_service', service_id=invalid_service_id) + url = '/notifications/sms/service/{}'.format(invalid_service_id) data = { 'to': '+441234123123',