From 29c46a27f885f941c23f3dd93ae5614004b305fc Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 18 Oct 2018 14:34:07 +0100 Subject: [PATCH] Stop url_for double-encoding .'s in password test One of the changes this pulls in is encoding of periods in the token used for new password requests. In real-life the resulting URL is build by concatenating the base url with the token so it isn't processed further. The test for new password requests builds the URL with url_for. This encodes the result returned so the periods are encoded twice. --- tests/app/main/views/test_new_password.py | 18 ++++++++++++------ tests/app/main/views/test_two_factor.py | 17 +++++++++++------ tests/app/main/views/test_user_profile.py | 4 +++- tests/conftest.py | 5 +++++ 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index d545638b3..7c88ea4ba 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -5,6 +5,8 @@ from flask import url_for from itsdangerous import SignatureExpired from notifications_utils.url_safe_token import generate_token +from tests.conftest import url_for_endpoint_with_token + def test_should_render_new_password_template( app_, @@ -17,7 +19,8 @@ def test_should_render_new_password_template( data = json.dumps({'email': api_user_active.email_address, 'created_at': str(datetime.utcnow())}) token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.get(url_for('.new_password', token=token)) + + response = client.get(url_for_endpoint_with_token('.new_password', token=token)) assert response.status_code == 200 assert 'You can now create a new password for your account.' in response.get_data(as_text=True) @@ -29,7 +32,7 @@ def test_should_return_404_when_email_address_does_not_exist( ): data = json.dumps({'email': 'no_user@d.gov.uk', 'created_at': str(datetime.utcnow())}) token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.get(url_for('.new_password', token=token)) + response = client.get(url_for_endpoint_with_token('.new_password', token=token)) assert response.status_code == 404 @@ -44,7 +47,8 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful( user = mock_get_user_by_email_request_password_reset.return_value data = json.dumps({'email': user.email_address, 'created_at': str(datetime.utcnow())}) token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + response = client.post(url_for_endpoint_with_token('.new_password', token=token), + data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.two_factor', _external=True) mock_get_user_by_email_request_password_reset.assert_called_once_with(user.email_address) @@ -61,7 +65,8 @@ def test_should_redirect_index_if_user_has_already_changed_password( user = mock_get_user_by_email_user_changed_password.return_value data = json.dumps({'email': user.email_address, 'created_at': str(datetime.utcnow())}) token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + response = client.post(url_for_endpoint_with_token('.new_password', token=token), + data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.index', _external=True) mock_get_user_by_email_user_changed_password.assert_called_once_with(user.email_address) @@ -76,7 +81,7 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp 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)) + response = client.get(url_for_endpoint_with_token('.new_password', token=token)) assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) @@ -97,7 +102,8 @@ def test_should_sign_in_when_password_reset_is_successful_for_email_auth( data = json.dumps({'email': user.email_address, 'created_at': str(datetime.utcnow())}) token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) - response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + response = client.post(url_for_endpoint_with_token('.new_password', token=token), + data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.show_accounts_or_dashboard', _external=True) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index c0e4aaf8b..dbc7c63b8 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,7 +1,12 @@ from bs4 import BeautifulSoup from flask import url_for -from tests.conftest import SERVICE_ONE_ID, normalize_spaces, set_config +from tests.conftest import ( + SERVICE_ONE_ID, + normalize_spaces, + set_config, + url_for_endpoint_with_token, +) def test_should_render_two_factor_page( @@ -206,7 +211,7 @@ def test_valid_two_factor_email_link_logs_in_user( mocker.patch('app.user_api_client.check_verify_code', return_value=(True, '')) response = client.get( - url_for('main.two_factor_email', token=valid_token), + url_for_endpoint_with_token('main.two_factor_email', token=valid_token), ) assert response.status_code == 302 @@ -223,7 +228,7 @@ def test_two_factor_email_link_has_expired( with set_config(app_, 'EMAIL_2FA_EXPIRY_SECONDS', -1): response = client.get( - url_for('main.two_factor_email', token=valid_token), + url_for_endpoint_with_token('main.two_factor_email', token=valid_token), follow_redirects=True, ) @@ -262,7 +267,7 @@ def test_two_factor_email_link_is_already_used( mocker.patch('app.user_api_client.check_verify_code', return_value=(False, 'Code has expired')) response = client.get( - url_for('main.two_factor_email', token=valid_token), + url_for_endpoint_with_token('main.two_factor_email', token=valid_token), follow_redirects=True ) @@ -282,7 +287,7 @@ def test_two_factor_email_link_when_user_is_locked_out( mocker.patch('app.user_api_client.check_verify_code', return_value=(False, 'Code not found')) response = client.get( - url_for('main.two_factor_email', token=valid_token), + url_for_endpoint_with_token('main.two_factor_email', token=valid_token), follow_redirects=True ) @@ -298,7 +303,7 @@ def test_two_factor_email_link_used_when_user_already_logged_in( valid_token ): response = logged_in_client.get( - url_for('main.two_factor_email', token=valid_token) + url_for_endpoint_with_token('main.two_factor_email', token=valid_token) ) assert response.status_code == 302 assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index d344c9b73..aad65ad82 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -6,6 +6,7 @@ from flask import url_for from notifications_utils.url_safe_token import generate_token from tests.conftest import api_user_active as create_user +from tests.conftest import url_for_endpoint_with_token def test_should_show_overview_page( @@ -104,7 +105,8 @@ def test_should_redirect_to_user_profile_when_user_confirms_email_link( token = generate_token(payload=json.dumps({'user_id': api_user_active.id, 'email': 'new_email@gov.uk'}), secret=app_.config['SECRET_KEY'], salt=app_.config['DANGEROUS_SALT']) - response = logged_in_client.get(url_for('main.user_profile_email_confirm', token=token)) + response = logged_in_client.get(url_for_endpoint_with_token('main.user_profile_email_confirm', + token=token)) assert response.status_code == 302 assert response.location == url_for('main.user_profile', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index cb43e4387..45edb97e6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3227,3 +3227,8 @@ def mock_create_event(mocker): return return mocker.patch('app.events_api_client.create_event', side_effect=_add_event) + + +def url_for_endpoint_with_token(endpoint, token): + token = token.replace('%2E', '.') + return url_for(endpoint, token=token)