From e91deff4480b97001a72c8f5240039134e24b1a7 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 5 Oct 2020 16:49:50 +0100 Subject: [PATCH 1/2] Put redirect link in reset password email link This is so when users reset their password they are still redirected to pages they were meant to visit. This change was done specifically so everyone who is meant to see broadcast tour sees it, but it will improve lives of all users who wanted to visit a page on Notify but then had to reset their password in the process --- app/user/rest.py | 14 ++++++++++---- tests/app/user/test_rest.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index e0115e57d..245ce643f 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -444,7 +444,10 @@ def send_user_reset_password(): service=service, personalisation={ 'user_name': user_to_send_to.name, - 'url': _create_reset_password_url(user_to_send_to.email_address) + 'url': _create_reset_password_url( + user_to_send_to.email_address, + next_redirect=request.get_json().get('next') + ) }, notification_type=template.template_type, api_key_id=None, @@ -477,10 +480,13 @@ def get_organisations_and_services_for_user(user_id): return jsonify(data) -def _create_reset_password_url(email): +def _create_reset_password_url(email, next_redirect=None): data = json.dumps({'email': email, 'created_at': str(datetime.utcnow())}) - url = '/new-password/' - return url_with_token(data, url, current_app.config) + static_url_part = '/new-password/' + full_url = url_with_token(data, static_url_part, current_app.config) + if next_redirect: + full_url += '?{}'.format(urlencode({'next': next_redirect})) + return full_url def _create_verification_url(user): diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 5a3440bb9..4c447d8c7 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -598,6 +598,24 @@ def test_send_user_reset_password_should_send_reset_password_link(client, assert notification.reply_to_text == notify_service.get_default_reply_to_email_address() +@freeze_time("2016-01-01 11:09:00.061258") +def test_send_user_reset_password_reset_password_link_contains_redirect_link_if_present_in_request( + client, sample_user, mocker, password_reset_email_template +): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = json.dumps({'email': sample_user.email_address, "next": "blob"}) + auth_header = create_authorization_header() + response = client.post( + url_for('user.send_user_reset_password'), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 204 + notification = Notification.query.first() + assert "?next=blob" in notification.content + mocked.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks") + + def test_send_user_reset_password_should_return_400_when_email_is_missing(client, mocker): mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = json.dumps({}) From 51811de919b39209dab0c07910b19163daa32512 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 9 Oct 2020 17:47:26 +0100 Subject: [PATCH 2/2] Improve variable names for readability Also next_redirect parameter in _create_reset_password_url does not have to be a default arg, so I removed that following review. --- app/user/rest.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 245ce643f..68cf78f1a 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -480,7 +480,7 @@ def get_organisations_and_services_for_user(user_id): return jsonify(data) -def _create_reset_password_url(email, next_redirect=None): +def _create_reset_password_url(email, next_redirect): data = json.dumps({'email': email, 'created_at': str(datetime.utcnow())}) static_url_part = '/new-password/' full_url = url_with_token(data, static_url_part, current_app.config) @@ -501,13 +501,13 @@ def _create_confirmation_url(user, email_address): return url_with_token(data, url, current_app.config) -def _create_2fa_url(user, secret_code, next_redir, email_auth_link_host): +def _create_2fa_url(user, secret_code, next_redirect, email_auth_link_host): data = json.dumps({'user_id': str(user.id), 'secret_code': secret_code}) url = '/email-auth/' - ret = url_with_token(data, url, current_app.config, base_url=email_auth_link_host) - if next_redir: - ret += '?{}'.format(urlencode({'next': next_redir})) - return ret + full_url = url_with_token(data, url, current_app.config, base_url=email_auth_link_host) + if next_redirect: + full_url += '?{}'.format(urlencode({'next': next_redirect})) + return full_url def get_orgs_and_services(user):