From e9afad80edd537ac6f30c0ff07fb78c68f605e74 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 16 Oct 2018 12:42:23 +0100 Subject: [PATCH 1/3] Bump utils to 3.5.5 --- requirements-app.txt | 2 +- requirements.txt | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements-app.txt b/requirements-app.txt index 5c301e7ea..85fa52528 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -23,4 +23,4 @@ awscli-cwlogs>=1.4,<1.5 awscli==1.15.82 botocore<1.11.0 -git+https://github.com/alphagov/notifications-utils.git@30.5.3#egg=notifications-utils==30.5.3 +git+https://github.com/alphagov/notifications-utils.git@30.5.5#egg=notifications-utils==30.5.5 diff --git a/requirements.txt b/requirements.txt index 4bd2c4a44..79cb83cc2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,12 +25,12 @@ awscli-cwlogs>=1.4,<1.5 awscli==1.15.82 botocore<1.11.0 -git+https://github.com/alphagov/notifications-utils.git@30.5.3#egg=notifications-utils==30.5.3 +git+https://github.com/alphagov/notifications-utils.git@30.5.5#egg=notifications-utils==30.5.5 ## The following requirements were added by pip freeze: bleach==2.1.3 boto3==1.6.16 -certifi==2018.8.24 +certifi==2018.10.15 chardet==3.0.4 Click==7.0 colorama==0.3.9 @@ -63,14 +63,14 @@ python-dateutil==2.7.3 python-json-logger==0.1.8 PyYAML==3.13 redis==2.10.6 -requests==2.19.1 +requests==2.20.0 rsa==3.4.2 s3transfer==0.1.13 six==1.11.0 smartypants==2.0.1 statsd==3.2.2 texttable==1.4.0 -urllib3==1.23 +urllib3==1.24 webencodings==0.5.1 Werkzeug==0.14.1 WTForms==2.2.1 From f5014e83525e3c30b4f890f6e09fbb5a706f6afd Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 16 Oct 2018 15:08:53 +0100 Subject: [PATCH 2/3] Fix references to email HTML in tests --- tests/app/main/views/test_email_preview.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py index 55421cb7f..9ce465b0f 100644 --- a/tests/app/main/views/test_email_preview.py +++ b/tests/app/main/views/test_email_preview.py @@ -54,7 +54,7 @@ def test_displays_both_branding(client, mock_get_email_branding_with_both_brand_ assert page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png$")}) - assert page.select("body > table table > tr:nth-of-type(2) > td:nth-of-type(5)")[0]\ + assert page.select("body > table:nth-of-type(3) table > tr:nth-of-type(1) > td:nth-of-type(2)")[0]\ .get_text().strip() == 'Organisation text' # brand text is set @@ -71,7 +71,7 @@ def test_displays_org_branding(client, mock_get_email_branding): assert not page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png")}) assert not page.select("body > table > tr > td[bgcolor='#f00']") # banner colour is not set - assert page.select("body > table table > tr:nth-of-type(2) > td:nth-of-type(5)")[0]\ + assert page.select("body > table:nth-of-type(1) > tr:nth-of-type(1) > td:nth-of-type(2)")[0]\ .get_text().strip() == 'Organisation text' # brand text is set From 29c46a27f885f941c23f3dd93ae5614004b305fc Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 18 Oct 2018 14:34:07 +0100 Subject: [PATCH 3/3] 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)