diff --git a/app/celery/tasks.py b/app/celery/tasks.py index c4cd157c8..b814407ec 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -197,7 +197,7 @@ def send_sms_code(encrypted_verification): try: firetext_client.send_sms(verification_message['to'], verification_message['secret_code']) except FiretextClientException as e: - current_app.logger.error(e) + current_app.logger.exception(e) @notify_celery.task(name='send-email-code') @@ -209,7 +209,7 @@ def send_email_code(encrypted_verification_message): "Verification code", verification_message['secret_code']) except AwsSesClientException as e: - current_app.logger.error(e) + current_app.logger.exception(e) # TODO: when placeholders in templates work, this will be a real template @@ -251,4 +251,27 @@ def email_invited_user(encrypted_invitation): subject_line, invitation_content) except AwsSesClientException as e: - current_app.logger.error(e) + current_app.logger.exception(e) + + +def password_reset_message(name, url): + from string import Template + t = Template("Hi $user_name,\n\n" + "We received a request to reset your password on GOV.UK Notify.\n\n" + "If you didn't request this email, you can ignore it – your password has not been changed.\n\n" + "To reset your password, click this link:\n\n" + "$url") + return t.substitute(user_name=name, url=url) + + +@notify_celery.task(name='email-reset-password') +def email_reset_password(encrypted_reset_password_message): + reset_password_message = encryption.decrypt(encrypted_reset_password_message) + try: + aws_ses_client.send_email(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], + reset_password_message['to'], + "Reset your GOV.UK Notify password", + password_reset_message(name=reset_password_message['name'], + url=reset_password_message['reset_password_url'])) + except AwsSesClientException as e: + current_app.logger.exception(e) diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 19e70302d..cfaf4cdb8 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -16,6 +16,7 @@ def save_model_user(usr, update_dict={}, pwd=None): if update_dict: if update_dict.get('id'): del update_dict['id'] + update_dict.pop('password_changed_at') db.session.query(User).filter_by(id=usr.id).update(update_dict) else: db.session.add(usr) diff --git a/app/errors.py b/app/errors.py index 890841cc3..465ed8acb 100644 --- a/app/errors.py +++ b/app/errors.py @@ -41,7 +41,7 @@ def register_errors(blueprint): @blueprint.app_errorhandler(500) def internal_server_error(e): if isinstance(e, str): - current_app.logger.error(e) + current_app.logger.exception(e) elif isinstance(e, Exception): current_app.logger.exception(e) return jsonify(result='error', message="Internal server error"), 500 diff --git a/app/schemas.py b/app/schemas.py index e89ccd21b..0ec518dc4 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -1,10 +1,8 @@ -import re -from flask import current_app from flask_marshmallow.fields import fields from . import ma from . import models from app.dao.permissions_dao import permission_dao -from marshmallow import (post_load, ValidationError, validates, validates_schema) +from marshmallow import (post_load, ValidationError, validates) from marshmallow_sqlalchemy import field_for from utils.recipients import ( validate_email_address, InvalidEmailError, @@ -50,6 +48,8 @@ class BaseSchema(ma.ModelSchema): class UserSchema(BaseSchema): permissions = fields.Method("user_permissions", dump_only=True) + password_changed_at = field_for(models.User, 'password_changed_at', format='%Y-%m-%d %H:%M:%S.%f') + created_at = field_for(models.User, 'created_at', format='%Y-%m-%d %H:%M:%S.%f') def user_permissions(self, usr): retval = {} @@ -95,18 +95,6 @@ class JobSchema(BaseSchema): model = models.Job -# TODO: Remove this schema once the admin app has stopped using the /user/code endpoint -class OldRequestVerifyCodeSchema(ma.Schema): - - code_type = fields.Str(required=True) - to = fields.Str(required=False) - - @validates('code_type') - def validate_code_type(self, code): - if code not in models.VERIFY_CODE_TYPES: - raise ValidationError('Invalid code type') - - class RequestVerifyCodeSchema(ma.Schema): to = fields.Str(required=False) @@ -142,8 +130,8 @@ class EmailNotificationSchema(NotificationSchema): def validate_to(self, value): try: validate_email_address(value) - except InvalidEmailError: - raise ValidationError('Invalid email') + except InvalidEmailError as e: + raise ValidationError(e.message) class SmsTemplateNotificationSchema(SmsNotificationSchema): @@ -180,8 +168,8 @@ class InvitedUserSchema(BaseSchema): def validate_to(self, value): try: validate_email_address(value) - except InvalidEmailError: - raise ValidationError('Invalid email') + except InvalidEmailError as e: + raise ValidationError(e.message) class PermissionSchema(BaseSchema): @@ -201,6 +189,16 @@ class PermissionSchema(BaseSchema): exclude = ("created_at",) +class EmailDataSchema(ma.Schema): + email = fields.Str(required=False) + + @validates('email') + def validate_email(self, value): + try: + validate_email_address(value) + except InvalidEmailError as e: + raise ValidationError(e.message) + user_schema = UserSchema() user_schema_load_json = UserSchema(load_json=True) service_schema = ServiceSchema() @@ -211,7 +209,6 @@ api_key_schema = ApiKeySchema() api_key_schema_load_json = ApiKeySchema(load_json=True) job_schema = JobSchema() job_schema_load_json = JobSchema(load_json=True) -old_request_verify_code_schema = OldRequestVerifyCodeSchema() request_verify_code_schema = RequestVerifyCodeSchema() sms_admin_notification_schema = SmsAdminNotificationSchema() sms_template_notification_schema = SmsTemplateNotificationSchema() @@ -222,4 +219,5 @@ notification_status_schema = NotificationStatusSchema() notification_status_schema_load_json = NotificationStatusSchema(load_json=True) invited_user_schema = InvitedUserSchema() permission_schema = PermissionSchema() +email_data_request_schema = EmailDataSchema() notifications_statistics_schema = NotificationsStatisticsSchema() diff --git a/app/user/rest.py b/app/user/rest.py index 6287522b6..8886429fd 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -1,5 +1,5 @@ from datetime import datetime -from flask import (jsonify, request, abort, Blueprint) +from flask import (jsonify, request, abort, Blueprint, current_app) from app import encryption from app.dao.users_dao import ( @@ -17,14 +17,14 @@ from app.dao.permissions_dao import permission_dao from app.dao.services_dao import dao_fetch_service_by_id from app.schemas import ( - old_request_verify_code_schema, + email_data_request_schema, user_schema, request_verify_code_schema, user_schema_load_json, permission_schema ) -from app.celery.tasks import (send_sms_code, send_email_code) +from app.celery.tasks import (send_sms_code, send_email_code, email_reset_password) from app.errors import register_errors user = Blueprint('user', __name__) @@ -49,7 +49,7 @@ def create_user(): def update_user(user_id): user_to_update = get_model_users(user_id=user_id) if not user_to_update: - return jsonify(result="error", message="User not found"), 404 + return _user_not_found(user_id) req_json = request.get_json() update_dct, errors = user_schema_load_json.load(req_json) @@ -118,7 +118,7 @@ def send_user_sms_code(user_id): user_to_send_to = get_model_users(user_id=user_id) if not user_to_send_to: - return jsonify(result="error", message="No user found"), 404 + return _user_not_found(user_id) verify_code, errors = request_verify_code_schema.load(request.get_json()) if errors: @@ -140,7 +140,7 @@ def send_user_sms_code(user_id): def send_user_email_code(user_id): user_to_send_to = get_model_users(user_id=user_id) if not user_to_send_to: - return jsonify(result="error", message="No user found"), 404 + return _user_not_found(user_id) verify_code, errors = request_verify_code_schema.load(request.get_json()) if errors: @@ -174,7 +174,7 @@ def set_permissions(user_id, service_id): # who is making this request has permission to make the request. user = get_model_users(user_id=user_id) if not user: - abort(404, 'User not found for id: {}'.format(user_id)) + _user_not_found(user_id) service = dao_fetch_service_by_id(service_id=service_id) if not service: abort(404, 'Service not found for id: {}'.format(service_id)) @@ -193,9 +193,45 @@ def get_by_email(): email = request.args.get('email') if not email: return jsonify(result="error", message="invalid request"), 400 - user = get_user_by_email(email) - if not user: - return jsonify(result="error", message="not found"), 404 - result = user_schema.dump(user) + fetched_user = get_user_by_email(email) + if not fetched_user: + return _user_not_found_for_email() + result = user_schema.dump(fetched_user) return jsonify(data=result.data) + + +@user.route('/reset-password', methods=['POST']) +def send_user_reset_password(): + email, errors = email_data_request_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + + user_to_send_to = get_user_by_email(email['email']) + if not user_to_send_to: + return _user_not_found_for_email() + + reset_password_message = {'to': user_to_send_to.email_address, + 'name': user_to_send_to.name, + 'reset_password_url': _create_reset_password_url(user_to_send_to.email_address)} + + email_reset_password.apply_async([encryption.encrypt(reset_password_message)], queue='email-reset-password') + + return jsonify({}), 204 + + +def _user_not_found(user_id): + return abort(404, 'User not found for id: {}'.format(user_id)) + + +def _user_not_found_for_email(): + return abort(404, 'User not found for email address') + + +def _create_reset_password_url(email): + from utils.url_safe_token import generate_token + import json + data = json.dumps({'email': email, 'created_at': str(datetime.now())}) + token = generate_token(data, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT']) + + return current_app.config['ADMIN_BASE_URL'] + '/new-password/' + token diff --git a/config.py b/config.py index fe8597a9f..617ae23dd 100644 --- a/config.py +++ b/config.py @@ -46,6 +46,7 @@ class Config(object): Queue('email', Exchange('default'), routing_key='email'), Queue('sms-code', Exchange('default'), routing_key='sms-code'), Queue('email-code', Exchange('default'), routing_key='email-code'), + Queue('email-reset-password', Exchange('default'), routing_key='email-reset-password'), 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'), diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 830c39e0b..1d96b0865 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,7 +1,13 @@ import uuid import pytest from flask import current_app -from app.celery.tasks import (send_sms, send_sms_code, send_email_code, send_email, process_job, email_invited_user) +from app.celery.tasks import (send_sms, + send_sms_code, + send_email_code, + send_email, + process_job, + email_invited_user, + email_reset_password) from app import (firetext_client, aws_ses_client, encryption, DATETIME_FORMAT) from app.clients.email.aws_ses import AwsSesClientException from app.clients.sms.firetext import FiretextClientException @@ -504,3 +510,22 @@ def test_email_invited_user_should_send_email(notify_api, mocker): invitation['to'], expected_subject, expected_content) + + +def test_email_reset_password_should_send_email(notify_api, mocker): + with notify_api.test_request_context(): + reset_password_message = {'to': 'someone@it.gov.uk', + 'name': 'Some One', + 'reset_password_url': 'bah'} + + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.encryption.decrypt', return_value=reset_password_message) + + encrypted_message = encryption.encrypt(reset_password_message) + email_reset_password(encrypted_message) + message = tasks.password_reset_message(reset_password_message['name'], + reset_password_message['reset_password_url']) + aws_ses_client.send_email(current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], + reset_password_message['to'], + "Reset password for GOV.UK Notify", + message) diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index c6299c740..2731efc82 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -90,7 +90,7 @@ def test_create_invited_user_invalid_email(notify_api, sample_service, mocker): assert response.status_code == 400 json_resp = json.loads(response.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == {'email_address': ['Invalid email']} + assert json_resp['message'] == {'email_address': ['Not a valid email address']} app.celery.tasks.email_invited_user.apply_async.assert_not_called() diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 1b84bc239..510068494 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -582,7 +582,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai app.celery.tasks.send_email.apply_async.assert_not_called() assert response.status_code == 400 assert data['result'] == 'error' - assert data['message']['to'][0] == 'Invalid email' + assert data['message']['to'][0] == 'Not a valid email address' def test_should_reject_email_notification_with_template_id_that_cant_be_found( diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index c014272a1..466cf0c50 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -1,10 +1,12 @@ import json +import uuid from flask import url_for +import app from app.models import (User, Permission, MANAGE_SETTINGS, MANAGE_TEMPLATES) from app.dao.permissions_dao import default_service_permissions -from app import db +from app import db, encryption from tests import create_authorization_header @@ -256,7 +258,7 @@ def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_us user = User.query.filter_by(id=sample_user.id).first() json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['result'] == "error" - assert json_resp['message'] == "User not found" + assert json_resp['message'] == "User not found for id: {}".format("9999") assert user == sample_user assert user.email_address != new_email @@ -284,7 +286,7 @@ def test_get_user_by_email(notify_api, notify_db, notify_db_session, sample_serv assert sorted(expected_permissions) == sorted(fetched['permissions'][str(sample_service.id)]) -def test_get_user_by_email_not_found_returns_400(notify_api, +def test_get_user_by_email_not_found_returns_404(notify_api, notify_db, notify_db_session, sample_user): @@ -297,7 +299,7 @@ def test_get_user_by_email_not_found_returns_400(notify_api, assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == 'not found' + assert json_resp['message'] == 'User not found for email address' def test_get_user_by_email_bad_url_returns_404(notify_api, @@ -426,3 +428,64 @@ def test_set_user_permissions_remove_old(notify_api, query = Permission.query.filter_by(user=sample_user) assert query.count() == 1 assert query.first().permission == MANAGE_SETTINGS + + +def test_send_user_reset_password_should_send_reset_password_link(notify_api, + sample_user, + mocker, + mock_encryption): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.email_reset_password.apply_async') + data = json.dumps({'email': sample_user.email_address}) + auth_header = create_authorization_header( + path=url_for('user.send_user_reset_password'), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + + assert resp.status_code == 204 + app.celery.tasks.email_reset_password.apply_async.assert_called_once_with(['something_encrypted'], + queue='email-reset-password') + + +def test_send_user_reset_password_should_return_400_when_user_doesnot_exist(notify_api, + mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + bad_email_address = 'bad@email.gov.uk' + data = json.dumps({'email': bad_email_address}) + auth_header = create_authorization_header( + path=url_for('user.send_user_reset_password'), + method='POST', + request_body=data) + + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + + assert resp.status_code == 404 + assert json.loads(resp.get_data(as_text=True))['message'] == 'User not found for email address' + + +def test_send_user_reset_password_should_return_400_when_data_is_not_email_address(notify_api, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + bad_email_address = 'bad.email.gov.uk' + data = json.dumps({'email': bad_email_address}) + auth_header = create_authorization_header( + path=url_for('user.send_user_reset_password'), + method='POST', + request_body=data) + + resp = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + + assert resp.status_code == 400 + assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address']} diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 98d9b65fd..b1564b372 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -10,8 +10,6 @@ from freezegun import freeze_time def test_user_verify_code_sms(notify_api, - notify_db, - notify_db_session, sample_sms_code): """ Tests POST endpoint '//verify/code' @@ -35,8 +33,6 @@ def test_user_verify_code_sms(notify_api, def test_user_verify_code_sms_missing_code(notify_api, - notify_db, - notify_db_session, sample_sms_code): """ Tests POST endpoint '//verify/code' @@ -59,8 +55,6 @@ def test_user_verify_code_sms_missing_code(notify_api, @moto.mock_sqs def test_user_verify_code_email(notify_api, - notify_db, - notify_db_session, sqs_client_conn, sample_email_code): """ @@ -85,8 +79,6 @@ def test_user_verify_code_email(notify_api, def test_user_verify_code_email_bad_code(notify_api, - notify_db, - notify_db_session, sample_email_code): """ Tests POST endpoint '//verify/code' @@ -110,8 +102,6 @@ def test_user_verify_code_email_bad_code(notify_api, def test_user_verify_code_email_expired_code(notify_api, - notify_db, - notify_db_session, sample_email_code): """ Tests POST endpoint '//verify/code' @@ -162,8 +152,6 @@ def test_user_verify_password(notify_api, def test_user_verify_password_invalid_password(notify_api, - notify_db, - notify_db_session, sample_user): """ Tests POST endpoint '//verify/password' invalid endpoint. @@ -189,8 +177,6 @@ def test_user_verify_password_invalid_password(notify_api, def test_user_verify_password_valid_password_resets_failed_logins(notify_api, - notify_db, - notify_db_session, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -227,8 +213,6 @@ def test_user_verify_password_valid_password_resets_failed_logins(notify_api, def test_user_verify_password_missing_password(notify_api, - notify_db, - notify_db_session, sample_user): """ Tests POST endpoint '//verify/password' missing password. @@ -314,7 +298,7 @@ def test_send_sms_code_returns_404_for_bad_input_data(notify_api, notify_db, not 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 user found' + assert json.loads(resp.get_data(as_text=True))['message'] == 'User not found for id: {}'.format(int(uuid_)) def test_send_user_email_code(notify_api, @@ -356,4 +340,4 @@ def test_send_user_email_code_returns_404_for_when_user_does_not_exist(notify_ap 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 user found' + assert json.loads(resp.get_data(as_text=True))['message'] == 'User not found for id: {}'.format(1)