From aba1cd2ed557c10c934ec1952ed0f11bf3af9081 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 3 Feb 2016 13:16:19 +0000 Subject: [PATCH] Sqs queues now populated from all create_notification api calls. Marshmallow schemas added for notification. --- app/aws_sqs.py | 18 ++++ app/dao/templates_dao.py | 1 - app/notifications/rest.py | 134 +++++---------------------- app/schemas.py | 75 ++++++++++++++- tests/app/notifications/test_rest.py | 108 +++++++++++++-------- tests/app/user/test_rest_verify.py | 2 - tests/conftest.py | 7 ++ 7 files changed, 184 insertions(+), 161 deletions(-) create mode 100644 app/aws_sqs.py diff --git a/app/aws_sqs.py b/app/aws_sqs.py new file mode 100644 index 000000000..3d6317b2a --- /dev/null +++ b/app/aws_sqs.py @@ -0,0 +1,18 @@ +import uuid +import boto3 +from itsdangerous import URLSafeSerializer +from flask import current_app + + +def add_notification_to_queue(service_id, template_id, type_, notification): + q = boto3.resource( + 'sqs', region_name=current_app.config['AWS_REGION'] + ).create_queue(QueueName=str(service_id)) + message_id = str(uuid.uuid4()) + serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) + encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) + q.send_message(MessageBody=encrypted, + MessageAttributes={'type': {'StringValue': type_, 'DataType': 'String'}, + 'message_id': {'StringValue': message_id, 'DataType': 'String'}, + 'service_id': {'StringValue': str(service_id), 'DataType': 'String'}, + 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 42ef387fb..d6557f29d 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -23,7 +23,6 @@ def delete_model_template(template): def get_model_templates(template_id=None, service_id=None): - temp = Template.query.first() # TODO need better mapping from function params to sql query. if template_id and service_id: return Template.query.filter_by( diff --git a/app/notifications/rest.py b/app/notifications/rest.py index a130ade45..59dc1eaa1 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -7,10 +7,11 @@ from flask import ( request, current_app ) -from itsdangerous import URLSafeSerializer -from app import notify_alpha_client -from app import api_user +from app import (notify_alpha_client, api_user) +from app.aws_sqs import add_notification_to_queue from app.dao import (templates_dao, services_dao) +from app.schemas import ( + email_notification_schema, sms_admin_notification_schema, sms_template_notification_schema) import re mobile_regex = re.compile("^\\+44[\\d]{10}$") @@ -25,129 +26,38 @@ def get_notifications(notification_id): @notifications.route('/sms', methods=['POST']) def create_sms_notification(): - notification = request.get_json()['notification'] - errors = {} - to, to_errors = validate_to(notification) - if to_errors['to']: - errors.update(to_errors) + resp_json = request.get_json() # TODO: should create a different endpoint for the admin client to send verify codes. if api_user['client'] == current_app.config.get('ADMIN_CLIENT_USER_NAME'): - content, content_errors = validate_content_for_admin_client(notification) - if content_errors['content']: - errors.update(content_errors) + notification, errors = sms_admin_notification_schema.load(resp_json) if errors: return jsonify(result="error", message=errors), 400 - - return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=content)), 200 - + template_id = 'admin' + message = notification['content'] else: - to, restricted_errors = validate_to_for_service(to, api_user['client']) - if restricted_errors['restricted']: - errors.update(restricted_errors) - - template, template_errors = validate_template(notification, api_user['client']) - if template_errors['template']: - errors.update(template_errors) + notification, errors = sms_template_notification_schema.load(resp_json) if errors: return jsonify(result="error", message=errors), 400 + template_id = notification['template'] + message = notification['template'] - # add notification to the queue - service = services_dao.get_model_services(api_user['client'], _raise=False) - _add_notification_to_queue(template.id, service, 'sms', to) - return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=template.content)), 200 + add_notification_to_queue(api_user['client'], template_id, 'sms', notification) + return jsonify(notify_alpha_client.send_sms( + mobile_number=notification['to'], message=message)), 200 @notifications.route('/email', methods=['POST']) def create_email_notification(): - notification = request.get_json()['notification'] - errors = {} - for k in ['to', 'from', 'subject', 'message']: - k_error = validate_required_and_something(notification, k) - if k_error: - errors.update(k_error) - + resp_json = request.get_json() + notification, errors = email_notification_schema.load(resp_json) if errors: return jsonify(result="error", message=errors), 400 - + # At the moment we haven't hooked up + # template handling for sending email notifications. + add_notification_to_queue(api_user['client'], "admin", 'email', notification) return jsonify(notify_alpha_client.send_email( - notification['to'], - notification['message'], - notification['from'], + notification['to_address'], + notification['body'], + notification['from_address'], notification['subject'])) - - -def validate_to_for_service(mob, service_id): - errors = {"restricted": []} - service = services_dao.get_model_services(service_id=service_id) - if service.restricted: - valid = False - for usr in service.users: - if mob == usr.mobile_number: - valid = True - break - if not valid: - errors['restricted'].append('Invalid phone number for restricted service') - return mob, errors - - -def validate_to(json_body): - errors = {"to": []} - mob = json_body.get('to', None) - if not mob: - errors['to'].append('Required data missing') - else: - if not mobile_regex.match(mob): - errors['to'].append('invalid phone number, must be of format +441234123123') - return mob, errors - - -def validate_template(json_body, service_id): - errors = {"template": []} - template_id = json_body.get('template', None) - template = '' - if not template_id: - errors['template'].append('Required data missing') - else: - try: - template = templates_dao.get_model_templates( - template_id=json_body['template'], - service_id=service_id) - except: - errors['template'].append("Unable to load template.") - return template, errors - - -def validate_content_for_admin_client(json_body): - errors = {"content": []} - content = json_body.get('template', None) - if not content: - errors['content'].append('Required content') - - return content, errors - - -def validate_required_and_something(json_body, field): - errors = [] - if field not in json_body and json_body[field]: - errors.append('Required data for field.') - return {field: errors} if errors else None - - -def _add_notification_to_queue(template_id, service, msg_type, to): - q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( - QueueName=str(service.id)) - import uuid - message_id = str(uuid.uuid4()) - notification = json.dumps({'message_id': message_id, - 'service_id': str(service.id), - 'to': to, - 'message_type': msg_type, - 'template_id': template_id}) - serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) - q.send_message(MessageBody=encrypted, - MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, - 'message_id': {'StringValue': message_id, 'DataType': 'String'}, - 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, - 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) diff --git a/app/schemas.py b/app/schemas.py index 4f2c3e3b8..f494c38f9 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -1,7 +1,11 @@ +import re from flask_marshmallow.fields import fields from . import ma from . import models -from marshmallow import post_load, ValidationError +from marshmallow_sqlalchemy.fields import Related +from marshmallow import (post_load, ValidationError, validates, validates_schema) + +mobile_regex = re.compile("^\\+44[\\d]{10}$") # TODO I think marshmallow provides a better integration and error handling. @@ -59,12 +63,70 @@ class JobSchema(BaseSchema): class RequestVerifyCodeSchema(ma.Schema): - def verify_code_type(self): - if self not in models.VERIFY_CODE_TYPES: + + code_type = fields.Str(required=True) + to = fields.Str(required=False) + + @validates('code_type') + def validate_code_type(self, code): + if code not in models.VERIFY_CODE_TYPES: raise ValidationError('Invalid code type') - code_type = fields.Str(required=True, validate=verify_code_type) - 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 + + +class SmsNotificationSchema(NotificationSchema): + to = fields.Str(required=True) + + @validates('to') + def validate_to(self, value): + if not mobile_regex.match(value): + raise ValidationError('Invalid phone number, must be of format +441234123123') + + +class SmsTemplateNotificationSchema(SmsNotificationSchema): + template = fields.Int(required=True) + + @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): + """ + Validate the to field is valid for this template + """ + template_id = data.get('template', None) + template = models.Template.query.filter_by(id=template_id).first() + if template: + service = template.service + 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') + + +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) user_schema = UserSchema() @@ -83,3 +145,6 @@ job_schema = JobSchema() job_schema_load_json = JobSchema(load_json=True) jobs_schema = JobSchema(many=True) request_verify_code_schema = RequestVerifyCodeSchema() +sms_admin_notification_schema = SmsAdminNotificationSchema() +sms_template_notification_schema = SmsTemplateNotificationSchema() +email_notification_schema = EmailNotificationSchema() diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index e940a8e6e..27d598f37 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1,4 +1,3 @@ -import boto3 import moto from tests import create_authorization_header @@ -85,9 +84,7 @@ def test_create_sms_should_reject_if_no_phone_numbers( return_value='success' ) data = { - 'notification': { - 'template': "my message" - } + 'template': "my message" } auth_header = create_authorization_header( service_id=sample_api_key.service_id, @@ -103,7 +100,7 @@ def test_create_sms_should_reject_if_no_phone_numbers( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 assert json_resp['result'] == 'error' - assert 'Required data missing' in json_resp['message']['to'][0] + assert 'Missing data for required field.' in json_resp['message']['to'][0] assert not notify_alpha_client.send_sms.called @@ -119,10 +116,8 @@ def test_should_reject_bad_phone_numbers( return_value='success' ) data = { - 'notification': { - 'to': 'invalid', - 'template': "my message" - } + 'to': 'invalid', + 'template': "my message" } auth_header = create_authorization_header( request_body=json.dumps(data), @@ -137,7 +132,7 @@ def test_should_reject_bad_phone_numbers( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 assert json_resp['result'] == 'error' - assert 'invalid phone number, must be of format +441234123123' in json_resp['message']['to'] + assert 'Invalid phone number, must be of format +441234123123' in json_resp['message']['to'] assert not notify_alpha_client.send_sms.called @@ -153,9 +148,7 @@ def test_should_reject_missing_content( return_value='success' ) data = { - 'notification': { - 'to': '+441234123123' - } + 'to': '+441234123123' } auth_header = create_authorization_header( request_body=json.dumps(data), @@ -170,7 +163,7 @@ def test_should_reject_missing_content( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 assert json_resp['result'] == 'error' - assert 'Required content' in json_resp['message']['content'] + assert 'Missing data for required field.' in json_resp['message']['content'] assert not notify_alpha_client.send_sms.called @@ -178,13 +171,13 @@ def test_should_reject_missing_content( def test_send_template_content(notify_api, notify_db, notify_db_session, + sqs_client_conn, mocker): """ Test POST endpoint '/sms' with service notification. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mobile = '+447719087678' msg = 'Message content' mocker.patch( @@ -202,10 +195,8 @@ def test_send_template_content(notify_api, } ) data = { - 'notification': { - 'to': mobile, - 'template': msg - } + 'to': mobile, + 'content': msg } auth_header = create_authorization_header( request_body=json.dumps(data), @@ -247,10 +238,8 @@ def test_send_notification_restrict_mobile(notify_api, return_value={} ) data = { - 'notification': { - 'to': invalid_mob, - 'template': sample_template.id - } + 'to': invalid_mob, + 'template': sample_template.id } assert invalid_mob != sample_user.mobile_number auth_header = create_authorization_header( @@ -270,14 +259,59 @@ def test_send_notification_restrict_mobile(notify_api, assert not notify_alpha_client.send_sms.called +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 + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + Service.query.filter_by( + id=sample_template.service.id).update({'restricted': True}) + invalid_mob = '+449999999999' + mocker.patch( + 'app.notify_alpha_client.send_sms', + return_value={} + ) + 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') + + response = client.post( + url_for('notifications.create_sms_notification'), + 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 + assert 'Template not found' in json_resp['message']['template'] + assert not notify_alpha_client.send_sms.called + + @moto.mock_sqs -def test_should_allow_valid_message(notify_api, notify_db, notify_db_session, mocker): +def test_should_allow_valid_message(notify_api, + notify_db, + notify_db_session, + sqs_client_conn, + mocker): """ Tests POST endpoint '/sms' with notifications-admin notification. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value={ @@ -293,10 +327,8 @@ def test_should_allow_valid_message(notify_api, notify_db, notify_db_session, mo } ) data = { - 'notification': { - 'to': '+441234123123', - 'template': 'valid' - } + 'to': '+441234123123', + 'content': 'valid' } auth_header = create_authorization_header( request_body=json.dumps(data), @@ -314,11 +346,13 @@ def test_should_allow_valid_message(notify_api, notify_db, notify_db_session, mo notify_alpha_client.send_sms.assert_called_with(mobile_number='+441234123123', message="valid") +@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): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -343,12 +377,10 @@ def test_send_email_valid_data(notify_api, } ) data = { - 'notification': { - 'to': to_address, - 'from': from_address, - 'subject': subject, - 'message': message - } + 'to': to_address, + 'from': from_address, + 'subject': subject, + 'message': message } auth_header = create_authorization_header( request_body=json.dumps(data), @@ -365,9 +397,3 @@ def test_send_email_valid_data(notify_api, assert json_resp['notification']['id'] == 100 notify_alpha_client.send_email.assert_called_with( to_address, message, from_address, subject) - - -def set_up_mock_queue(): - # set up mock queue - boto3.setup_default_session(region_name='eu-west-1') - conn = boto3.resource('sqs') diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index a2228d44b..8d325d191 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -351,11 +351,9 @@ def test_send_user_code_for_email_uses_optional_to_field(notify_api, def test_request_verify_code_schema_invalid_code_type(notify_api, notify_db, notify_db_session, sample_user): - import json from app.schemas import request_verify_code_schema data = json.dumps({'code_type': 'not_sms'}) code, error = request_verify_code_schema.loads(data) - assert code == {} assert error == {'code_type': ['Invalid code type']} diff --git a/tests/conftest.py b/tests/conftest.py index 9ca193f57..37ee9bbd3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ import pytest import mock import os +import boto3 from config import configs from alembic.command import upgrade from alembic.config import Config @@ -70,3 +71,9 @@ def os_environ(request): request.addfinalizer(env_patch.stop) return env_patch.start() + + +@pytest.fixture(scope='function') +def sqs_client_conn(request): + boto3.setup_default_session(region_name='eu-west-1') + return boto3.resource('sqs')