diff --git a/app/config.py b/app/config.py index 2cd1bc3fb..dce3d4104 100644 --- a/app/config.py +++ b/app/config.py @@ -41,7 +41,7 @@ class Config(object): 'local': 25000, 'nhs': 25000, } - EMAIL_EXPIRY_SECONDS = 3600 * 24 * 7 # one week + EMAIL_EXPIRY_SECONDS = 3600 # 1 hour HEADER_COLOUR = '#FFBF47' # $yellow HTTP_PROTOCOL = 'http' MAX_FAILED_LOGIN_COUNT = 10 @@ -56,7 +56,6 @@ class Config(object): SHOW_STYLEGUIDE = True # TODO: move to utils SMS_CHAR_COUNT_LIMIT = 459 - TOKEN_MAX_AGE_SECONDS = 3600 WTF_CSRF_ENABLED = True WTF_CSRF_TIME_LIMIT = None CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index c615c5ea8..84f5051d8 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,20 +1,20 @@ +from datetime import datetime import json from flask import (render_template, url_for, redirect, flash, session, current_app) from itsdangerous import SignatureExpired +from notifications_utils.url_safe_token import check_token +from app import user_api_client from app.main import main from app.main.forms import NewPasswordForm -from datetime import datetime -from app import user_api_client @main.route('/new-password/', methods=['GET', 'POST']) def new_password(token): - from notifications_utils.url_safe_token import check_token try: token_data = check_token(token, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT'], - current_app.config['TOKEN_MAX_AGE_SECONDS']) + current_app.config['EMAIL_EXPIRY_SECONDS']) except SignatureExpired: flash('The link in the email we sent you has expired. Enter your email address to resend.') return redirect(url_for('.forgot_password')) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index a3163a538..e94af163e 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -50,34 +50,26 @@ def verify(): @main.route('/verify-email/') def verify_email(token): try: - token_data = check_token(token, - current_app.config['SECRET_KEY'], - current_app.config['DANGEROUS_SALT'], - current_app.config['EMAIL_EXPIRY_SECONDS']) - - token_data = json.loads(token_data) - verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email') - user = user_api_client.get_user(token_data['user_id']) - if not user: - abort(404) - - if user.is_active: - flash("That verification link has expired.") - return redirect(url_for('main.sign_in')) - - session['user_details'] = {"email": user.email_address, "id": user.id} - if verified[0]: - user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) - return redirect('verify') - else: - if verified[1] == 'Code has expired': - flash("The link in the email we sent you has expired. We've sent you a new one.") - return redirect(url_for('main.resend_email_verification')) - else: - message = "There was a problem verifying your account. Error message: '{}'".format(verified[1]) - flash(message) - return redirect(url_for('main.index')) - + token_data = check_token( + token, + current_app.config['SECRET_KEY'], + current_app.config['DANGEROUS_SALT'], + current_app.config['EMAIL_EXPIRY_SECONDS'] + ) except SignatureExpired: - flash('The link in the email we sent you has expired') + flash("The link in the email we sent you has expired. We've sent you a new one.") return redirect(url_for('main.resend_email_verification')) + + # token contains json blob of format: {'user_id': '...', 'secret_code': '...'} (secret_code is unused) + token_data = json.loads(token_data) + user = user_api_client.get_user(token_data['user_id']) + if not user: + abort(404) + + if user.is_active: + flash("That verification link has expired.") + return redirect(url_for('main.sign_in')) + + session['user_details'] = {"email": user.email_address, "id": user.id} + user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) + return redirect('verify') diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index e18601970..6efbeec87 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,6 +1,7 @@ import json from datetime import datetime +from itsdangerous import SignatureExpired from flask import url_for from notifications_utils.url_safe_token import generate_token @@ -69,13 +70,13 @@ def test_should_redirect_index_if_user_has_already_changed_password( def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired( app_, client, - mock_get_user_by_email_request_password_reset, mock_login, + mocker ): - app_.config['TOKEN_MAX_AGE_SECONDS'] = -1000 - user = mock_get_user_by_email_request_password_reset.return_value - token = generate_token(user.email_address, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + mocker.patch('app.main.views.new_password.check_token', side_effect=SignatureExpired('expired')) + token = generate_token('foo@bar.com', app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) + + response = client.get(url_for('.new_password', token=token)) + assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) - app_.config['TOKEN_MAX_AGE_SECONDS'] = 3600 diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index ce27250b3..5cda6794f 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -1,6 +1,7 @@ import uuid import json +from itsdangerous import SignatureExpired from flask import url_for from bs4 import BeautifulSoup @@ -97,7 +98,7 @@ def test_verify_email_redirects_to_verify_if_token_valid( mock_send_verify_code, mock_check_verify_code, ): - token_data = {"user_id": api_user_pending.id, "secret_code": 12345} + token_data = {"user_id": api_user_pending.id, "secret_code": 'UNUSED'} mocker.patch('app.main.views.verify.check_token', return_value=json.dumps(token_data)) with client.session_transaction() as session: @@ -108,39 +109,20 @@ def test_verify_email_redirects_to_verify_if_token_valid( assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) + assert not mock_check_verify_code.called + mock_send_verify_code.assert_called_once_with(api_user_pending.id, 'sms', api_user_pending.mobile_number) + + with client.session_transaction() as session: + assert session['user_details'] == {'email': api_user_pending.email_address, 'id': api_user_pending.id} + def test_verify_email_redirects_to_email_sent_if_token_expired( client, mocker, api_user_pending, - mock_check_verify_code, ): - from itsdangerous import SignatureExpired mocker.patch('app.main.views.verify.check_token', side_effect=SignatureExpired('expired')) - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} - - response = client.get(url_for('main.verify_email', token='notreal')) - - assert response.status_code == 302 - assert response.location == url_for('main.resend_email_verification', _external=True) - - -def test_verify_email_redirects_to_email_sent_if_token_used( - client, - mocker, - api_user_pending, - mock_get_user_pending, - mock_send_verify_code, - mock_check_verify_code_code_expired, -): - from itsdangerous import SignatureExpired - mocker.patch('app.main.views.verify.check_token', side_effect=SignatureExpired('expired')) - - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} - response = client.get(url_for('main.verify_email', token='notreal')) assert response.status_code == 302 @@ -158,9 +140,6 @@ def test_verify_email_redirects_to_sign_in_if_user_active( token_data = {"user_id": api_user_active.id, "secret_code": 12345} mocker.patch('app.main.views.verify.check_token', return_value=json.dumps(token_data)) - with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} - response = client.get(url_for('main.verify_email', token='notreal'), follow_redirects=True) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text == 'Sign in'