From cfd31541f4c2b96be82453675923929257bf69b5 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 9 Jun 2016 16:41:20 +0100 Subject: [PATCH 1/5] Use notify to send email verification --- app/user/rest.py | 31 +++++++++++++++++------- tests/app/conftest.py | 34 ++++++++++++++++++++++++++ tests/app/user/test_rest_verify.py | 39 ++++++++++++++++++++++++++---- 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 12fde343a..f4905ef36 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -28,8 +28,8 @@ from app.schemas import ( from app.celery.tasks import ( send_sms, email_reset_password, - email_registration_verification -) + email_registration_verification, + send_email) from app.errors import register_errors @@ -157,13 +157,26 @@ def send_user_email_verification(user_id): 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') + template = dao_get_template_by_id(current_app.config['EMAIL_VERIFY_CODE_TEMPLATE_ID']) + message = { + 'template': template.id, + 'template_version': template.version, + 'to': user_to_send_to.email_address, + 'personalisation': { + 'user_name': user_to_send_to.name, + 'url': _create_verification_url(user_to_send_to, secret_code) + } + } + email_from = '"GOV.UK Notify" <{}>'.format( + current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] + ) + send_email.apply_async(( + current_app.config['NOTIFY_SERVICE_ID'], + str(uuid.uuid4()), + email_from, + encryption.encrypt(message), + datetime.utcnow().strftime(DATETIME_FORMAT) + ), queue='email-registration-verification') return jsonify({}), 204 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 01c6cb952..70b4ddbe0 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -589,3 +589,37 @@ def sms_code_template(notify_db, template = Template(**data) db.session.add(template) return template + + +@pytest.fixture(scope='function') +def email_verification_template(notify_db, + notify_db_session): + user = sample_user(notify_db, notify_db_session) + service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) + if not service: + data = { + 'id': current_app.config['NOTIFY_SERVICE_ID'], + 'name': 'Notify Service', + 'message_limit': 1000, + 'active': True, + 'restricted': False, + 'email_from': 'notify.service', + 'created_by': user + } + service = Service(**data) + db.session.add(service) + + template = Template.query.get(current_app.config['SMS_CODE_TEMPLATE_ID']) + if not template: + data = { + 'id': current_app.config['EMAIL_VERIFY_CODE_TEMPLATE_ID'], + 'name': 'Email verification template', + 'template_type': 'email', + 'content': '((user_name)) use ((url)) to complete registration', + 'service': service, + 'created_by': user, + 'archived': False + } + template = Template(**data) + db.session.add(template) + return template diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 3ca379171..7f2a6130e 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -309,18 +309,47 @@ def test_send_sms_code_returns_404_for_bad_input_data(notify_api, notify_db, not assert json.loads(resp.get_data(as_text=True))['message'] == 'No result found' +@freeze_time("2016-01-01 11:09:00.061258") def test_send_user_email_verification(notify_api, - sample_email_code, - mock_celery_email_registration_verification, - mock_encryption): + sample_user, + mocker, + email_verification_template): with notify_api.test_request_context(): with notify_api.test_client() as client: data = json.dumps({}) + mocker.patch('uuid.uuid4', return_value='some_uuid') # for the notification id + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocked = mocker.patch('app.celery.tasks.send_email.apply_async') auth_header = create_authorization_header() resp = client.post( - url_for('user.send_user_email_verification', user_id=str(sample_email_code.user.id)), + url_for('user.send_user_email_verification', user_id=str(sample_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 + assert mocked.call_count == 1 + app.celery.tasks.send_email.apply_async.assert_called_once_with( + (str(current_app.config['NOTIFY_SERVICE_ID']), + 'some_uuid', + '"GOV.UK Notify" <{}>'.format(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS']), + "something_encrypted", + "2016-01-01T11:09:00.061258"), + queue="email-registration-verification") + + +def test_send_email_verification_returns_404_for_bad_input_data(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint /user//sms-code return 404 for bad input data + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + import uuid + uuid_ = uuid.uuid4() + auth_header = create_authorization_header() + resp = client.post( + url_for('user.send_user_email_verification', user_id=uuid_), + data=data, + 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' From 877fd6fdc4899f510b20d2ad97e40ce69b93bcc2 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 10 Jun 2016 17:19:10 +0100 Subject: [PATCH 2/5] Make template id a string in the json messge --- app/user/rest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/user/rest.py b/app/user/rest.py index f4905ef36..61d35e812 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -159,7 +159,7 @@ def send_user_email_verification(user_id): template = dao_get_template_by_id(current_app.config['EMAIL_VERIFY_CODE_TEMPLATE_ID']) message = { - 'template': template.id, + 'template': str(template.id), 'template_version': template.version, 'to': user_to_send_to.email_address, 'personalisation': { From ea80596e7375ec07964e22438a82742a20f2efaa Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 13 Jun 2016 11:29:07 +0100 Subject: [PATCH 3/5] Correct template id for email verification template in the history table. Correct personalisation name when sending registration message. --- app/user/rest.py | 2 +- .../versions/0028_fix_reg_template_history.py | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0028_fix_reg_template_history.py diff --git a/app/user/rest.py b/app/user/rest.py index 61d35e812..bb31985eb 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -163,7 +163,7 @@ def send_user_email_verification(user_id): 'template_version': template.version, 'to': user_to_send_to.email_address, 'personalisation': { - 'user_name': user_to_send_to.name, + 'name': user_to_send_to.name, 'url': _create_verification_url(user_to_send_to, secret_code) } } diff --git a/migrations/versions/0028_fix_reg_template_history.py b/migrations/versions/0028_fix_reg_template_history.py new file mode 100644 index 000000000..88faac00e --- /dev/null +++ b/migrations/versions/0028_fix_reg_template_history.py @@ -0,0 +1,40 @@ +"""empty message + +Revision ID: 0028_fix_reg_template_history +Revises: 0027_update_provider_rates +Create Date: 2016-06-13 11:04:15.888017 + +""" + +# revision identifiers, used by Alembic. +from datetime import datetime + +revision = '0028_fix_reg_template_history' +down_revision = '0027_update_provider_rates' + +from alembic import op +import sqlalchemy as sa + +service_id = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553' +user_id= '6af522d0-2915-4e52-83a3-3690455a5fe6' + +def upgrade(): + op.get_bind() + op.execute("delete from templates_history where name = 'Notify email verification code'") + + template_history_insert = """INSERT INTO templates_history (id, name, template_type, created_at, + content, archived, service_id, + subject, created_by_id, version) + VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', 1) + """ + email_verification_content = \ + """Hi ((name)),\n\nTo complete your registration for GOV.UK Notify please click the link below\n\n((url))""" + op.execute(template_history_insert.format('ece42649-22a8-4d06-b87f-d52d5d3f0a27', + 'Notify email verification code', 'email', + datetime.utcnow(), email_verification_content, service_id, + 'Confirm GOV.UK Notify registration', user_id)) + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + pass + ### end Alembic commands ### From c9f1eb65a74e2703a2d0ba61b58e6c05dfd439f1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 13 Jun 2016 14:09:03 +0100 Subject: [PATCH 4/5] Build the from address in the task instead of the rest call. --- app/celery/tasks.py | 3 +++ app/notifications/rest.py | 2 +- tests/app/notifications/test_rest.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d3ccbdfc4..ceaf7c240 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -343,6 +343,9 @@ def send_email(service_id, notification_id, from_address, encrypted_notification (provider.get_name(), str(reference), notification['to']), queue='research-mode' ) else: + # First step setting the from_address here rather than the method creating the task + from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config[ + 'NOTIFY_EMAIL_DOMAIN']) if from_address == "" else from_address reference = provider.send_email( from_address, notification['to'], diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 3b7541354..a98cff336 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -387,7 +387,7 @@ def send_notification(notification_type): send_email.apply_async(( service_id, notification_id, - '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + '', encryption.encrypt(notification), datetime.utcnow().strftime(DATETIME_FORMAT) ), queue='email') diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 7c12f6e17..0853e66f5 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -946,7 +946,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template app.celery.tasks.send_email.apply_async.assert_called_once_with( (str(sample_email_template.service_id), notification_id, - "\"Sample service\" ", + "", "something_encrypted", "2016-01-01T11:09:00.061258"), queue="email" From 5fc14940f3777af870ef9e2cae469b23ee521a71 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 13 Jun 2016 14:56:21 +0100 Subject: [PATCH 5/5] Let the send_email task set the from address --- app/celery/tasks.py | 8 +------- app/user/rest.py | 5 +---- tests/app/celery/test_tasks.py | 18 +++--------------- tests/app/user/test_rest_verify.py | 2 +- 4 files changed, 6 insertions(+), 27 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ceaf7c240..2859cf64c 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -180,16 +180,10 @@ def process_job(job_id): ) if template.template_type == 'email': - from_email = '"{}" <{}@{}>'.format( - service.name, - service.email_from, - current_app.config['NOTIFY_EMAIL_DOMAIN'] - ) - send_email.apply_async(( str(job.service_id), create_uuid(), - from_email.encode('ascii', 'ignore').decode('ascii'), + '', encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), {'reply_to_addresses': service.reply_to_email_address}, diff --git a/app/user/rest.py b/app/user/rest.py index bb31985eb..1907caff4 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -167,13 +167,10 @@ def send_user_email_verification(user_id): 'url': _create_verification_url(user_to_send_to, secret_code) } } - email_from = '"GOV.UK Notify" <{}>'.format( - current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'] - ) send_email.apply_async(( current_app.config['NOTIFY_SERVICE_ID'], str(uuid.uuid4()), - email_from, + '', encryption.encrypt(message), datetime.utcnow().strftime(DATETIME_FORMAT) ), queue='email-registration-verification') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 7034368d8..201982295 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -248,11 +248,7 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, ( str(job.service_id), "uuid", - "\"{}\" <{}@{}>".format( - service.name, - service.email_from, - "test.notify.com" - ), + "", "something_encrypted", "2016-01-01T11:09:00.061258" ), @@ -298,11 +294,7 @@ def test_should_process_email_job(sample_email_job, mocker, mock_celery_remove_j ( str(sample_email_job.service_id), "uuid", - "\"{}\" <{}@{}>".format( - sample_email_job.service.name, - sample_email_job.service.email_from, - "test.notify.com" - ), + "", "something_encrypted", "2016-01-01T11:09:00.061258" ), @@ -960,11 +952,7 @@ def test_process_email_job_should_use_reply_to_email_if_present(sample_email_job ( str(sample_email_job.service_id), "uuid", - "\"{}\" <{}@{}>".format( - sample_email_job.service.name, - sample_email_job.service.email_from, - "test.notify.com" - ), + "", "something_encrypted", ANY ), diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 7f2a6130e..9d2d31687 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -331,7 +331,7 @@ def test_send_user_email_verification(notify_api, app.celery.tasks.send_email.apply_async.assert_called_once_with( (str(current_app.config['NOTIFY_SERVICE_ID']), 'some_uuid', - '"GOV.UK Notify" <{}>'.format(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS']), + '', "something_encrypted", "2016-01-01T11:09:00.061258"), queue="email-registration-verification")