From 2caea40212204de7e10103be8381b5038a910e7c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Mar 2016 09:32:43 +0000 Subject: [PATCH 01/23] Pass the utcnow function, rather than the result of executing the function. --- app/models.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models.py b/app/models.py index 0f28d2452..d437a5427 100644 --- a/app/models.py +++ b/app/models.py @@ -144,13 +144,13 @@ class Template(db.Model): index=False, unique=False, nullable=False, - default=datetime.datetime.now) + default=datetime.datetime.utcnow) updated_at = db.Column( db.DateTime, index=False, unique=False, nullable=True, - onupdate=datetime.datetime.utcnow()) + onupdate=datetime.datetime.utcnow) content = db.Column(db.Text, index=False, unique=False, nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False) service = db.relationship('Service', backref=db.backref('templates', lazy='dynamic')) @@ -176,13 +176,13 @@ class Job(db.Model): index=False, unique=False, nullable=False, - default=datetime.datetime.utcnow()) + default=datetime.datetime.utcnow) updated_at = db.Column( db.DateTime, index=False, unique=False, nullable=True, - onupdate=datetime.datetime.utcnow()) + onupdate=datetime.datetime.utcnow) status = db.Column(db.Enum(*JOB_STATUS_TYPES, name='job_status_types'), nullable=False, default='pending') notification_count = db.Column(db.Integer, nullable=False) notifications_sent = db.Column(db.Integer, nullable=False, default=0) @@ -217,7 +217,7 @@ class VerifyCode(db.Model): index=False, unique=False, nullable=False, - default=datetime.datetime.utcnow()) + default=datetime.datetime.utcnow) @property def code(self): @@ -262,7 +262,7 @@ class Notification(db.Model): index=False, unique=False, nullable=True, - onupdate=datetime.datetime.utcnow()) + onupdate=datetime.datetime.utcnow) status = db.Column( db.Enum(*NOTIFICATION_STATUS_TYPES, name='notification_status_types'), nullable=False, default='sent') reference = db.Column(db.String, nullable=True, index=True) @@ -286,7 +286,7 @@ class InvitedUser(db.Model): index=False, unique=False, nullable=False, - default=datetime.datetime.utcnow()) + default=datetime.datetime.utcnow) status = db.Column( db.Enum(*INVITED_USER_STATUS_TYPES, name='invited_users_status_types'), nullable=False, default='pending') permissions = db.Column(db.String, nullable=False) @@ -338,7 +338,7 @@ class Permission(db.Model): index=False, unique=False, nullable=False, - default=datetime.datetime.utcnow()) + default=datetime.datetime.utcnow) __table_args__ = ( UniqueConstraint('service_id', 'user_id', 'permission', name='uix_service_user_permission'), From 356083e8ac10713e43ee421da9a8f52eae5b465f Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 15 Mar 2016 14:24:10 +0000 Subject: [PATCH 02/23] Update schemas to return more details about the job and template for notifications. --- app/schemas.py | 3 +++ tests/app/notifications/test_rest.py | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 4c5fe961a..932fd1990 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -171,6 +171,9 @@ class SmsAdminNotificationSchema(SmsNotificationSchema): class NotificationStatusSchema(BaseSchema): + template = fields.Nested(TemplateSchema, only=["id", "name"], dump_only=True) + job = fields.Nested(JobSchema, only=["id", "file_name"], dump_only=True) + class Meta: model = models.Notification diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 1d6428e5e..3abe3fc97 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -25,7 +25,9 @@ def test_get_notification_by_id(notify_api, sample_notification): notification = json.loads(response.get_data(as_text=True))['notification'] assert notification['status'] == 'sent' - assert notification['template'] == sample_notification.template.id + assert notification['template'] == { + 'id': sample_notification.template.id, + 'name': sample_notification.template.name} assert notification['to'] == '+447700900855' assert notification['service'] == str(sample_notification.service_id) assert response.status_code == 200 @@ -64,7 +66,9 @@ def test_get_all_notifications(notify_api, sample_notification): notifications = json.loads(response.get_data(as_text=True)) assert notifications['notifications'][0]['status'] == 'sent' - assert notifications['notifications'][0]['template'] == sample_notification.template.id + assert notifications['notifications'][0]['template'] == { + 'id': sample_notification.template.id, + 'name': sample_notification.template.name} assert notifications['notifications'][0]['to'] == '+447700900855' assert notifications['notifications'][0]['service'] == str(sample_notification.service_id) assert response.status_code == 200 From 001c875d02303e4b049f214ab72338e555b34a20 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 15 Mar 2016 14:51:40 +0000 Subject: [PATCH 03/23] Updated aws start/stop scripts to handle multiple start/stop scripts Primarily to handle the two celery processes --- scripts/aws_start_app.sh | 19 +++++++++++++++++-- scripts/aws_stop_app.sh | 32 ++++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/scripts/aws_start_app.sh b/scripts/aws_start_app.sh index 40bd7d50e..7be657a03 100755 --- a/scripts/aws_start_app.sh +++ b/scripts/aws_start_app.sh @@ -1,4 +1,19 @@ #!/bin/bash -echo "Starting application" -sudo service notifications-api start \ No newline at end of file +if [ -e "/etc/init/notifications-api.conf" ] +then + echo "Starting api" + sudo service notifications-api start +fi + +if [ -e "/etc/init/notifications-api-celery-worker.conf" ] +then + echo "Starting celery worker" + sudo service notifications-api-celery-worker start +fi + +if [ -e "/etc/init/notifications-api-celery-beat.conf" ] +then + echo "Starting celery beat" + sudo service notifications-api-celery-beat start +fi diff --git a/scripts/aws_stop_app.sh b/scripts/aws_stop_app.sh index 188467062..2f5660ae1 100755 --- a/scripts/aws_stop_app.sh +++ b/scripts/aws_stop_app.sh @@ -7,9 +7,29 @@ function error_exit exit 0 } -echo "Stopping application" -if sudo service notifications-api stop; then - exit 0 -else - error_exit "Could not stop application" -fi \ No newline at end of file +if [ -e "/etc/init/notifications-api.conf" ]; then + echo "stopping notifications-api" + if sudo service notifications-api stop; then + echo "notifications-api stopped" + else + error_exit "Could not stop notifications-api" + fi +fi + +if [ -e "/etc/init/notifications-api-celery-beat.conf" ]; then + echo "stopping notifications-api-celery-beat" + if sudo service notifications-api-celery-beat stop; then + echo "notifications-api stopped" + else + error_exit "Could not stop notifications-celery-beat" + fi +fi + +if [ -e "/etc/init/notifications-api-celery-worker.conf" ]; then + echo "stopping notifications-api-celery-worker" + if sudo service notifications-api-celery-worker stop; then + echo "notifications-api stopped" + else + error_exit "Could not stop notifications-celery-worker" + fi +fi From 16882961ad517d45c27349357ea4b2e3bb343741 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 15 Mar 2016 14:57:15 +0000 Subject: [PATCH 04/23] Added check for job in notifications json dump. --- tests/app/notifications/test_rest.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 3abe3fc97..da5cc1dc7 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -28,6 +28,10 @@ def test_get_notification_by_id(notify_api, sample_notification): assert notification['template'] == { 'id': sample_notification.template.id, 'name': sample_notification.template.name} + assert notification['job'] == { + 'id': str(sample_notification.job.id), + 'file_name': sample_notification.job.file_name + } assert notification['to'] == '+447700900855' assert notification['service'] == str(sample_notification.service_id) assert response.status_code == 200 @@ -69,6 +73,10 @@ def test_get_all_notifications(notify_api, sample_notification): assert notifications['notifications'][0]['template'] == { 'id': sample_notification.template.id, 'name': sample_notification.template.name} + assert notifications['notifications'][0]['job'] == { + 'id': str(sample_notification.job.id), + 'file_name': sample_notification.job.file_name + } assert notifications['notifications'][0]['to'] == '+447700900855' assert notifications['notifications'][0]['service'] == str(sample_notification.service_id) assert response.status_code == 200 From 35f4cca62dd66125aa708bed3d5e14956c1c66f4 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 16 Mar 2016 10:56:27 +0000 Subject: [PATCH 05/23] Add all deplpyment groups to travis --- .travis.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 29bfb975f..d4e4e1332 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,7 +46,17 @@ deploy: key: notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip bundle_type: zip application: notifications-api - deployment_group: notifications-api-celery + deployment_group: notifications_admin_api_deployment_group + region: eu-west-1 + on: *2 +- provider: codedeploy + access_key_id: AKIAJQPPNM6P6V53SWKA + secret_access_key: *1 + bucket: notifications-api-codedeploy + key: notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip + bundle_type: zip + application: notifications-api + deployment_group: notifications_delivery_api_deployment_group region: eu-west-1 on: *2 - provider: s3 From 27d48a97467d1d7bae7d753d5e521fe973956838 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 16 Mar 2016 13:25:09 +0000 Subject: [PATCH 06/23] Pass application name in from start scripts - allows logger to log as correct application --- app/__init__.py | 12 +++++++----- run_celery.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 3b92f9265..79a62e78a 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -27,10 +27,12 @@ encryption = Encryption() api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user) -def create_app(): +def create_app(app_name=None): application = Flask(__name__) application.config.from_object(os.environ['NOTIFY_API_ENVIRONMENT']) + if app_name: + application.config['NOTIFY_APP_NAME'] = app_name init_app(application) db.init_app(application) @@ -92,10 +94,10 @@ def init_app(app): def email_safe(string): return "".join([ - character.lower() - if character.isalnum() or character == "." - else "" for character in re.sub("\s+", ".", string.strip()) - ]) + character.lower() + if character.isalnum() or character == "." + else "" for character in re.sub("\s+", ".", string.strip()) + ]) def create_uuid(): diff --git a/run_celery.py b/run_celery.py index 51ce92910..066735e63 100644 --- a/run_celery.py +++ b/run_celery.py @@ -1,5 +1,5 @@ #!/usr/bin/env python from app import notify_celery, create_app -application = create_app() +application = create_app('delivery') application.app_context().push() From 4268f8453be6961a625369d0b6328173f39f63fc Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 16 Mar 2016 13:33:12 +0000 Subject: [PATCH 07/23] Use the same validation in the endpoint and the task to validate the phone number is ok to send to. Format the phone number before sending it to the sms provider. --- app/celery/tasks.py | 23 ++++-------- app/notifications/rest.py | 5 +-- app/validation.py | 15 ++++++++ tests/app/celery/test_tasks.py | 40 ++++++++++++--------- tests/app/test_validation.py | 65 ++++++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 36 deletions(-) create mode 100644 app/validation.py create mode 100644 tests/app/test_validation.py diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ff952d6e2..94fb59bc4 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -25,7 +25,8 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from datetime import datetime from utils.template import Template -from utils.recipients import RecipientCSV, validate_phone_number, format_phone_number +from utils.recipients import RecipientCSV, format_phone_number, validate_phone_number +from app.validation import (allowed_send_to_email, allowed_send_to_number) @notify_celery.task(name="delete-verify-codes") @@ -204,7 +205,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): ) client.send_sms( - to=notification['to'], + to=format_phone_number(validate_phone_number(notification['to'])), content=template.replaced, reference=str(notification_id) ) @@ -223,20 +224,6 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): current_app.logger.debug(e) -def allowed_send_to_number(service, to): - if service.restricted and format_phone_number(validate_phone_number(to)) not in [ - format_phone_number(validate_phone_number(user.mobile_number)) for user in service.users - ]: - return False - return True - - -def allowed_send_to_email(service, to): - if service.restricted and to not in [user.email_address for user in service.users]: - return False - return True - - @notify_celery.task(name="send-email") def send_email(service_id, notification_id, subject, from_address, encrypted_notification, created_at): notification = encryption.decrypt(encrypted_notification) @@ -300,7 +287,9 @@ def send_sms_code(encrypted_verification): verification_message = encryption.decrypt(encrypted_verification) try: firetext_client.send_sms( - verification_message['to'], verification_message['secret_code'], 'send-sms-code' + format_phone_number(validate_phone_number(verification_message['to'])), + verification_message['secret_code'], + 'send-sms-code' ) except FiretextClientException as e: current_app.logger.exception(e) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 594ba5613..03869cfd3 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -26,6 +26,7 @@ from app.schemas import ( notification_status_schema ) from app.celery.tasks import send_sms, send_email +from app.validation import allowed_send_to_number, allowed_send_to_email notifications = Blueprint('notifications', __name__) @@ -320,7 +321,7 @@ def send_notification(notification_type): notification_id = create_uuid() if notification_type == 'sms': - if service.restricted and notification['to'] not in [user.mobile_number for user in service.users]: + if not allowed_send_to_number(service, notification['to']): return jsonify( result="error", message={'to': ['Invalid phone number for restricted service']}), 400 send_sms.apply_async(( @@ -330,7 +331,7 @@ def send_notification(notification_type): datetime.utcnow().strftime(DATETIME_FORMAT) ), queue='sms') else: - if service.restricted and notification['to'] not in [user.email_address for user in service.users]: + if not allowed_send_to_email(service, notification['to']): return jsonify( result="error", message={'to': ['Email address not permitted for restricted service']}), 400 send_email.apply_async(( diff --git a/app/validation.py b/app/validation.py new file mode 100644 index 000000000..49a6b0664 --- /dev/null +++ b/app/validation.py @@ -0,0 +1,15 @@ +from utils.recipients import format_phone_number, validate_phone_number + + +def allowed_send_to_number(service, to): + if service.restricted and format_phone_number(validate_phone_number(to)) not in [ + format_phone_number(validate_phone_number(user.mobile_number)) for user in service.users + ]: + return False + return True + + +def allowed_send_to_email(service, to): + if service.restricted and to not in [user.email_address for user in service.users]: + return False + return True diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index e2f334697..737f08864 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,6 +1,8 @@ import uuid import pytest from flask import current_app +from utils.recipients import validate_phone_number, format_phone_number + from app.celery.tasks import ( send_sms, send_sms_code, @@ -248,7 +250,7 @@ def test_should_process_all_sms_job(sample_job, sample_job_with_placeholdered_te def test_should_send_template_to_correct_sms_provider_and_persist(sample_template_with_placeholders, mocker): notification = { "template": sample_template_with_placeholders.id, - "to": "+441234123123", + "to": "+447234123123", "personalisation": {"name": "Jo"} } mocker.patch('app.encryption.decrypt', return_value=notification) @@ -265,7 +267,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat ) firetext_client.send_sms.assert_called_once_with( - to="+441234123123", + to=format_phone_number(validate_phone_number("+447234123123")), content="Sample service: Hello Jo", reference=str(notification_id) ) @@ -273,7 +275,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat sample_template_with_placeholders.service_id, notification_id ) assert persisted_notification.id == notification_id - assert persisted_notification.to == '+441234123123' + assert persisted_notification.to == '+447234123123' assert persisted_notification.template_id == sample_template_with_placeholders.id assert persisted_notification.status == 'sent' assert persisted_notification.created_at == now @@ -285,7 +287,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat def test_should_send_sms_without_personalisation(sample_template, mocker): notification = { "template": sample_template.id, - "to": "+441234123123" + "to": "+447234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.firetext_client.send_sms') @@ -301,7 +303,7 @@ def test_should_send_sms_without_personalisation(sample_template, mocker): ) firetext_client.send_sms.assert_called_once_with( - to="+441234123123", + to=format_phone_number(validate_phone_number("+447234123123")), content="Sample service: This is a template", reference=str(notification_id) ) @@ -330,7 +332,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif ) firetext_client.send_sms.assert_called_once_with( - to="+447700900890", + to=format_phone_number(validate_phone_number("+447700900890")), content="Sample service: This is a template", reference=str(notification_id) ) @@ -396,7 +398,7 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa notification = { "template": sample_job.template.id, "job": sample_job.id, - "to": "+441234123123" + "to": "+447234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.firetext_client.send_sms') @@ -411,13 +413,13 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa now.strftime(DATETIME_FORMAT) ) firetext_client.send_sms.assert_called_once_with( - to="+441234123123", + to=format_phone_number(validate_phone_number("+447234123123")), content="Sample service: This is a template", reference=str(notification_id) ) persisted_notification = notifications_dao.get_notification(sample_job.template.service_id, notification_id) assert persisted_notification.id == notification_id - assert persisted_notification.to == '+441234123123' + assert persisted_notification.to == '+447234123123' assert persisted_notification.job_id == sample_job.id assert persisted_notification.template_id == sample_job.template.id assert persisted_notification.status == 'sent' @@ -520,7 +522,7 @@ def test_should_use_email_template_and_persist_without_personalisation( def test_should_persist_notification_as_failed_if_sms_client_fails(sample_template, mocker): notification = { "template": sample_template.id, - "to": "+441234123123" + "to": "+447234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error())) @@ -536,13 +538,13 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa now.strftime(DATETIME_FORMAT) ) firetext_client.send_sms.assert_called_once_with( - to="+441234123123", + to=format_phone_number(validate_phone_number("+447234123123")), content="Sample service: This is a template", reference=str(notification_id) ) persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id) assert persisted_notification.id == notification_id - assert persisted_notification.to == '+441234123123' + assert persisted_notification.to == '+447234123123' assert persisted_notification.template_id == sample_template.id assert persisted_notification.status == 'failed' assert persisted_notification.created_at == now @@ -590,7 +592,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker): notification = { "template": sample_template.id, - "to": "+441234123123" + "to": "+447234123123" } mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.firetext_client.send_sms') @@ -638,24 +640,28 @@ def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mo def test_should_send_sms_code(mocker): - notification = {'to': '+441234123123', + notification = {'to': '+447234123123', 'secret_code': '12345'} encrypted_notification = encryption.encrypt(notification) mocker.patch('app.firetext_client.send_sms') send_sms_code(encrypted_notification) - firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code') + firetext_client.send_sms.assert_called_once_with(format_phone_number(validate_phone_number(notification['to'])), + notification['secret_code'], + 'send-sms-code') def test_should_throw_firetext_client_exception(mocker): - notification = {'to': '+441234123123', + notification = {'to': '+447234123123', 'secret_code': '12345'} encrypted_notification = encryption.encrypt(notification) mocker.patch('app.firetext_client.send_sms', side_effect=FiretextClientException(firetext_error())) send_sms_code(encrypted_notification) - firetext_client.send_sms.assert_called_once_with(notification['to'], notification['secret_code'], 'send-sms-code') + firetext_client.send_sms.assert_called_once_with(format_phone_number(validate_phone_number(notification['to'])), + notification['secret_code'], + 'send-sms-code') def test_should_send_email_code(mocker): diff --git a/tests/app/test_validation.py b/tests/app/test_validation.py new file mode 100644 index 000000000..9c822533d --- /dev/null +++ b/tests/app/test_validation.py @@ -0,0 +1,65 @@ +from app.models import User, Service +from app.validation import allowed_send_to_number, allowed_send_to_email + + +def test_allowed_send_to_number_returns_true_for_restricted_service_with_same_number(): + mobile_number = '07524609792' + service = _create_service_data(mobile_number) + assert allowed_send_to_number(service, mobile_number) + + +def test_allowed_send_to_number_returns_false_for_restricted_service_with_different_number(): + mobile_number = '00447524609792' + service = _create_service_data(mobile_number) + assert not allowed_send_to_number(service, '+447344609793') + + +def test_allowed_send_to_number_returns_true_for_unrestricted_service_with_different_number(): + mobile_number = '+447524609792' + service = _create_service_data(mobile_number, False) + assert allowed_send_to_number(service, '+447344609793') + + +def test_allowed_send_to_email__returns_true_for_restricted_service_with_same_email(): + email = 'testing@it.gov.uk' + service = _create_service_data(email_address=email) + assert allowed_send_to_email(service, email) + + +def test_allowed_send_to_email__returns_false_for_restricted_service_with_different_email(): + email = 'testing@it.gov.uk' + service = _create_service_data(email_address=email) + assert not allowed_send_to_email(service, 'another@it.gov.uk') + + +def test_allowed_send_to_email__returns_false_for_restricted_service_with_different_email(): + email = 'testing@it.gov.uk' + service = _create_service_data(email_address=email) + assert not allowed_send_to_email(service, 'another@it.gov.uk') + + +def test_allowed_send_to_email__returns_true_for_unrestricted_service_with_different_email(): + email = 'testing@it.gov.uk' + service = _create_service_data(email_address=email, restricted=False) + assert allowed_send_to_number(service, 'another@it.gov.uk') + + +def _create_service_data(mobile_number='+447524609792', restricted=True, email_address='test_user@it.gov.uk'): + usr = { + 'name': 'Test User', + 'email_address': email_address, + 'password': 'password', + 'mobile_number': mobile_number, + 'state': 'active' + } + user = User(**usr) + data = { + 'name': 'Test service', + 'limit': 10, + 'active': False, + 'restricted': restricted, + 'email_from': 'test_service@it.gov.uk' + } + service = Service(**data) + service.users = [user] + return service From 6ceddf0ebf1890ad6660d3a77ced9ce867253dc3 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 16 Mar 2016 14:08:25 +0000 Subject: [PATCH 08/23] pep8 fixed --- app/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 79a62e78a..28508cb1f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -94,10 +94,8 @@ def init_app(app): def email_safe(string): return "".join([ - character.lower() - if character.isalnum() or character == "." - else "" for character in re.sub("\s+", ".", string.strip()) - ]) + character.lower() if character.isalnum() or character == "." else "" for character in re.sub("\s+", ".", string.strip()) # noqa + ]) def create_uuid(): From c3a15f9f3005da2b8558bb33516be9f02cbd1291 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 16 Mar 2016 16:47:18 +0000 Subject: [PATCH 09/23] Fix pagination bug and swapped file name with original file name. --- app/notifications/rest.py | 24 ++++++++++++++---------- app/schemas.py | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 594ba5613..1c59df0d8 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -193,13 +193,12 @@ def get_all_notifications(): return jsonify(result="error", message="Invalid page"), 400 all_notifications = notifications_dao.get_notifications_for_service(api_user['client'], page) - return jsonify( notifications=notification_status_schema.dump(all_notifications.items, many=True).data, links=pagination_links( all_notifications, '.get_all_notifications', - request.args + **request.args.to_dict() ) ), 200 @@ -213,13 +212,14 @@ def get_all_notifications_for_service(service_id): return jsonify(result="error", message="Invalid page"), 400 all_notifications = notifications_dao.get_notifications_for_service(service_id, page) - + kwargs = request.args.to_dict() + kwargs['service_id'] = service_id return jsonify( notifications=notification_status_schema.dump(all_notifications.items, many=True).data, links=pagination_links( all_notifications, '.get_all_notifications_for_service', - request.args + **kwargs ) ), 200 @@ -233,13 +233,15 @@ def get_all_notifications_for_service_job(service_id, job_id): return jsonify(result="error", message="Invalid page"), 400 all_notifications = notifications_dao.get_notifications_for_job(service_id, job_id, page) - + kwargs = request.args.to_dict() + kwargs['service_id'] = service_id + kwargs['job_id'] = job_id return jsonify( notifications=notification_status_schema.dump(all_notifications.items, many=True).data, links=pagination_links( all_notifications, '.get_all_notifications_for_service_job', - request.args + **kwargs ) ), 200 @@ -255,13 +257,15 @@ def get_page_from_request(): return 1 -def pagination_links(pagination, endpoint, args): +def pagination_links(pagination, endpoint, **kwargs): + if 'page' in kwargs: + kwargs.pop('page', None) links = dict() if pagination.has_prev: - links['prev'] = url_for(endpoint, **dict(list(args.items()) + [('page', pagination.prev_num)])) + links['prev'] = url_for(endpoint, page=pagination.prev_num, **kwargs) if pagination.has_next: - links['next'] = url_for(endpoint, **dict(list(args.items()) + [('page', pagination.next_num)])) - links['last'] = url_for(endpoint, **dict(list(args.items()) + [('page', pagination.pages)])) + links['next'] = url_for(endpoint, page=pagination.next_num, **kwargs) + links['last'] = url_for(endpoint, page=pagination.pages, **kwargs) return links diff --git a/app/schemas.py b/app/schemas.py index 932fd1990..575b23ea9 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -171,8 +171,8 @@ class SmsAdminNotificationSchema(SmsNotificationSchema): class NotificationStatusSchema(BaseSchema): - template = fields.Nested(TemplateSchema, only=["id", "name"], dump_only=True) - job = fields.Nested(JobSchema, only=["id", "file_name"], dump_only=True) + template = fields.Nested(TemplateSchema, only=["id", "name", "template_type"], dump_only=True) + job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) class Meta: model = models.Notification From f41d01780a55bb9d2cf793055e4dbd15067f1b94 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 16 Mar 2016 17:01:58 +0000 Subject: [PATCH 10/23] whoops test fixes. --- tests/app/notifications/test_rest.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index da5cc1dc7..9bc9ada62 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -27,10 +27,11 @@ def test_get_notification_by_id(notify_api, sample_notification): assert notification['status'] == 'sent' assert notification['template'] == { 'id': sample_notification.template.id, - 'name': sample_notification.template.name} + 'name': sample_notification.template.name, + 'template_type': sample_notification.template.template_type} assert notification['job'] == { 'id': str(sample_notification.job.id), - 'file_name': sample_notification.job.file_name + 'original_file_name': sample_notification.job.original_file_name } assert notification['to'] == '+447700900855' assert notification['service'] == str(sample_notification.service_id) @@ -72,10 +73,11 @@ def test_get_all_notifications(notify_api, sample_notification): assert notifications['notifications'][0]['status'] == 'sent' assert notifications['notifications'][0]['template'] == { 'id': sample_notification.template.id, - 'name': sample_notification.template.name} + 'name': sample_notification.template.name, + 'template_type': sample_notification.template.template_type} assert notifications['notifications'][0]['job'] == { 'id': str(sample_notification.job.id), - 'file_name': sample_notification.job.file_name + 'original_file_name': sample_notification.job.original_file_name } assert notifications['notifications'][0]['to'] == '+447700900855' assert notifications['notifications'][0]['service'] == str(sample_notification.service_id) From bd2d77fec8fd58d23789fe8aa8b2f9150a529360 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 17 Mar 2016 10:37:24 +0000 Subject: [PATCH 11/23] Add platform_admin boolean on the User data model. --- app/models.py | 5 +++- migrations/versions/0041_platform_admin.py | 33 ++++++++++++++++++++++ tests/app/dao/test_users_dao.py | 1 + 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0041_platform_admin.py diff --git a/app/models.py b/app/models.py index d437a5427..63fdc7480 100644 --- a/app/models.py +++ b/app/models.py @@ -40,6 +40,7 @@ class User(db.Model): logged_in_at = db.Column(db.DateTime, nullable=True) failed_login_count = db.Column(db.Integer, nullable=False, default=0) state = db.Column(db.String, nullable=False, default='pending') + platform_admin = db.Column(db.Boolean, nullable=False, default=False) @property def password(self): @@ -306,6 +307,7 @@ SEND_EMAILS = 'send_emails' SEND_LETTERS = 'send_letters' MANAGE_API_KEYS = 'manage_api_keys' ACCESS_DEVELOPER_DOCS = 'access_developer_docs' +PLATFORM_ADMIN = 'platform_admin' # List of permissions PERMISSION_LIST = [ @@ -316,7 +318,8 @@ PERMISSION_LIST = [ SEND_EMAILS, SEND_LETTERS, MANAGE_API_KEYS, - ACCESS_DEVELOPER_DOCS] + ACCESS_DEVELOPER_DOCS, + PLATFORM_ADMIN] class Permission(db.Model): diff --git a/migrations/versions/0041_platform_admin.py b/migrations/versions/0041_platform_admin.py new file mode 100644 index 000000000..d75c8f5c0 --- /dev/null +++ b/migrations/versions/0041_platform_admin.py @@ -0,0 +1,33 @@ +"""empty message + +Revision ID: 0041_platform_admin +Revises: 0040_add_reference +Create Date: 2016-03-16 16:33:15.279429 + +""" + +# revision identifiers, used by Alembic. +revision = '0041_platform_admin' +down_revision = '0040_add_reference' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_index(op.f('ix_notification_statistics_service_id'), 'notification_statistics', ['service_id'], unique=False) + op.drop_index('ix_service_notification_stats_service_id', table_name='notification_statistics') + op.add_column('users', sa.Column('platform_admin', sa.Boolean(), nullable=True, default=False)) + op.get_bind() + op.execute('update users set platform_admin = False') + op.alter_column('users', 'platform_admin', nullable=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'platform_admin') + op.create_index('ix_service_notification_stats_service_id', 'notification_statistics', ['service_id'], unique=False) + op.drop_index(op.f('ix_notification_statistics_service_id'), table_name='notification_statistics') + ### end Alembic commands ### diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 8f928a954..35dce1b60 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -32,6 +32,7 @@ def test_create_user(notify_api, notify_db, notify_db_session): assert User.query.count() == 1 assert User.query.first().email_address == email assert User.query.first().id == user.id + assert not user.platform_admin def test_get_all_users(notify_api, notify_db, notify_db_session, sample_user): From b7f65feadd7cd0842f33570564e0992458239595 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 17 Mar 2016 11:32:55 +0000 Subject: [PATCH 12/23] Set default to 0 for the notification_statistics table. --- app/models.py | 12 ++--- .../versions/0042_default_stats_to_zero.py | 45 +++++++++++++++++++ tests/app/dao/test_notification_dao.py | 9 ++++ 3 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 migrations/versions/0042_default_stats_to_zero.py diff --git a/app/models.py b/app/models.py index d437a5427..c6d1e8c2c 100644 --- a/app/models.py +++ b/app/models.py @@ -114,12 +114,12 @@ class NotificationStatistics(db.Model): day = db.Column(db.String(255), nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) service = db.relationship('Service', backref=db.backref('service_notification_stats', lazy='dynamic')) - emails_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False) - emails_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=True) - emails_error = db.Column(db.BigInteger, index=False, unique=False, nullable=True) - sms_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False) - sms_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=True) - sms_error = db.Column(db.BigInteger, index=False, unique=False, nullable=True) + emails_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) + emails_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) + emails_error = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) + sms_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) + sms_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) + sms_error = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) __table_args__ = ( UniqueConstraint('service_id', 'day', name='uix_service_to_day'), diff --git a/migrations/versions/0042_default_stats_to_zero.py b/migrations/versions/0042_default_stats_to_zero.py new file mode 100644 index 000000000..4b20a9805 --- /dev/null +++ b/migrations/versions/0042_default_stats_to_zero.py @@ -0,0 +1,45 @@ +"""empty message + +Revision ID: 0042_default_stats_to_zero +Revises: 0040_add_reference +Create Date: 2016-03-17 11:09:17.906910 + +""" + +# revision identifiers, used by Alembic. +revision = '0042_default_stats_to_zero' +down_revision = '0040_add_reference' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.get_bind() + op.execute('update notification_statistics set emails_delivered = 0 where emails_delivered is Null') + op.execute('update notification_statistics set emails_error = 0 where emails_error is Null') + op.execute('update notification_statistics set sms_delivered = 0 where sms_delivered is Null') + op.execute('update notification_statistics set sms_error = 0 where sms_error is Null') + op.alter_column('notification_statistics', 'emails_requested', server_default='0') + op.alter_column('notification_statistics', 'emails_delivered', server_default='0', nullable=False) + op.alter_column('notification_statistics', 'emails_error', server_default='0', nullable=False) + op.alter_column('notification_statistics', 'sms_requested', server_default='0') + op.alter_column('notification_statistics', 'sms_delivered', server_default='0', nullable=False) + op.alter_column('notification_statistics', 'sms_error', server_default='0', nullable=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.alter_column('notification_statistics', 'emails_requested', server_default=None) + op.alter_column('notification_statistics', 'emails_delivered', server_default=None) + op.alter_column('notification_statistics', 'emails_error', server_default=None) + op.alter_column('notification_statistics', 'sms_requested', server_default=None) + op.alter_column('notification_statistics', 'sms_delivered', server_default=None) + op.alter_column('notification_statistics', 'sms_error', server_default=None) + op.execute('update notification_statistics set emails_delivered = Null where emails_delivered = 0') + op.execute('update notification_statistics set emails_error = Null where emails_error = 0') + op.execute('update notification_statistics set sms_delivered = Null where sms_delivered = 0') + op.execute('update notification_statistics set sms_error = Null where sms_error = 0') + ### end Alembic commands ### diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index d6a5a95b5..badb93b57 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -70,8 +70,13 @@ def test_should_be_able_to_get_statistics_for_a_service(sample_template): assert len(stats) == 1 assert stats[0].emails_requested == 0 assert stats[0].sms_requested == 1 + assert stats[0].sms_delivered == 0 + assert stats[0].sms_error == 0 assert stats[0].day == notification.created_at.strftime(DATE_FORMAT) assert stats[0].service_id == notification.service_id + assert stats[0].emails_requested == 0 + assert stats[0].emails_delivered == 0 + assert stats[0].emails_error == 0 def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_template): @@ -90,7 +95,11 @@ def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_templat sample_template.service.id, now.strftime(DATE_FORMAT) ) assert stat.emails_requested == 0 + assert stat.emails_error == 0 + assert stat.emails_delivered == 0 assert stat.sms_requested == 1 + assert stat.sms_error == 0 + assert stat.sms_delivered == 0 assert stat.day == notification.created_at.strftime(DATE_FORMAT) assert stat.service_id == notification.service_id From 9a7788a6f5b04051442ae50e8490d31249be771c Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 17 Mar 2016 11:47:44 +0000 Subject: [PATCH 13/23] Allowing overrides on a per environment basis --- app/__init__.py | 1 + aws_run_celery.py | 10 ++++++++++ config.py | 6 ++++++ config_live.py | 16 ++++++++++++++++ config_staging.py | 16 ++++++++++++++++ wsgi.py | 13 ++++++++++++- 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 config_live.py create mode 100644 config_staging.py diff --git a/app/__init__.py b/app/__init__.py index 28508cb1f..f34177786 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -31,6 +31,7 @@ def create_app(app_name=None): application = Flask(__name__) application.config.from_object(os.environ['NOTIFY_API_ENVIRONMENT']) + if app_name: application.config['NOTIFY_APP_NAME'] = app_name diff --git a/aws_run_celery.py b/aws_run_celery.py index 814e07f4b..9ee774ff6 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -2,11 +2,21 @@ from app import notify_celery, create_app from credstash import getAllSecrets import os +from config import configs + +default_env_file = '/home/ubuntu/environment' +environment = 'live' + +if os.path.isfile(default_env_file): + environment_file = open(default_env_file, 'r') + environment = environment_file.readline().strip() # on aws get secrets and export to env secrets = getAllSecrets(region="eu-west-1") for key, val in secrets.items(): os.environ[key] = val +os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] + application = create_app() application.app_context().push() diff --git a/config.py b/config.py index c2b6ce984..0234e9e7b 100644 --- a/config.py +++ b/config.py @@ -84,3 +84,9 @@ class Development(Config): class Test(Development): pass + +configs = { + 'live': 'config_live.Live', + 'staging': 'config_staging.Staging', + 'preview': 'config.Config' +} diff --git a/config_live.py b/config_live.py new file mode 100644 index 000000000..57236f71c --- /dev/null +++ b/config_live.py @@ -0,0 +1,16 @@ +import os +from config import Config + + +class Live(Config): + ADMIN_BASE_URL = os.environ['LIVE_ADMIN_BASE_URL'] + ADMIN_CLIENT_SECRET = os.environ['LIVE_ADMIN_CLIENT_SECRET'] + DANGEROUS_SALT = os.environ['LIVE_DANGEROUS_SALT'] + NOTIFICATION_QUEUE_PREFIX = os.environ['LIVE_NOTIFICATION_QUEUE_PREFIX'] + NOTIFY_JOB_QUEUE = os.environ['LIVE_NOTIFY_JOB_QUEUE'] + SECRET_KEY = os.environ['LIVE_SECRET_KEY'] + SQLALCHEMY_DATABASE_URI = os.environ['LIVE_SQLALCHEMY_DATABASE_URI'] + VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['LIVE_VERIFY_CODE_FROM_EMAIL_ADDRESS'] + NOTIFY_EMAIL_DOMAIN = os.environ['LIVE_NOTIFY_EMAIL_DOMAIN'] + FIRETEXT_API_KEY = os.getenv("LIVE_FIRETEXT_API_KEY") + TWILIO_AUTH_TOKEN = os.getenv('LIVE_TWILIO_AUTH_TOKEN') diff --git a/config_staging.py b/config_staging.py new file mode 100644 index 000000000..3c36cc1b7 --- /dev/null +++ b/config_staging.py @@ -0,0 +1,16 @@ +import os +from config import Config + + +class Staging(Config): + ADMIN_BASE_URL = os.environ['STAGING_ADMIN_BASE_URL'] + ADMIN_CLIENT_SECRET = os.environ['STAGING_ADMIN_CLIENT_SECRET'] + DANGEROUS_SALT = os.environ['STAGING_DANGEROUS_SALT'] + NOTIFICATION_QUEUE_PREFIX = os.environ['STAGING_NOTIFICATION_QUEUE_PREFIX'] + NOTIFY_JOB_QUEUE = os.environ['STAGING_NOTIFY_JOB_QUEUE'] + SECRET_KEY = os.environ['STAGING_SECRET_KEY'] + SQLALCHEMY_DATABASE_URI = os.environ['STAGING_SQLALCHEMY_DATABASE_URI'] + VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['STAGING_VERIFY_CODE_FROM_EMAIL_ADDRESS'] + NOTIFY_EMAIL_DOMAIN = os.environ['STAGING_NOTIFY_EMAIL_DOMAIN'] + FIRETEXT_API_KEY = os.getenv("STAGING_FIRETEXT_API_KEY") + TWILIO_AUTH_TOKEN = os.getenv('STAGING_TWILIO_AUTH_TOKEN') diff --git a/wsgi.py b/wsgi.py index 3e9e20d2f..0dac23248 100644 --- a/wsgi.py +++ b/wsgi.py @@ -2,13 +2,24 @@ import os from app import create_app from credstash import getAllSecrets +from config import configs + + +default_env_file = '/home/ubuntu/environment' +environment = 'live' + +if os.path.isfile(default_env_file): + environment_file = open(default_env_file, 'r') + environment = environment_file.readline().strip() # on aws get secrets and export to env secrets = getAllSecrets(region="eu-west-1") for key, val in secrets.items(): os.environ[key] = val +os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] + application = create_app() if __name__ == "__main__": - application.run() + application.run() From b0c6a1a7c6764e97efcc7d42fe6aa4e1f928939a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 17 Mar 2016 11:50:32 +0000 Subject: [PATCH 14/23] Fix the order for the migration script --- migrations/versions/0042_default_stats_to_zero.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/migrations/versions/0042_default_stats_to_zero.py b/migrations/versions/0042_default_stats_to_zero.py index 4b20a9805..4d862c903 100644 --- a/migrations/versions/0042_default_stats_to_zero.py +++ b/migrations/versions/0042_default_stats_to_zero.py @@ -1,14 +1,14 @@ """empty message Revision ID: 0042_default_stats_to_zero -Revises: 0040_add_reference +Revises: 0041_platform_admin Create Date: 2016-03-17 11:09:17.906910 """ # revision identifiers, used by Alembic. revision = '0042_default_stats_to_zero' -down_revision = '0040_add_reference' +down_revision = '0041_platform_admin' from alembic import op import sqlalchemy as sa @@ -33,11 +33,11 @@ def upgrade(): def downgrade(): ### commands auto generated by Alembic - please adjust! ### op.alter_column('notification_statistics', 'emails_requested', server_default=None) - op.alter_column('notification_statistics', 'emails_delivered', server_default=None) - op.alter_column('notification_statistics', 'emails_error', server_default=None) + op.alter_column('notification_statistics', 'emails_delivered', server_default=None, nullable=True) + op.alter_column('notification_statistics', 'emails_error', server_default=None, nullable=True) op.alter_column('notification_statistics', 'sms_requested', server_default=None) - op.alter_column('notification_statistics', 'sms_delivered', server_default=None) - op.alter_column('notification_statistics', 'sms_error', server_default=None) + op.alter_column('notification_statistics', 'sms_delivered', server_default=None, nullable=True) + op.alter_column('notification_statistics', 'sms_error', server_default=None, nullable=True) op.execute('update notification_statistics set emails_delivered = Null where emails_delivered = 0') op.execute('update notification_statistics set emails_error = Null where emails_error = 0') op.execute('update notification_statistics set sms_delivered = Null where sms_delivered = 0') From 6aec6a0bdaddb4ee6c4bb69487c7f8e6bcb6ac31 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 17 Mar 2016 12:51:14 +0000 Subject: [PATCH 15/23] Update as per pull request comments: - context manager for file handling - os.environ.update for setting overrides --- aws_run_celery.py | 8 +++----- wsgi.py | 9 +++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/aws_run_celery.py b/aws_run_celery.py index 9ee774ff6..bbc64d2ad 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -8,13 +8,11 @@ default_env_file = '/home/ubuntu/environment' environment = 'live' if os.path.isfile(default_env_file): - environment_file = open(default_env_file, 'r') - environment = environment_file.readline().strip() + with open(default_env_file, 'r') as environment_file: + environment = environment_file.readline().strip() # on aws get secrets and export to env -secrets = getAllSecrets(region="eu-west-1") -for key, val in secrets.items(): - os.environ[key] = val +os.environ.update(getAllSecrets(region="eu-west-1")) os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] diff --git a/wsgi.py b/wsgi.py index 0dac23248..4149fb92a 100644 --- a/wsgi.py +++ b/wsgi.py @@ -9,14 +9,11 @@ default_env_file = '/home/ubuntu/environment' environment = 'live' if os.path.isfile(default_env_file): - environment_file = open(default_env_file, 'r') - environment = environment_file.readline().strip() + with open(default_env_file, 'r') as environment_file: + environment = environment_file.readline().strip() # on aws get secrets and export to env -secrets = getAllSecrets(region="eu-west-1") -for key, val in secrets.items(): - os.environ[key] = val - +os.environ.update(getAllSecrets(region="eu-west-1")) os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] application = create_app() From 8b83e6a02a2e5e66d5b86d9443fcedf2986a8c86 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 17 Mar 2016 13:21:01 +0000 Subject: [PATCH 16/23] Moved import until after env setup --- aws_run_celery.py | 3 ++- wsgi.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/aws_run_celery.py b/aws_run_celery.py index bbc64d2ad..d5a58a09f 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -2,7 +2,6 @@ from app import notify_celery, create_app from credstash import getAllSecrets import os -from config import configs default_env_file = '/home/ubuntu/environment' environment = 'live' @@ -14,6 +13,8 @@ if os.path.isfile(default_env_file): # on aws get secrets and export to env os.environ.update(getAllSecrets(region="eu-west-1")) +from config import configs + os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] application = create_app() diff --git a/wsgi.py b/wsgi.py index 4149fb92a..52e95994c 100644 --- a/wsgi.py +++ b/wsgi.py @@ -2,7 +2,6 @@ import os from app import create_app from credstash import getAllSecrets -from config import configs default_env_file = '/home/ubuntu/environment' @@ -14,6 +13,9 @@ if os.path.isfile(default_env_file): # on aws get secrets and export to env os.environ.update(getAllSecrets(region="eu-west-1")) + +from config import configs + os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] application = create_app() From 2d1d8832835be5994cb9cb083fbda319c00df7e0 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 17 Mar 2016 13:06:17 +0000 Subject: [PATCH 17/23] Added task for sending email verification links out on intial registration. Left original email code endpoint in as it is still used for things like email change. --- app/celery/tasks.py | 20 +++++++++++++++ app/user/rest.py | 39 +++++++++++++++++++++++++++++- config.py | 3 ++- tests/app/conftest.py | 5 ++++ tests/app/user/test_rest_verify.py | 38 ++++++++++++++++++++++++++--- 5 files changed, 100 insertions(+), 5 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 94fb59bc4..c354273a9 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -370,3 +370,23 @@ def email_reset_password(encrypted_reset_password_message): url=reset_password_message['reset_password_url'])) except AwsSesClientException as e: current_app.logger.exception(e) + + +def registration_verification_template(name, url): + from string import Template + t = Template("Hi $name,\n\n" + "To complete your registration for GOV.UK Notify please click the link below\n\n $url") + return t.substitute(name=name, url=url) + + +@notify_celery.task(name='email-registration-verification') +def email_registration_verification(encrypted_verification_message): + verification_message = encryption.decrypt(encrypted_verification_message) + try: + aws_ses_client.send_email(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], + verification_message['to'], + "Confirm GOV.UK Notify registration", + registration_verification_template(name=verification_message['name'], + url=verification_message['url'])) + except AwsSesClientException as e: + current_app.logger.exception(e) diff --git a/app/user/rest.py b/app/user/rest.py index cb574a909..06adc72e4 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -24,7 +24,13 @@ from app.schemas import ( permission_schema ) -from app.celery.tasks import (send_sms_code, send_email_code, email_reset_password) +from app.celery.tasks import ( + send_sms_code, + send_email_code, + email_reset_password, + email_registration_verification +) + from app.errors import register_errors user = Blueprint('user', __name__) @@ -148,6 +154,28 @@ def send_user_email_code(user_id): return jsonify({}), 204 +@user.route('//email-verification', methods=['POST']) +def send_user_email_verification(user_id): + user_to_send_to = get_model_users(user_id=user_id) + verify_code, errors = request_verify_code_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + + from app.dao.users_dao import create_secret_code + secret_code = create_secret_code() + create_user_code(user_to_send_to, secret_code, 'email') + + email = user_to_send_to.email_address + verification_message = {'to': email, + 'name': user_to_send_to.name, + 'url': _create_verification_url(user_to_send_to, secret_code)} + + email_registration_verification.apply_async([encryption.encrypt(verification_message)], + queue='email-registration-verification') + + return jsonify({}), 204 + + @user.route('/', methods=['GET']) @user.route('', methods=['GET']) def get_user(user_id=None): @@ -207,3 +235,12 @@ def _create_reset_password_url(email): token = generate_token(data, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT']) return current_app.config['ADMIN_BASE_URL'] + '/new-password/' + token + + +def _create_verification_url(user, secret_code): + from utils.url_safe_token import generate_token + import json + data = json.dumps({'user_id': user.id, 'email': user.email_address, 'secret_code': secret_code}) + token = generate_token(data, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT']) + + return current_app.config['ADMIN_BASE_URL'] + '/verify-email/' + token diff --git a/config.py b/config.py index 0234e9e7b..0bf45618c 100644 --- a/config.py +++ b/config.py @@ -69,7 +69,8 @@ class Config(object): Queue('process-job', Exchange('default'), routing_key='process-job'), Queue('bulk-sms', Exchange('default'), routing_key='bulk-sms'), Queue('bulk-email', Exchange('default'), routing_key='bulk-email'), - Queue('email-invited-user', Exchange('default'), routing_key='email-invited-user') + Queue('email-invited-user', Exchange('default'), routing_key='email-invited-user'), + Queue('email-registration-verification', Exchange('default'), routing_key='email-registration-verification') ] TWILIO_ACCOUNT_SID = os.getenv('TWILIO_ACCOUNT_SID') TWILIO_AUTH_TOKEN = os.getenv('TWILIO_AUTH_TOKEN') diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e954d6cb0..63bad1b8f 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -327,6 +327,11 @@ def mock_celery_send_email_code(mocker): return mocker.patch('app.celery.tasks.send_email_code.apply_async') +@pytest.fixture(scope='function') +def mock_celery_email_registration_verification(mocker): + return mocker.patch('app.celery.tasks.email_registration_verification.apply_async') + + @pytest.fixture(scope='function') def mock_encryption(mocker): return mocker.patch('app.encryption.encrypt', return_value="something_encrypted") diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 12dd3b5ba..e666c0140 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -1,13 +1,25 @@ import json import moto -from datetime import (datetime, timedelta) + +from datetime import ( + datetime, + timedelta +) + from flask import url_for -from app.models import (VerifyCode, User) -import app.celery.tasks + +from app.models import ( + VerifyCode, + User +) + from app import db, encryption + from tests import create_authorization_header from freezegun import freeze_time +import app.celery.tasks + def test_user_verify_code_sms(notify_api, sample_sms_code): @@ -341,3 +353,23 @@ def test_send_user_email_code_returns_404_for_when_user_does_not_exist(notify_ap headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' + + +def test_send_user_email_verification(notify_api, + sample_email_code, + mock_celery_email_registration_verification, + mock_encryption): + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + auth_header = create_authorization_header( + path=url_for('user.send_user_email_verification', user_id=sample_email_code.user.id), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_email_verification', user_id=sample_email_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + app.celery.tasks.email_registration_verification.apply_async.assert_called_once_with(['something_encrypted'], queue='email-registration-verification') # noqa From c052f884080164e63143a445d18e2cc14e51ddc8 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 09:12:19 +0000 Subject: [PATCH 18/23] Adding specific staging/live property for API HOST --- config_live.py | 1 + config_staging.py | 1 + 2 files changed, 2 insertions(+) diff --git a/config_live.py b/config_live.py index 57236f71c..779d9bef3 100644 --- a/config_live.py +++ b/config_live.py @@ -4,6 +4,7 @@ from config import Config class Live(Config): ADMIN_BASE_URL = os.environ['LIVE_ADMIN_BASE_URL'] + API_HOST_NAME = os.environ['LIVE_API_HOST_NAME'] ADMIN_CLIENT_SECRET = os.environ['LIVE_ADMIN_CLIENT_SECRET'] DANGEROUS_SALT = os.environ['LIVE_DANGEROUS_SALT'] NOTIFICATION_QUEUE_PREFIX = os.environ['LIVE_NOTIFICATION_QUEUE_PREFIX'] diff --git a/config_staging.py b/config_staging.py index 3c36cc1b7..a06ad3daf 100644 --- a/config_staging.py +++ b/config_staging.py @@ -4,6 +4,7 @@ from config import Config class Staging(Config): ADMIN_BASE_URL = os.environ['STAGING_ADMIN_BASE_URL'] + API_HOST_NAME = os.environ['STAGING_API_HOST_NAME'] ADMIN_CLIENT_SECRET = os.environ['STAGING_ADMIN_CLIENT_SECRET'] DANGEROUS_SALT = os.environ['STAGING_DANGEROUS_SALT'] NOTIFICATION_QUEUE_PREFIX = os.environ['STAGING_NOTIFICATION_QUEUE_PREFIX'] From 8871d4eda620b93e7b02fdb8a199e1cc6622a78e Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 09:15:05 +0000 Subject: [PATCH 19/23] Env specific firetext from numbers --- config_live.py | 1 + config_staging.py | 1 + 2 files changed, 2 insertions(+) diff --git a/config_live.py b/config_live.py index 779d9bef3..ce3f75409 100644 --- a/config_live.py +++ b/config_live.py @@ -14,4 +14,5 @@ class Live(Config): VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['LIVE_VERIFY_CODE_FROM_EMAIL_ADDRESS'] NOTIFY_EMAIL_DOMAIN = os.environ['LIVE_NOTIFY_EMAIL_DOMAIN'] FIRETEXT_API_KEY = os.getenv("LIVE_FIRETEXT_API_KEY") + FIRETEXT_NUMBER = os.getenv("LIVE_FIRETEXT_NUMBER") TWILIO_AUTH_TOKEN = os.getenv('LIVE_TWILIO_AUTH_TOKEN') diff --git a/config_staging.py b/config_staging.py index a06ad3daf..e5addd7f4 100644 --- a/config_staging.py +++ b/config_staging.py @@ -14,4 +14,5 @@ class Staging(Config): VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['STAGING_VERIFY_CODE_FROM_EMAIL_ADDRESS'] NOTIFY_EMAIL_DOMAIN = os.environ['STAGING_NOTIFY_EMAIL_DOMAIN'] FIRETEXT_API_KEY = os.getenv("STAGING_FIRETEXT_API_KEY") + FIRETEXT_NUMBER = os.getenv("STAGING_FIRETEXT_NUMBER") TWILIO_AUTH_TOKEN = os.getenv('STAGING_TWILIO_AUTH_TOKEN') From fc7ad7d55622aa9edb77b9f7822260110a772805 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 09:55:19 +0000 Subject: [PATCH 20/23] Bring DB script into line with other prod scripts --- db.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/db.py b/db.py index 8e2efab6e..d923c8c75 100644 --- a/db.py +++ b/db.py @@ -4,9 +4,19 @@ from app import create_app, db from credstash import getAllSecrets import os -secrets = getAllSecrets(region="eu-west-1") -for key, val in secrets.items(): - os.environ[key] = val +default_env_file = '/home/ubuntu/environment' +environment = 'live' + +if os.path.isfile(default_env_file): + with open(default_env_file, 'r') as environment_file: + environment = environment_file.readline().strip() + +# on aws get secrets and export to env +os.environ.update(getAllSecrets(region="eu-west-1")) + +from config import configs + +os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment] application = create_app() From c3ffae3cf7d412dabd8bdd1c3d5ba182c0d64206 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 10:23:32 +0000 Subject: [PATCH 21/23] Live deployment on api codedeploy groups --- .travis.yml | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/.travis.yml b/.travis.yml index d4e4e1332..08ca9cbf3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -100,6 +100,47 @@ deploy: deployment_group: notifications_admin_api_deployment_group region: eu-west-1 on: *2 +- provider: s3 + access_key_id: AKIAJ5MKF6G3P2JQP4QQ + secret_access_key: &1 + secure: daC1bCHXqLRK+iIZ8P699KCnTh77lwV4KxrZxL1yd6cstgfptyd/rg1WgRwE6QdxOCT9gQvKWUZFCzFy7M6E/Ih8EUHqEXTzC5M4oAye8rhePIBMQwqkgfYyIoZ3LdDMMP5JfBhiz0zS3Vj7HerL2qIu12adJBjkRJx3XAGimCrFOMQ0xUXQAKDjL6Xmv+gVz2f/ISLy6icKY4KNGt3cQV+8pa5aMF34C9R2udA9N67EWlXlh7hJbFtmY+0Zqpo8Rr6wKRb5MA0xEcTVLORSz1aa6GkxUCbzaIH99p7z3Ghz0qW2bUi9ZcDrvg0GLbVe1T+1HXhfktJfW8wnzw6A/2U/CIIFDQZ/qk0w/DkEwpQinXow99Zl49CcEU+v8llKhg5nM3LmAZCQg1c/iZyP/d90AwAMoMA/VTDD72M93IqTJQH18eC8g02DwE0hNDD6aos5wzeuDeiH/6BG+Tq0pDl0y0aWCcHf3vGRlo/5GlWfpE0vMQEC+qnEOWOUqSprCdSypgD2Aip9mCC98w4BkqKKvGNHPZolA7rxf7E9hTK+BNPRATpYsHR1X/1Xl0TMc/pHhjU1yNXzWnI/kOlNV2CRq3slEtcWihaEo8oDHJ+BhGT49Ps3Je7UB2xO/jXXFPhwJotPMOacTcnUkGqVJSlK1g6TIn4t9nTVSY8KFUs= + local_dir: dpl_cd_upload + skip_cleanup: true + region: eu-west-1 + on: &2 + repo: alphagov/notifications-api + branch: live + bucket: live-notifications-api-codedeploy +- provider: codedeploy + access_key_id: AKIAJ5MKF6G3P2JQP4QQ + secret_access_key: *1 + bucket: live-notifications-api-codedeploy + key: notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip + bundle_type: zip + application: notifications-api + deployment_group: live_notifications_delivery_api_deployment_group + region: eu-west-1 + on: *2 +- provider: codedeploy + access_key_id: AKIAJ5MKF6G3P2JQP4QQ + secret_access_key: *1 + bucket: live-notifications-api-codedeploy + key: notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip + bundle_type: zip + application: notifications-api + deployment_group: live_notifications_api_deployment_group + region: eu-west-1 + on: *2 +- provider: codedeploy + access_key_id: AKIAJ5MKF6G3P2JQP4QQ + secret_access_key: *1 + bucket: live-notifications-api-codedeploy + key: notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip + bundle_type: zip + application: notifications-api + deployment_group: live_notifications_admin_api_deployment_group + region: eu-west-1 + on: *2 before_deploy: - ./scripts/update_version_file.sh - zip -r --exclude=*__pycache__* notifications-api * From 02bbb05654593e7cd683bbaf921ffae268de9610 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 11:01:40 +0000 Subject: [PATCH 22/23] Per environment celery setups --- config_live.py | 7 +++++++ config_staging.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/config_live.py b/config_live.py index ce3f75409..65f36eeb7 100644 --- a/config_live.py +++ b/config_live.py @@ -16,3 +16,10 @@ class Live(Config): FIRETEXT_API_KEY = os.getenv("LIVE_FIRETEXT_API_KEY") FIRETEXT_NUMBER = os.getenv("LIVE_FIRETEXT_NUMBER") TWILIO_AUTH_TOKEN = os.getenv('LIVE_TWILIO_AUTH_TOKEN') + + BROKER_TRANSPORT_OPTIONS = { + 'region': 'eu-west-1', + 'polling_interval': 1, # 1 second + 'visibility_timeout': 60, # 60 seconds + 'queue_name_prefix': os.environ['LIVE_NOTIFICATION_QUEUE_PREFIX'] + '-' + } diff --git a/config_staging.py b/config_staging.py index e5addd7f4..bf0f44119 100644 --- a/config_staging.py +++ b/config_staging.py @@ -16,3 +16,10 @@ class Staging(Config): FIRETEXT_API_KEY = os.getenv("STAGING_FIRETEXT_API_KEY") FIRETEXT_NUMBER = os.getenv("STAGING_FIRETEXT_NUMBER") TWILIO_AUTH_TOKEN = os.getenv('STAGING_TWILIO_AUTH_TOKEN') + + BROKER_TRANSPORT_OPTIONS = { + 'region': 'eu-west-1', + 'polling_interval': 1, # 1 second + 'visibility_timeout': 60, # 60 seconds + 'queue_name_prefix': os.environ['STAGING_NOTIFICATION_QUEUE_PREFIX'] + '-' + } From 0b62d65e7a31c5f45a72bd70287d4146e5817516 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 18 Mar 2016 12:58:17 +0000 Subject: [PATCH 23/23] Readme update --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 5c6d177b4..0171a2ace 100644 --- a/README.md +++ b/README.md @@ -62,3 +62,6 @@ scripts/run_app.sh scripts/run_celery.sh ``` +``` +scripts/run_celery_beat.sh +```