diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 54fb5dd01..80ff0b815 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -35,13 +35,15 @@ def delete_model_service(service): db.session.commit() -def get_model_services(service_id=None, user_id=None): +def get_model_services(service_id=None, user_id=None, _raise=True): # TODO need better mapping from function params to sql query. if user_id and service_id: return Service.query.filter( Service.users.any(id=user_id)).filter_by(id=service_id).one() elif service_id: - return Service.query.filter_by(id=service_id).one() + result = Service.query.filter_by(id=service_id).one() if _raise else Service.query.filter_by( + id=service_id).first() + return result elif user_id: return Service.query.filter(Service.users.any(id=user_id)).all() return Service.query.all() diff --git a/app/models.py b/app/models.py index 40acb7b66..e081653cf 100644 --- a/app/models.py +++ b/app/models.py @@ -86,6 +86,7 @@ class Service(db.Model): restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) queue_name = db.Column(UUID(as_uuid=True)) + class ApiKey(db.Model): __tablename__ = 'api_key' diff --git a/app/notifications/rest.py b/app/notifications/rest.py index d8da3202a..bddd13532 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -1,10 +1,13 @@ +import json + +import boto3 from flask import ( Blueprint, jsonify, request, current_app ) - +from itsdangerous import URLSafeSerializer from app import notify_alpha_client from app import api_user from app.dao import (templates_dao, services_dao) @@ -20,23 +23,54 @@ def get_notifications(notification_id): return jsonify(notify_alpha_client.fetch_notification_by_id(notification_id)), 200 +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.queue_name)) + import uuid + notification = json.dumps({'message_id': str(uuid.uuid4()), + 'service_id': 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'}, + 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, + 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) + + @notifications.route('/sms', methods=['POST']) def create_sms_notification(): notification = request.get_json()['notification'] errors = {} - to, to_errors = validate_to(notification, api_user['client']) - print("create sms") - print(notification) - template, template_errors = validate_template(notification, api_user['client']) - if to_errors['to']: - errors.update(to_errors) - if template_errors['template']: - errors.update(template_errors) - if errors: - return jsonify(result="error", message=errors), 400 - return jsonify(notify_alpha_client.send_sms( - mobile_number=to, - message=template)), 200 + # 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'): + to, to_errors = validate_to_for_admin_client(notification) + content, content_errors = validate_content_for_admin_client(notification) + if to_errors['to']: + errors.update(to_errors) + if content_errors['content']: + errors.update(content_errors) + if errors: + return jsonify(result="error", message=errors), 400 + + return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=content)), 200 + + else: + to, to_errors = validate_to(notification, api_user['client']) + template, template_errors = validate_template(notification, api_user['client']) + if to_errors['to']: + errors.update(to_errors) + if template_errors['template']: + errors.update(template_errors) + if errors: + return jsonify(result="error", message=errors), 400 + + # 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 @notifications.route('/email', methods=['POST']) @@ -66,36 +100,52 @@ def validate_to(json_body, service_id): else: if not mobile_regex.match(mob): errors['to'].append('invalid phone number, must be of format +441234123123') - if service_id != current_app.config.get('ADMIN_CLIENT_USER_NAME'): - 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['to'].append('Invalid phone number for restricted service') + + 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['to'].append('Invalid phone number for restricted service') + return mob, errors + + +def validate_to_for_admin_client(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) - content = '' + template = '' if not template_id: errors['template'].append('Required data missing') else: - if service_id == current_app.config.get('ADMIN_CLIENT_USER_NAME'): - content = json_body['template'] - else: - try: - template = templates_dao.get_model_templates( - template_id=json_body['template'], - service_id=service_id) - content = template.content - except: - errors['template'].append("Unable to load template.") + 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 diff --git a/app/schemas.py b/app/schemas.py index a6e6c1377..d6ab588ba 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -20,7 +20,7 @@ class UserSchema(ma.ModelSchema): class ServiceSchema(ma.ModelSchema): class Meta: model = models.Service - exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs") + exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs", "queue_name") class TemplateSchema(ma.ModelSchema): diff --git a/config.py b/config.py index b26a526a8..1dd256571 100644 --- a/config.py +++ b/config.py @@ -14,6 +14,8 @@ class Config(object): ADMIN_CLIENT_USER_NAME = None ADMIN_CLIENT_SECRET = None + AWS_REGION = 'eu-west-1' + class Development(Config): DEBUG = True diff --git a/migrations/versions/0010_add_queue_name_to_service.py b/migrations/versions/0010_add_queue_name_to_service.py index 774b4d3ba..636847cd3 100644 --- a/migrations/versions/0010_add_queue_name_to_service.py +++ b/migrations/versions/0010_add_queue_name_to_service.py @@ -18,6 +18,8 @@ from sqlalchemy.dialects import postgresql def upgrade(): ### commands auto generated by Alembic - please adjust! ### op.add_column('services', sa.Column('queue_name', postgresql.UUID(as_uuid=True), nullable=True)) + op.get_bind() + op.execute('update services set queue_name = (SELECT uuid_in(md5(random()::text || now()::text)::cstring))') ### end Alembic commands ### def downgrade(): diff --git a/requirements.txt b/requirements.txt index b85bac9a7..96d3663b6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,7 @@ flask-marshmallow==0.6.2 itsdangerous==0.24 Flask-Bcrypt==0.6.2 credstash==1.8.0 +boto3==1.2.3 git+https://github.com/alphagov/notifications-python-client.git@0.2.1#egg=notifications-python-client==0.2.1 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 263c2f4da..60ceea35b 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -3,4 +3,5 @@ pep8==1.5.7 pytest==2.8.1 pytest-mock==0.8.1 pytest-cov==2.2.0 -mock==1.0.1 \ No newline at end of file +mock==1.0.1 +moto==0.4.19 \ No newline at end of file diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 9241873ca..8ab965606 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -78,7 +78,8 @@ def sample_service(notify_db, 'users': [user], 'limit': 1000, 'active': False, - 'restricted': False} + 'restricted': False, + 'queue_name': str(uuid.uuid4())} service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) @@ -158,7 +159,7 @@ def sample_admin_service_id(notify_db, notify_db_session): @pytest.fixture(scope='function') def mock_notify_client_send_sms(mocker): def _send(mobile_number, message): - return jsonify('sms sent'), 200 + pass mock_class = mocker.patch('app.notify_alpha_client.send_sms', side_effect=_send) return mock_class diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 8e3a4d11b..c8c13d5db 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1,3 +1,6 @@ +import boto3 +import moto + from tests import create_authorization_header from flask import url_for, json from app import notify_alpha_client @@ -70,6 +73,7 @@ def test_get_notifications_empty_result( notify_alpha_client.fetch_notification_by_id.assert_called_with("123") +@moto.mock_sqs def test_should_reject_if_no_phone_numbers( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -77,6 +81,7 @@ def test_should_reject_if_no_phone_numbers( """ 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='success' @@ -104,6 +109,7 @@ def test_should_reject_if_no_phone_numbers( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_should_reject_bad_phone_numbers( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -111,6 +117,7 @@ def test_should_reject_bad_phone_numbers( """ 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='success' @@ -139,6 +146,7 @@ def test_should_reject_bad_phone_numbers( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_should_reject_missing_template( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -146,6 +154,7 @@ def test_should_reject_missing_template( """ 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='success' @@ -173,6 +182,7 @@ def test_should_reject_missing_template( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_send_template_content(notify_api, notify_db, notify_db_session, @@ -185,6 +195,7 @@ def test_send_template_content(notify_api, """ 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={ @@ -224,6 +235,7 @@ def test_send_template_content(notify_api, message=sample_template.content) +@moto.mock_sqs def test_send_notification_restrict_mobile(notify_api, notify_db, notify_db_session, @@ -237,6 +249,7 @@ def test_send_notification_restrict_mobile(notify_api, """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() Service.query.filter_by( id=sample_template.service.id).update({'restricted': True}) invalid_mob = '+449999999999' @@ -267,13 +280,15 @@ def test_send_notification_restrict_mobile(notify_api, assert 'Invalid phone number for restricted service' in json_resp['message']['to'] +@moto.mock_sqs def test_should_allow_valid_message( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_service, 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={ @@ -307,7 +322,7 @@ def test_should_allow_valid_message( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert json_resp['notification']['id'] == 100 - notify_alpha_client.send_sms.assert_called_with(mobile_number='+441234123123', message='valid') + notify_alpha_client.send_sms.assert_called_with(mobile_number='+441234123123', message="valid") def test_send_email_valid_data(notify_api, @@ -318,7 +333,6 @@ def test_send_email_valid_data(notify_api, 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" @@ -362,3 +376,16 @@ 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) + + +@moto.mock_sqs +def test_add_notification_to_queue(notify_api, notify_db, notify_db_session, sample_service): + set_up_mock_queue() + from app.notifications.rest import _add_notification_to_queue + _add_notification_to_queue('some message', sample_service, 'sms', '+447515349060') + + +def set_up_mock_queue(): + # set up mock queue + boto3.setup_default_session(region_name='eu-west-1') + conn = boto3.resource('sqs')