diff --git a/app/templates/views/cancelled-invitation.html b/app/templates/views/cancelled-invitation.html index f18ebcfe0..5aae6c68f 100644 --- a/app/templates/views/cancelled-invitation.html +++ b/app/templates/views/cancelled-invitation.html @@ -1,5 +1,5 @@ {% extends "withoutnav_template.html" %} -{% block per_page_title %}Invitation has been cancelled{% endblock %} +{% block per_page_title %}The invitation you were sent has been cancelled{% endblock %} {% block maincolumn_content %}
diff --git a/app/templates/views/forgot-password.html b/app/templates/views/forgot-password.html index 8364f1f30..fe73b9524 100644 --- a/app/templates/views/forgot-password.html +++ b/app/templates/views/forgot-password.html @@ -3,7 +3,7 @@ {% from "components/form.html" import form_wrapper %} {% block per_page_title %} -Create a new password + Forgotten your password? {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/two-factor-sms.html b/app/templates/views/two-factor-sms.html index 64f575a12..c4863351e 100644 --- a/app/templates/views/two-factor-sms.html +++ b/app/templates/views/two-factor-sms.html @@ -3,7 +3,7 @@ {% from "components/form.html" import form_wrapper %} {% block per_page_title %} - Text verification + Check your phone {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/verification-not-received.html b/app/templates/views/verification-not-received.html index 085b9c210..8eab295ea 100644 --- a/app/templates/views/verification-not-received.html +++ b/app/templates/views/verification-not-received.html @@ -2,7 +2,7 @@ {% from "components/button/macro.njk" import govukButton %} {% block per_page_title %} - Resend verification code + Resend security code {% endblock %} {% block maincolumn_content %} diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index 8f9b2237d..7c2e14df8 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -1,14 +1,14 @@ import pytest -from bs4 import BeautifulSoup from flask import Response, url_for from flask_wtf.csrf import CSRFError from notifications_python_client.errors import HTTPError -def test_bad_url_returns_page_not_found(client): - response = client.get('/bad_url') - assert response.status_code == 404 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') +def test_bad_url_returns_page_not_found(client_request): + page = client_request.get_url( + '/bad_url', + _expected_status=404, + ) assert page.h1.string.strip() == 'Page not found' assert page.title.string.strip() == 'Page not found – GOV.UK Notify' @@ -55,20 +55,19 @@ def test_csrf_returns_400(client_request, mocker): assert page.title.string.strip() == 'Sorry, there’s a problem with the service – GOV.UK Notify' -def test_csrf_redirects_to_sign_in_page_if_not_signed_in(client, mocker): +def test_csrf_redirects_to_sign_in_page_if_not_signed_in(client_request, mocker): csrf_err = CSRFError('400 Bad Request: The CSRF tokens do not match.') mocker.patch('app.main.views.index.render_template', side_effect=csrf_err) - response = client.get('/cookies') - - assert response.status_code == 302 - assert response.location == url_for('main.sign_in', next='/cookies', _external=True) + client_request.logout() + client_request.get_url( + '/cookies', + _expected_redirect=url_for('main.sign_in', next='/cookies', _external=True), + ) -def test_405_returns_something_went_wrong_page(client, mocker): - response = client.post('/') +def test_405_returns_something_went_wrong_page(client_request, mocker): + page = client_request.post_url('/', _expected_status=405) - assert response.status_code == 405 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Sorry, there’s a problem with GOV.UK Notify' assert page.title.string.strip() == 'Sorry, there’s a problem with the service – GOV.UK Notify' diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py index 42938a5c2..261ad56a8 100644 --- a/tests/app/main/views/accounts/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -2,7 +2,6 @@ import uuid from itertools import repeat import pytest -from bs4 import BeautifulSoup from flask import url_for from tests.conftest import SERVICE_ONE_ID, SERVICE_TWO_ID, normalize_spaces @@ -240,13 +239,12 @@ def test_choose_account_should_show_back_to_service_link( def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_session( - client, client_request, mock_get_orgs_and_services, mock_get_organisation, mock_get_organisation_services, ): - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['service_id'] = None page = client_request.get('main.choose_account') @@ -254,13 +252,14 @@ def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_se def test_choose_account_should_not_show_back_to_service_link_if_not_signed_in( - client, + client_request, mock_get_service, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['service_id'] = SERVICE_ONE_ID - response = client.get(url_for('main.sign_in')) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get('main.sign_in') assert page.select_one('h1').text == 'Sign in' # We’re not signed in assert page.select_one('.navigation-service a') is None diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py index 87ca79bf6..ce8cfbb5b 100644 --- a/tests/app/main/views/organisations/test_organisation_invites.py +++ b/tests/app/main/views/organisations/test_organisation_invites.py @@ -2,7 +2,6 @@ from datetime import datetime, timedelta from unittest.mock import ANY import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time @@ -145,16 +144,21 @@ def test_accepted_invite_when_other_user_already_logged_in( def test_cancelled_invite_opened_by_user( - client, + mocker, + client_request, + api_user_active, mock_check_org_cancelled_invite_token, mock_get_organisation, - mock_get_user, fake_uuid ): - response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken'), follow_redirects=True) + client_request.logout() + mock_get_user = mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get( + 'main.accept_org_invite', + token='thisisnotarealtoken', + _follow_redirects=True, + ) assert normalize_spaces( page.select_one('h1').text @@ -171,22 +175,24 @@ def test_cancelled_invite_opened_by_user( def test_user_invite_already_accepted( - client, + client_request, mock_check_org_accepted_invite_token ): - response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id=ORGANISATION_ID, - _external=True + client_request.logout() + client_request.get( + 'main.accept_org_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id=ORGANISATION_ID, + _external=True, + ), ) @freeze_time('2021-12-12 12:12:12') def test_existing_user_invite_already_is_member_of_organisation( - client, + client_request, mock_check_org_invite_token, mock_get_user, mock_get_user_by_email, @@ -196,13 +202,16 @@ def test_existing_user_invite_already_is_member_of_organisation( mock_add_user_to_organisation, mock_update_user_attribute, ): - response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id=ORGANISATION_ID, - _external=True + client_request.logout() + mock_update_user_attribute.reset_mock() + client_request.get( + 'main.accept_org_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id=ORGANISATION_ID, + _external=True + ), ) mock_check_org_invite_token.assert_called_once_with('thisisnotarealtoken') @@ -217,7 +226,7 @@ def test_existing_user_invite_already_is_member_of_organisation( @freeze_time('2021-12-12 12:12:12') def test_existing_user_invite_not_a_member_of_organisation( - client, + client_request, api_user_active, mock_check_org_invite_token, mock_get_user_by_email, @@ -226,13 +235,16 @@ def test_existing_user_invite_not_a_member_of_organisation( mock_add_user_to_organisation, mock_update_user_attribute, ): - response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id=ORGANISATION_ID, - _external=True + client_request.logout() + mock_update_user_attribute.reset_mock() + client_request.get( + 'main.accept_org_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id=ORGANISATION_ID, + _external=True, + ), ) mock_check_org_invite_token.assert_called_once_with('thisisnotarealtoken') @@ -250,15 +262,17 @@ def test_existing_user_invite_not_a_member_of_organisation( def test_user_accepts_invite( - client, + client_request, mock_check_org_invite_token, mock_dont_get_user_by_email, mock_get_users_for_organisation, ): - response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken')) - - assert response.status_code == 302 - assert response.location == url_for('main.register_from_org_invite', _external=True) + client_request.logout() + client_request.get( + 'main.accept_org_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for('main.register_from_org_invite', _external=True) + ) mock_check_org_invite_token.assert_called_once_with('thisisnotarealtoken') mock_dont_get_user_by_email.assert_called_once_with('invited_user@test.gov.uk') @@ -266,10 +280,13 @@ def test_user_accepts_invite( def test_registration_from_org_invite_404s_if_user_not_in_session( - client, + client_request, ): - response = client.get(url_for('main.register_from_org_invite')) - assert response.status_code == 404 + client_request.logout() + client_request.get( + 'main.register_from_org_invite', + _expected_status=404, + ) @pytest.mark.parametrize('data, error', [ @@ -285,19 +302,24 @@ def test_registration_from_org_invite_404s_if_user_not_in_session( }, 'Choose a password that’s harder to guess'], ]) def test_registration_from_org_invite_has_bad_data( - client, + client_request, sample_org_invite, data, error, mock_get_invited_org_user_by_id, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['invited_org_user_id'] = sample_org_invite['id'] - response = client.post(url_for('main.register_from_org_invite'), data=data) + page = client_request.post( + 'main.register_from_org_invite', + _data=data, + _expected_status=200, + ) - assert response.status_code == 200 - assert error in response.get_data(as_text=True) + assert error in page.text @pytest.mark.parametrize('diff_data', [ @@ -306,12 +328,13 @@ def test_registration_from_org_invite_has_bad_data( ['email_address', 'organisation'] ]) def test_registration_from_org_invite_has_different_email_or_organisation( - client, + client_request, sample_org_invite, diff_data, mock_get_invited_org_user_by_id, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['invited_org_user_id'] = sample_org_invite['id'] data = { @@ -324,13 +347,15 @@ def test_registration_from_org_invite_has_different_email_or_organisation( for field in diff_data: data[field] = 'different' - response = client.post(url_for('main.register_from_org_invite'), data=data) - - assert response.status_code == 400 + client_request.post( + 'main.register_from_org_invite', + _data=data, + _expected_status=400, + ) def test_org_user_registers_with_email_already_in_use( - client, + client_request, sample_org_invite, mock_get_user_by_email, mock_accept_org_invite, @@ -339,19 +364,21 @@ def test_org_user_registers_with_email_already_in_use( mock_register_user, mock_get_invited_org_user_by_id, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['invited_org_user_id'] = sample_org_invite['id'] - response = client.post(url_for('main.register_from_org_invite'), data={ - 'name': 'Test User', - 'mobile_number': '+4407700900460', - 'password': 'validPassword!', - 'email_address': sample_org_invite['email_address'], - 'organisation': sample_org_invite['organisation'] - }) - - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) + client_request.post( + 'main.register_from_org_invite', + _data={ + 'name': 'Test User', + 'mobile_number': '+4407700900460', + 'password': 'validPassword!', + 'email_address': sample_org_invite['email_address'], + 'organisation': sample_org_invite['organisation'], + }, + _expected_redirect=url_for('main.verify', _external=True), + ) mock_get_user_by_email.assert_called_once_with( sample_org_invite['email_address'] @@ -361,7 +388,7 @@ def test_org_user_registers_with_email_already_in_use( def test_org_user_registration( - client, + client_request, sample_org_invite, mock_email_is_not_already_in_use, mock_register_user, @@ -372,19 +399,21 @@ def test_org_user_registration( mock_add_user_to_organisation, mock_get_invited_org_user_by_id, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['invited_org_user_id'] = sample_org_invite['id'] - response = client.post(url_for('main.register_from_org_invite'), data={ - 'name': 'Test User', - 'email_address': sample_org_invite['email_address'], - 'mobile_number': '+4407700900460', - 'password': 'validPassword!', - 'organisation': sample_org_invite['organisation'] - }) - - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) + client_request.post( + 'main.register_from_org_invite', + _data={ + 'name': 'Test User', + 'email_address': sample_org_invite['email_address'], + 'mobile_number': '+4407700900460', + 'password': 'validPassword!', + 'organisation': sample_org_invite['organisation'], + }, + _expected_redirect=url_for('main.verify', _external=True) + ) assert mock_get_user_by_email.called is False mock_register_user.assert_called_once_with( @@ -403,24 +432,26 @@ def test_org_user_registration( def test_verified_org_user_redirects_to_dashboard( - client, + client_request, sample_org_invite, mock_check_verify_code, mock_get_user, mock_activate_user, mock_login, ): + client_request.logout() invited_org_user = InvitedOrgUser(sample_org_invite).serialize() - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['expiry_date'] = str(datetime.utcnow() + timedelta(hours=1)) session['user_details'] = {"email": invited_org_user['email_address'], "id": invited_org_user['id']} session['organisation_id'] = invited_org_user['organisation'] - response = client.post(url_for('main.verify'), data={'sms_code': '12345'}) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id=invited_org_user['organisation'], - _external=True + client_request.post( + 'main.verify', + _data={'sms_code': '12345'}, + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id=invited_org_user['organisation'], + _external=True + ), ) diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 4b0ddc5ba..783133a9a 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -1,7 +1,6 @@ from unittest.mock import ANY, Mock, call import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time from notifications_python_client.errors import HTTPError @@ -33,7 +32,7 @@ def mock_check_invite_token(mocker, sample_invite): @freeze_time('2021-12-12 12:12:12') def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard( - client, + client_request, service_one, api_user_active, mock_check_invite_token, @@ -47,10 +46,15 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard( mock_get_user, mock_update_user_attribute, ): + client_request.logout() expected_service = service_one['id'] expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'} - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for('main.service_dashboard', service_id=expected_service, _external=True), + ) mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_get_existing_user_by_email.assert_called_with('invited_user@test.gov.uk') @@ -62,16 +66,13 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard( [], ) - assert response.status_code == 302 - assert response.location == url_for('main.service_dashboard', service_id=expected_service, _external=True) - @pytest.mark.parametrize('trial_mode, expected_endpoint', ( (True, '.broadcast_tour'), (False, '.broadcast_tour_live'), )) def test_broadcast_service_shows_tour( - client, + client_request, service_one, mock_check_invite_token, mock_get_existing_user_by_email, @@ -85,6 +86,7 @@ def test_broadcast_service_shows_tour( trial_mode, expected_endpoint, ): + client_request.logout() service_one['permissions'] = ['broadcast'] service_one['restricted'] = trial_mode @@ -92,21 +94,20 @@ def test_broadcast_service_shows_tour( 'data': service_one, }) - response = client.get(url_for( + client_request.get( 'main.accept_invite', - token='thisisnotarealtoken' - )) - assert response.status_code == 302 - assert response.location == url_for( - expected_endpoint, - service_id=SERVICE_ONE_ID, - step_index=1, - _external=True, + token='thisisnotarealtoken', + _expected_redirect=url_for( + expected_endpoint, + service_id=SERVICE_ONE_ID, + step_index=1, + _external=True, + ), ) def test_existing_user_with_no_permissions_or_folder_permissions_accept_invite( - client, + client_request, mocker, service_one, api_user_active, @@ -120,34 +121,44 @@ def test_existing_user_with_no_permissions_or_folder_permissions_accept_invite( mock_get_user, mock_update_user_attribute, ): + client_request.logout() + expected_service = service_one['id'] sample_invite['permissions'] = '' expected_permissions = set() expected_folder_permissions = [] mocker.patch('app.invite_api_client.accept_invite', return_value=sample_invite) - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_status=302, + ) mock_add_user_to_service.assert_called_with(expected_service, api_user_active['id'], expected_permissions, expected_folder_permissions) - assert response.status_code == 302 - def test_if_existing_user_accepts_twice_they_redirect_to_sign_in( - client, + client_request, mocker, sample_invite, mock_check_invite_token, mock_get_service, mock_update_user_attribute, ): + client_request.logout() + # Logging out updates the current session ID to `None` + mock_update_user_attribute.reset_mock() sample_invite['status'] = 'accepted' - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _follow_redirects=True, + ) + assert ( page.h1.string, page.select('main p')[0].text.strip(), @@ -237,7 +248,7 @@ def test_accepting_invite_removes_invite_from_session( @freeze_time('2021-12-12T12:12:12') def test_existing_user_of_service_get_redirected_to_signin( - client, + client_request, mocker, api_user_active, sample_invite, @@ -247,12 +258,16 @@ def test_existing_user_of_service_get_redirected_to_signin( mock_accept_invite, mock_update_user_attribute, ): + client_request.logout() sample_invite['email_address'] = api_user_active['email_address'] mocker.patch('app.models.user.Users.client_method', return_value=[api_user_active]) - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _follow_redirects=True, + ) + assert ( page.h1.string, page.select('main p')[0].text.strip(), @@ -264,7 +279,7 @@ def test_existing_user_of_service_get_redirected_to_signin( def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_part_of_the_service( - client, + client_request, mocker, api_user_active, sample_invite, @@ -276,6 +291,8 @@ def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_pa mock_get_user, mock_update_user_attribute, ): + client_request.logout() + mocker.patch('app.user_api_client.add_user_to_service', side_effect=HTTPError( response=Mock( status_code=400, @@ -287,12 +304,16 @@ def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_pa message=f"User id: {api_user_active['id']} already part of service id: {SERVICE_ONE_ID}" )) - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=False) - assert response.location == url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True) + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _follow_redirects=False, + _expected_redirect=url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True) + ) def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( - client, + client_request, service_one, api_user_active, sample_invite, @@ -307,10 +328,15 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( mock_get_user, mock_update_user_attribute, ): + client_request.logout() expected_service = service_one['id'] expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'} - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) + page = client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _follow_redirects=True, + ) mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_get_existing_user_by_email.assert_called_with('invited_user@test.gov.uk') @@ -319,9 +345,6 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( expected_permissions, sample_invite['folder_permissions']) assert mock_accept_invite.call_count == 1 - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert ( page.h1.string, page.select('main p')[0].text.strip(), @@ -332,7 +355,7 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( def test_new_user_accept_invite_calls_api_and_redirects_to_registration( - client, + client_request, service_one, mock_check_invite_token, mock_dont_get_user_by_email, @@ -341,19 +364,19 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration( mock_get_service, mocker, ): - expected_redirect_location = 'http://localhost/register-from-invite' - - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + client_request.logout() + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_redirect='http://localhost/register-from-invite', + ) mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') - assert response.status_code == 302 - assert response.location == expected_redirect_location - def test_new_user_accept_invite_calls_api_and_views_registration_page( - client, + client_request, service_one, sample_invite, mock_check_invite_token, @@ -364,14 +387,17 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( mock_get_service, mocker, ): - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) + client_request.logout() + page = client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _follow_redirects=True, + ) mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') mock_get_invited_user_by_id.assert_called_once_with(sample_invite['id']) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Create an account' assert normalize_spaces(page.select_one('main p').text) == ( @@ -394,19 +420,20 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation( - client, + client_request, mock_get_user, mock_get_service, sample_invite, mock_check_invite_token, mock_update_user_attribute, ): + client_request.logout() + mock_update_user_attribute.reset_mock() sample_invite['status'] = 'cancelled' - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + page = client_request.get('main.accept_invite', token='thisisnotarealtoken') app.invite_api_client.check_token.assert_called_with('thisisnotarealtoken') - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'The invitation you were sent has been cancelled' # We don’t let people update `email_access_validated_at` using an # cancelled invite @@ -420,10 +447,11 @@ def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation def test_new_user_accept_invite_with_malformed_token( admin_endpoint, api_endpoint, - client, + client_request, service_one, mocker, ): + client_request.logout() mocker.patch(api_endpoint, side_effect=HTTPError( response=Mock( status_code=400, @@ -439,10 +467,7 @@ def test_new_user_accept_invite_with_malformed_token( message={'invitation': 'Something’s wrong with this link. Make sure you’ve copied the whole thing.'} )) - response = client.get(url_for(admin_endpoint, token='thisisnotarealtoken'), follow_redirects=True) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get(admin_endpoint, token='thisisnotarealtoken', _follow_redirects=True) assert normalize_spaces( page.select_one('.banner-dangerous').text @@ -450,7 +475,7 @@ def test_new_user_accept_invite_with_malformed_token( def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( - client, + client_request, service_one, sample_invite, api_user_active, @@ -466,12 +491,15 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( mock_get_service, mocker, ): + client_request.logout() expected_redirect_location = 'http://localhost/register-from-invite' - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) - with client.session_transaction() as session: - assert response.status_code == 302 - assert response.location == expected_redirect_location + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_redirect=expected_redirect_location, + ) + with client_request.session_transaction() as session: assert session.get('invited_user_id') == sample_invite['id'] data = {'service': sample_invite['service'], @@ -484,9 +512,11 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( } expected_redirect_location = 'http://localhost/verify' - response = client.post(url_for('main.register_from_invite'), data=data) - assert response.status_code == 302 - assert response.location == expected_redirect_location + client_request.post( + 'main.register_from_invite', + _data=data, + _expected_redirect=expected_redirect_location, + ) mock_send_verify_code.assert_called_once_with(ANY, 'sms', data['mobile_number']) mock_get_invited_user_by_id.assert_called_once_with(sample_invite['id']) @@ -554,7 +584,7 @@ def test_accept_invite_does_not_treat_email_addresses_as_case_sensitive( def test_new_invited_user_verifies_and_added_to_service( - client, + client_request, service_one, sample_invite, api_user_active, @@ -582,10 +612,14 @@ def test_new_invited_user_verifies_and_added_to_service( mock_create_event, mocker, ): + client_request.logout() + # visit accept token page - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) - assert response.status_code == 302 - assert response.location == url_for('main.register_from_invite', _external=True) + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_redirect=url_for('main.register_from_invite', _external=True), + ) # get redirected to register from invite data = { @@ -597,19 +631,24 @@ def test_new_invited_user_verifies_and_added_to_service( 'name': 'Invited User', 'auth_type': 'sms_auth' } - response = client.post(url_for('main.register_from_invite'), data=data) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) + client_request.post( + 'main.register_from_invite', + _data=data, + _expected_redirect=url_for('main.verify', _external=True), + ) # that sends user on to verify - response = client.post(url_for('main.verify'), data={'sms_code': '12345'}, follow_redirects=True) - assert response.status_code == 200 + page = client_request.post( + 'main.verify', + _data={'sms_code': '12345'}, + _follow_redirects=True, + ) # when they post codes back to admin user should be added to # service and sent on to dash board expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'} - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'invited_user_id' not in session new_user_id = session['user_id'] mock_add_user_to_service.assert_called_with(data['service'], new_user_id, expected_permissions, []) @@ -617,8 +656,6 @@ def test_new_invited_user_verifies_and_added_to_service( mock_check_verify_code.assert_called_once_with(new_user_id, '12345', 'sms') assert service_one['id'] == session['service_id'] - raw_html = response.data.decode('utf-8') - page = BeautifulSoup(raw_html, 'html.parser') assert page.find('h1').text == 'Dashboard' @@ -630,7 +667,7 @@ def test_new_invited_user_verifies_and_added_to_service( )) def test_new_invited_user_is_redirected_to_correct_place( mocker, - client, + client_request, sample_invite, mock_check_invite_token, mock_check_verify_code, @@ -645,6 +682,7 @@ def test_new_invited_user_is_redirected_to_correct_place( expected_endpoint, extra_args, ): + client_request.logout() mocker.patch('app.service_api_client.get_service', return_value={ 'data': service_json( sample_invite['service'], @@ -652,21 +690,27 @@ def test_new_invited_user_is_redirected_to_correct_place( permissions=service_permissions, ) }) - client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + client_request.get( + 'main.accept_invite', + token='thisisnotarealtoken', + _expected_status=302, + ) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'email': sample_invite['email_address'], 'id': sample_invite['id'], } - response = client.post(url_for('main.verify'), data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for( - expected_endpoint, - service_id=sample_invite['service'], - _external=True, - **extra_args + client_request.post( + 'main.verify', + _data={'sms_code': '12345'}, + _expected_redirect=url_for( + expected_endpoint, + service_id=sample_invite['service'], + _external=True, + **extra_args + ) ) diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index c3b2dd0ec..528467272 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -498,7 +498,7 @@ def test_confirm_agreement_page_persists( ('foo', 404), )) def test_show_public_agreement_page( - client, + client_request, mocker, endpoint, variant, @@ -508,8 +508,9 @@ def test_show_public_agreement_page( 'app.s3_client.s3_mou_client.get_s3_object', return_value=MockS3Object() ) - response = client.get(url_for( + client_request.logout() + client_request.get_response( endpoint, variant=variant, - )) - assert response.status_code == expected_status + _expected_status=expected_status, + ) diff --git a/tests/app/main/views/test_code_not_received.py b/tests/app/main/views/test_code_not_received.py index 6343dd271..4c95f0e79 100644 --- a/tests/app/main/views/test_code_not_received.py +++ b/tests/app/main/views/test_code_not_received.py @@ -1,25 +1,22 @@ import pytest -from bs4 import BeautifulSoup from flask import url_for from tests.conftest import SERVICE_ONE_ID def test_should_render_email_verification_resend_show_email_address_and_resend_verify_email( - client, + client_request, mocker, api_user_active, mock_get_user_by_email, mock_send_verify_email, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.get(url_for('main.resend_email_verification')) - assert response.status_code == 200 - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get('main.resend_email_verification') assert page.h1.string == 'Check your email' expected = "A new confirmation email has been sent to {}".format(api_user_active['email_address']) @@ -34,20 +31,19 @@ def test_should_render_email_verification_resend_show_email_address_and_resend_v f'/services/{SERVICE_ONE_ID}/templates', ]) def test_should_render_correct_resend_template_for_active_user( - client, + client_request, api_user_active, mock_get_user_by_email, mock_send_verify_code, redirect_url ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.get(url_for('main.check_and_resend_text_code', next=redirect_url)) - assert response.status_code == 200 + page = client_request.get('main.check_and_resend_text_code', next=redirect_url) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string == 'Resend security code' # there shouldn't be a form for updating mobile number assert page.find('form') is None @@ -58,22 +54,20 @@ def test_should_render_correct_resend_template_for_active_user( def test_should_render_correct_resend_template_for_pending_user( - client, + client_request, mocker, api_user_pending, mock_send_verify_code, ): - + client_request.logout() mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_pending) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_pending['id'], 'email': api_user_pending['email_address']} - response = client.get(url_for('main.check_and_resend_text_code')) - assert response.status_code == 200 + page = client_request.get('main.check_and_resend_text_code') - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string == 'Check your mobile number' expected = 'Check your mobile phone number is correct and then resend the security code.' @@ -91,7 +85,7 @@ def test_should_render_correct_resend_template_for_pending_user( '+1800-555-555', ]) def test_should_resend_verify_code_and_update_mobile_for_pending_user( - client, + client_request, mocker, api_user_pending, mock_update_user_attribute, @@ -99,16 +93,21 @@ def test_should_resend_verify_code_and_update_mobile_for_pending_user( phone_number_to_register_with, redirect_url ): + client_request.logout() + mock_update_user_attribute.reset_mock() mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_pending) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_pending['id'], 'email': api_user_pending['email_address']} - response = client.post(url_for('main.check_and_resend_text_code', next=redirect_url), - data={'mobile_number': phone_number_to_register_with}) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True, next=redirect_url) + + client_request.post( + 'main.check_and_resend_text_code', + next=redirect_url, + _data={'mobile_number': phone_number_to_register_with}, + _expected_redirect=url_for('main.verify', _external=True, next=redirect_url), + ) mock_update_user_attribute.assert_called_once_with( api_user_pending['id'], @@ -126,19 +125,22 @@ def test_should_resend_verify_code_and_update_mobile_for_pending_user( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_check_and_redirect_to_two_factor_if_user_active( - client, + client_request, api_user_active, mock_get_user_by_email, mock_send_verify_code, redirect_url ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.get(url_for('main.check_and_resend_verification_code', next=redirect_url)) - assert response.status_code == 302 - assert response.location == url_for('main.two_factor_sms', _external=True, next=redirect_url) + client_request.get( + 'main.check_and_resend_verification_code', + next=redirect_url, + _expected_redirect=url_for('main.two_factor_sms', _external=True, next=redirect_url) + ) @pytest.mark.parametrize('redirect_url', [ @@ -146,23 +148,26 @@ def test_check_and_redirect_to_two_factor_if_user_active( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_check_and_redirect_to_verify_if_user_pending( - client, + client_request, mocker, api_user_pending, mock_get_user_pending, mock_send_verify_code, redirect_url ): - + client_request.logout() mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_pending) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_pending['id'], 'email': api_user_pending['email_address']} - response = client.get(url_for('main.check_and_resend_verification_code', next=redirect_url)) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True, next=redirect_url) + + client_request.get( + 'main.check_and_resend_verification_code', + next=redirect_url, + _expected_redirect=url_for('main.verify', _external=True, next=redirect_url), + ) @pytest.mark.parametrize('endpoint', [ @@ -171,13 +176,14 @@ def test_check_and_redirect_to_verify_if_user_pending( 'main.check_and_resend_verification_code', ]) def test_redirect_to_sign_in_if_not_logged_in( - client, + client_request, endpoint, ): - response = client.get(url_for(endpoint)) - - assert response.location == url_for('main.sign_in', _external=True) - assert response.status_code == 302 + client_request.logout() + client_request.get( + endpoint, + _expected_redirect=url_for('main.sign_in', _external=True), + ) @pytest.mark.parametrize('redirect_url', [ @@ -185,20 +191,19 @@ def test_redirect_to_sign_in_if_not_logged_in( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_should_render_correct_email_not_received_template_for_active_user( - client, + client_request, api_user_active, mock_get_user_by_email, mock_send_verify_code, redirect_url ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.get(url_for('main.email_not_received', next=redirect_url)) - assert response.status_code == 200 + page = client_request.get('main.email_not_received', next=redirect_url) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string == 'Resend email link' # there shouldn't be a form for updating mobile number assert page.find('form') is None diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py index 9ce465b0f..da6ca3aa2 100644 --- a/tests/app/main/views/test_email_preview.py +++ b/tests/app/main/views/test_email_preview.py @@ -1,8 +1,6 @@ import re import pytest -from bs4 import BeautifulSoup -from flask import url_for @pytest.mark.parametrize( @@ -11,45 +9,33 @@ from flask import url_for ({'govuk_banner': 'false'}, 'false') ] ) -def test_renders(client, mocker, query_args, result): +def test_renders(client_request, mocker, query_args, result): mocker.patch('app.main.views.index.HTMLEmailTemplate.__str__', return_value='rendered') - response = client.get(url_for('main.email_template', **query_args)) + response = client_request.get_response('main.email_template', **query_args) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'rendered' -def test_displays_govuk_branding_by_default(client): +def test_displays_govuk_branding_by_default(client_request): - response = client.get(url_for('main.email_template')) - - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 + page = client_request.get('main.email_template', _test_page_title=False) assert page.find("a", attrs={"href": "https://www.gov.uk"}) -def test_displays_govuk_branding(client, mock_get_email_branding_with_govuk_brand_type): +def test_displays_govuk_branding(client_request, mock_get_email_branding_with_govuk_brand_type): - response = client.get(url_for('main.email_template', branding_style="1")) - - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 + page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) assert page.find("a", attrs={"href": "https://www.gov.uk"}) -def test_displays_both_branding(client, mock_get_email_branding_with_both_brand_type): +def test_displays_both_branding(client_request, mock_get_email_branding_with_both_brand_type): - response = client.get(url_for('main.email_template', branding_style="1")) + page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 mock_get_email_branding_with_both_brand_type.assert_called_once_with('1') assert page.find("a", attrs={"href": "https://www.gov.uk"}) @@ -58,14 +44,10 @@ def test_displays_both_branding(client, mock_get_email_branding_with_both_brand_ .get_text().strip() == 'Organisation text' # brand text is set -def test_displays_org_branding(client, mock_get_email_branding): - +def test_displays_org_branding(client_request, mock_get_email_branding): # mock_get_email_branding has 'brand_type' of 'org' - response = client.get(url_for('main.email_template', branding_style="1")) + page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 mock_get_email_branding.assert_called_once_with('1') assert not page.find("a", attrs={"href": "https://www.gov.uk"}) @@ -76,13 +58,10 @@ def test_displays_org_branding(client, mock_get_email_branding): def test_displays_org_branding_with_banner( - client, mock_get_email_branding_with_org_banner_brand_type): + client_request, mock_get_email_branding_with_org_banner_brand_type +): + page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) - response = client.get(url_for('main.email_template', branding_style="1")) - - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 mock_get_email_branding_with_org_banner_brand_type.assert_called_once_with('1') assert not page.find("a", attrs={"href": "https://www.gov.uk"}) @@ -93,14 +72,12 @@ def test_displays_org_branding_with_banner( def test_displays_org_branding_with_banner_without_brand_text( - client, mock_get_email_branding_without_brand_text): + client_request, mock_get_email_branding_without_brand_text +): # mock_get_email_branding_without_brand_text has 'brand_type' of 'org_banner' - response = client.get(url_for('main.email_template', branding_style="1")) + page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - - assert response.status_code == 200 mock_get_email_branding_without_brand_text.assert_called_once_with('1') assert not page.find("a", attrs={"href": "https://www.gov.uk"}) diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 5e8116c27..389fc9d92 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -2,7 +2,6 @@ from functools import partial from unittest.mock import ANY, PropertyMock import pytest -from bs4 import BeautifulSoup, element from flask import url_for from freezegun import freeze_time from notifications_utils.clients.zendesk.zendesk_client import ( @@ -133,9 +132,13 @@ def test_get_support_as_member_of_public( (QUESTION_TICKET_TYPE, 200), ('gripe', 404) ]) -def test_get_feedback_page(client, ticket_type, expected_status_code): - response = client.get(url_for('main.feedback', ticket_type=ticket_type)) - assert response.status_code == expected_status_code +def test_get_feedback_page(client_request, ticket_type, expected_status_code): + client_request.logout() + client_request.get( + 'main.feedback', + ticket_type=ticket_type, + _expected_status=expected_status_code, + ) @freeze_time('2016-12-12 12:00:00.000000') @@ -144,7 +147,8 @@ def test_get_feedback_page(client, ticket_type, expected_status_code): (QUESTION_TICKET_TYPE, 'question'), (GENERAL_TICKET_TYPE, 'question'), ]) -def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_type, zendesk_ticket_type): +def test_passed_non_logged_in_user_details_through_flow(client_request, mocker, ticket_type, zendesk_ticket_type): + client_request.logout() mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__') mock_send_ticket_to_zendesk = mocker.patch( 'app.main.views.feedback.zendesk_client.send_ticket_to_zendesk', @@ -153,18 +157,18 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t data = {'feedback': 'blah', 'name': 'Anne Example', 'email_address': 'anne@example.com'} - resp = client.post( - url_for('main.feedback', ticket_type=ticket_type), - data=data + client_request.post( + 'main.feedback', + ticket_type=ticket_type, + _data=data, + _expected_redirect=url_for( + 'main.thanks', + out_of_hours_emergency=False, + email_address_provided=True, + _external=True, + ), ) - assert resp.status_code == 302 - assert resp.location == url_for( - 'main.thanks', - out_of_hours_emergency=False, - email_address_provided=True, - _external=True, - ) mock_create_ticket.assert_called_once_with( ANY, subject='Notify feedback', @@ -265,7 +269,9 @@ def test_email_address_required_for_problems_and_questions( _data=data, _expected_status=200 ) - assert isinstance(page.find('span', {'class': 'govuk-error-message'}), element.Tag) + assert normalize_spaces(page.select_one('.govuk-error-message').text) == ( + 'Error: Cannot be empty' + ) @freeze_time('2016-12-12 12:00:00.000000') @@ -273,19 +279,20 @@ def test_email_address_required_for_problems_and_questions( PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE )) def test_email_address_must_be_valid_if_provided_to_support_form( - client, + client_request, mocker, ticket_type, ): - response = client.post( - url_for('main.feedback', ticket_type=ticket_type), - data={ + client_request.logout() + page = client_request.post( + 'main.feedback', + ticket_type=ticket_type, + _data={ 'feedback': 'blah', 'email_address': 'not valid', }, + _expected_status=200, ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces(page.select_one('span.govuk-error-message').text) == ( 'Error: Enter a valid email address' diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index f3a106da6..4d6567bac 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -7,11 +7,10 @@ from tests import user_json from tests.conftest import SERVICE_ONE_ID -def test_should_render_forgot_password(client): - response = client.get(url_for('.forgot_password')) - assert response.status_code == 200 - assert 'We’ll send you an email to create a new password.' \ - in response.get_data(as_text=True) +def test_should_render_forgot_password(client_request): + client_request.logout() + page = client_request.get('.forgot_password') + assert 'We’ll send you an email to create a new password.' in page.text @pytest.mark.parametrize('email_address', [ @@ -19,52 +18,55 @@ def test_should_render_forgot_password(client): 'someuser@notgovernment.com' ]) def test_should_redirect_to_password_reset_sent_for_valid_email( - client, + client_request, fake_uuid, email_address, mocker, ): + client_request.logout() sample_user = user_json(email_address=email_address) mocker.patch('app.user_api_client.send_reset_password_url', return_value=None) - response = client.post( - url_for('.forgot_password'), - data={'email_address': sample_user['email_address']}) - assert response.status_code == 200 - assert 'Click the link in the email to reset your password.' \ - in response.get_data(as_text=True) + page = client_request.post( + '.forgot_password', + _data={'email_address': sample_user['email_address']}, + _expected_status=200, + ) + assert 'Click the link in the email to reset your password.' in page.text app.user_api_client.send_reset_password_url.assert_called_once_with(sample_user['email_address'], next_string=None) def test_forgot_password_sends_next_link_with_reset_password_email_request( - client, + client_request, fake_uuid, mocker, ): + client_request.logout() sample_user = user_json(email_address='test@user.gov.uk') mocker.patch('app.user_api_client.send_reset_password_url', return_value=None) - response = client.post( + client_request.post_url( url_for('.forgot_password') + f"?next=/services/{SERVICE_ONE_ID}/templates", - data={'email_address': sample_user['email_address']}) - assert response.status_code == 200 + _data={'email_address': sample_user['email_address']}, + _expected_status=200, + ) app.user_api_client.send_reset_password_url.assert_called_once_with( sample_user['email_address'], next_string=f'/services/{SERVICE_ONE_ID}/templates' ) def test_should_redirect_to_password_reset_sent_for_missing_email( - client, + client_request, api_user_active, mocker, ): - + client_request.logout() mocker.patch('app.user_api_client.send_reset_password_url', side_effect=HTTPError(Response(status=404), 'Not found')) - response = client.post( - url_for('.forgot_password'), - data={'email_address': api_user_active['email_address']}) - assert response.status_code == 200 - assert 'Click the link in the email to reset your password.' \ - in response.get_data(as_text=True) + page = client_request.post( + '.forgot_password', + _data={'email_address': api_user_active['email_address']}, + _expected_status=200, + ) + assert 'Click the link in the email to reset your password.' in page.text app.user_api_client.send_reset_password_url.assert_called_once_with( api_user_active['email_address'], next_string=None ) diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py index 9bbba9f17..16acf2ca3 100644 --- a/tests/app/main/views/test_headers.py +++ b/tests/app/main/views/test_headers.py @@ -1,15 +1,15 @@ def test_owasp_useful_headers_set( - client, + client_request, mocker, mock_get_service_and_organisation_counts, ): + client_request.logout() mocker.patch('app.get_logo_cdn_domain', return_value='static-logos.test.com') - response = client.get('/') + response = client_request.get_response('.index') - assert response.status_code == 200 assert response.headers['X-Frame-Options'] == 'deny' assert response.headers['X-Content-Type-Options'] == 'nosniff' assert response.headers['X-XSS-Protection'] == '1; mode=block' @@ -31,15 +31,15 @@ def test_owasp_useful_headers_set( def test_headers_non_ascii_characters_are_replaced( - client, + client_request, mocker, mock_get_service_and_organisation_counts, ): + client_request.logout() mocker.patch('app.get_logo_cdn_domain', return_value='static-logos€æ.test.com') - response = client.get('/') + response = client_request.get_response('.index') - assert response.status_code == 200 assert response.headers['Content-Security-Policy'] == ( "default-src 'self' static.example.com 'unsafe-inline';" "script-src 'self' static.example.com *.google-analytics.com 'unsafe-inline' 'unsafe-eval' data:;" diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 1b7a43b58..78deab351 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -10,13 +10,11 @@ from tests.conftest import SERVICE_ONE_ID, normalize_spaces, sample_uuid def test_non_logged_in_user_can_see_homepage( - client, + client_request, mock_get_service_and_organisation_counts, ): - response = client.get(url_for('main.index')) - assert response.status_code == 200 - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.logout() + page = client_request.get('main.index', _test_page_title=False) assert page.h1.text.strip() == ( 'Send emails, text messages and letters to your users' @@ -80,14 +78,16 @@ def test_robots(client_request): )) @freeze_time('2012-12-12 12:12') # So we don’t go out of business hours def test_hiding_pages_from_search_engines( - client, + client_request, mock_get_service_and_organisation_counts, endpoint, kwargs, ): - response = client.get(url_for(f'main.{endpoint}', **kwargs)) + client_request.logout() + response = client_request.get_response(f'main.{endpoint}', **kwargs) assert 'X-Robots-Tag' in response.headers assert response.headers['X-Robots-Tag'] == 'noindex' + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select_one('meta[name=robots]')['content'] == 'noindex' @@ -167,15 +167,18 @@ def test_guidance_pages_link_to_service_pages_when_signed_in( ('who_its_for', 'who_can_use_notify'), ]) def test_old_static_pages_redirect( - client, + client_request, view, expected_view ): - response = client.get(url_for('main.{}'.format(view))) - assert response.status_code == 301 - assert response.location == url_for( - 'main.{}'.format(expected_view), - _external=True + client_request.logout() + client_request.get( + 'main.{}'.format(view), + _expected_status=301, + _expected_redirect=url_for( + 'main.{}'.format(expected_view), + _external=True, + ), ) @@ -306,11 +309,11 @@ def test_letter_template_preview_links_to_the_correct_image( def test_letter_template_preview_headers( - client, + client_request, mock_get_letter_branding_by_id, ): - response = client.get( - url_for('main.letter_template', branding_style='hm-government') + response = client_request.get_response( + 'main.letter_template', branding_style='hm-government' ) assert response.headers.get('X-Frame-Options') == 'SAMEORIGIN' diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index df94dae67..39fbc3fca 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -14,10 +14,11 @@ from tests.conftest import SERVICE_ONE_ID, url_for_endpoint_with_token def test_should_render_new_password_template( mocker, notify_admin, - client, + client_request, mock_send_verify_code, mock_get_user_by_email_request_password_reset, ): + client_request.logout() user = mock_get_user_by_email_request_password_reset.return_value user['password_changed_at'] = '2021-01-01 00:00:00' mock_update_user_attribute = mocker.patch( @@ -28,9 +29,8 @@ def test_should_render_new_password_template( token = generate_token(data, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - 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) + page = client_request.get_url(url_for_endpoint_with_token('.new_password', token=token)) + assert 'You can now create a new password for your account.' in page.text mock_update_user_attribute.assert_called_once_with( user['id'], @@ -40,13 +40,16 @@ def test_should_render_new_password_template( def test_should_return_404_when_email_address_does_not_exist( notify_admin, - client, + client_request, mock_get_user_by_email_not_found, ): + client_request.logout() data = json.dumps({'email': 'no_user@d.gov.uk', 'created_at': str(datetime.utcnow())}) token = generate_token(data, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - response = client.get(url_for_endpoint_with_token('.new_password', token=token)) - assert response.status_code == 404 + client_request.get_url( + url_for_endpoint_with_token('.new_password', token=token), + _expected_status=404, + ) @pytest.mark.parametrize('redirect_url', [ @@ -55,20 +58,22 @@ def test_should_return_404_when_email_address_does_not_exist( ]) def test_should_redirect_to_two_factor_when_password_reset_is_successful( notify_admin, - client, + client_request, mock_get_user_by_email_request_password_reset, mock_login, mock_send_verify_code, mock_reset_failed_login_count, redirect_url ): + client_request.logout() 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, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - response = client.post(url_for_endpoint_with_token('.new_password', token=token, next=redirect_url), - data={'new_password': 'a-new_password'}) - assert response.status_code == 302 - assert response.location == url_for('.two_factor_sms', _external=True, next=redirect_url) + client_request.post_url( + url_for_endpoint_with_token('.new_password', token=token, next=redirect_url), + _data={'new_password': 'a-new_password'}, + _expected_redirect=url_for('.two_factor_sms', _external=True, next=redirect_url), + ) mock_get_user_by_email_request_password_reset.assert_called_once_with(user['email_address']) @@ -78,20 +83,22 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful( ]) def test_should_redirect_to_two_factor_webauthn_when_password_reset_is_successful( notify_admin, - client, + client_request, mock_get_user_by_email_request_password_reset, mock_send_verify_code, mock_reset_failed_login_count, redirect_url ): + client_request.logout() user = mock_get_user_by_email_request_password_reset.return_value user['auth_type'] = 'webauthn_auth' data = json.dumps({'email': user['email_address'], 'created_at': str(datetime.utcnow())}) token = generate_token(data, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - response = client.post(url_for_endpoint_with_token('.new_password', token=token, next=redirect_url), - data={'new_password': 'a-new_password'}) - assert response.status_code == 302 - assert response.location == url_for('.two_factor_webauthn', _external=True, next=redirect_url) + client_request.post_url( + url_for_endpoint_with_token('.new_password', token=token, next=redirect_url), + _data={'new_password': 'a-new_password'}, + _expected_redirect=url_for('.two_factor_webauthn', _external=True, next=redirect_url), + ) mock_get_user_by_email_request_password_reset.assert_called_once_with(user['email_address']) assert not mock_send_verify_code.called @@ -100,57 +107,64 @@ def test_should_redirect_to_two_factor_webauthn_when_password_reset_is_successfu def test_should_redirect_index_if_user_has_already_changed_password( notify_admin, - client, + client_request, mock_get_user_by_email_user_changed_password, mock_login, mock_send_verify_code, mock_reset_failed_login_count ): + client_request.logout() 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, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - 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) + client_request.post_url( + url_for_endpoint_with_token('.new_password', token=token), + _data={'new_password': 'a-new_password'}, + _expected_redirect=url_for('.index', _external=True), + ) mock_get_user_by_email_user_changed_password.assert_called_once_with(user['email_address']) def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired( notify_admin, - client, + client_request, mock_login, mocker ): + client_request.logout() mocker.patch('app.main.views.new_password.check_token', side_effect=SignatureExpired('expired')) token = generate_token('foo@bar.com', notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - 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) + client_request.get_url( + url_for_endpoint_with_token('.new_password', token=token), + _expected_redirect=url_for('.forgot_password', _external=True), + ) def test_should_sign_in_when_password_reset_is_successful_for_email_auth( + mocker, notify_admin, - client, - mock_get_user, + client_request, + api_user_active, mock_get_user_by_email_request_password_reset, mock_login, mock_send_verify_code, mock_reset_failed_login_count, mock_update_user_password ): + client_request.logout() user = mock_get_user_by_email_request_password_reset.return_value + mock_get_user = mocker.patch('app.user_api_client.get_user', return_value=api_user_active) user['auth_type'] = 'email_auth' data = json.dumps({'email': user['email_address'], 'created_at': str(datetime.utcnow())}) token = generate_token(data, notify_admin.config['SECRET_KEY'], notify_admin.config['DANGEROUS_SALT']) - response = client.post(url_for_endpoint_with_token('.new_password', token=token), - data={'new_password': 'a-new_password'}) + client_request.post_url( + url_for_endpoint_with_token('.new_password', token=token), + _data={'new_password': 'a-new_password'}, + _expected_redirect=url_for('.show_accounts_or_dashboard', _external=True), + ) - assert response.status_code == 302 - assert response.location == url_for('.show_accounts_or_dashboard', _external=True) assert mock_get_user_by_email_request_password_reset.called assert mock_reset_failed_login_count.called diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index c44d53d9d..b5674f2fc 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -25,12 +25,14 @@ from tests.conftest import SERVICE_ONE_ID, SERVICE_TWO_ID, normalize_spaces 'main.trial_services', ]) def test_should_redirect_if_not_logged_in( - client, + client_request, endpoint ): - response = client.get(url_for(endpoint)) - assert response.status_code == 302 - assert response.location == url_for('main.sign_in', next=url_for(endpoint), _external=True) + client_request.logout() + client_request.get( + endpoint, + _expected_redirect=url_for('main.sign_in', next=url_for(endpoint), _external=True), + ) @pytest.mark.parametrize('endpoint', [ diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index f8c974148..190fe7ddb 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -1,23 +1,23 @@ from unittest.mock import ANY import pytest -from bs4 import BeautifulSoup from flask import url_for from app.models.user import User from tests.conftest import normalize_spaces -def test_render_register_returns_template_with_form(client): - response = client.get('/register') +def test_render_register_returns_template_with_form( + client_request, +): + client_request.logout() + page = client_request.get_url('/register') - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.find('input', attrs={'name': 'auth_type'}).attrs['value'] == 'sms_auth' assert page.select_one('#email_address')['spellcheck'] == 'false' assert page.select_one('#email_address')['autocomplete'] == 'email' assert page.select_one('#password')['autocomplete'] == 'new-password' - assert 'Create an account' in response.get_data(as_text=True) + assert 'Create an account' in page.text def test_logged_in_user_redirects_to_account( @@ -39,7 +39,7 @@ def test_logged_in_user_redirects_to_account( ' the quick brown fox ', ]) def test_register_creates_new_user_and_redirects_to_continue_page( - client, + client_request, mock_send_verify_code, mock_register_user, mock_get_user_by_email_not_found, @@ -49,6 +49,7 @@ def test_register_creates_new_user_and_redirects_to_continue_page( phone_number_to_register_with, password, ): + client_request.logout() user_data = {'name': 'Some One Valid', 'email_address': 'notfound@example.gov.uk', 'mobile_number': phone_number_to_register_with, @@ -56,10 +57,12 @@ def test_register_creates_new_user_and_redirects_to_continue_page( 'auth_type': 'sms_auth' } - response = client.post(url_for('main.register'), data=user_data, follow_redirects=True) - assert response.status_code == 200 + page = client_request.post( + 'main.register', + _data=user_data, + _follow_redirects=True, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select('main p')[0].text == 'An email has been sent to notfound@example.gov.uk.' mock_send_verify_email.assert_called_with(ANY, user_data['email_address']) @@ -71,28 +74,35 @@ def test_register_creates_new_user_and_redirects_to_continue_page( def test_register_continue_handles_missing_session_sensibly( - client, + client_request, ): + client_request.logout() # session is not set - response = client.get(url_for('main.registration_continue')) - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) + client_request.get( + 'main.registration_continue', + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True), + ) def test_process_register_returns_200_when_mobile_number_is_invalid( - client, + client_request, mock_send_verify_code, mock_get_user_by_email_not_found, mock_login, ): - response = client.post(url_for('main.register'), - data={'name': 'Bad Mobile', - 'email_address': 'bad_mobile@example.gov.uk', - 'mobile_number': 'not good', - 'password': 'validPassword!'}) + client_request.logout() + page = client_request.post( + 'main.register', + _data={ + 'name': 'Bad Mobile', + 'email_address': 'bad_mobile@example.gov.uk', + 'mobile_number': 'not good', + 'password': 'validPassword!', + }, + _expected_status=200, + ) - assert response.status_code == 200 - assert 'Must not contain letters or symbols' in response.get_data(as_text=True) + assert 'Must not contain letters or symbols' in page.text def test_should_return_200_when_email_is_not_gov_uk( @@ -125,7 +135,7 @@ def test_should_return_200_when_email_is_not_gov_uk( pytest.param('example@ellipsis.com', marks=pytest.mark.xfail(raises=AssertionError)), )) def test_should_add_user_details_to_session( - client, + client_request, mock_send_verify_code, mock_register_user, mock_get_user_by_email_not_found, @@ -135,41 +145,47 @@ def test_should_add_user_details_to_session( mock_login, email_address, ): - response = client.post( - url_for('main.register'), - data={ + client_request.logout() + client_request.post( + 'main.register', + _data={ 'name': 'Test Codes', 'email_address': email_address, 'mobile_number': '+4407700900460', 'password': 'validPassword!' }, ) - assert response.status_code == 302 - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert session['user_details']['email'] == email_address def test_should_return_200_if_password_is_on_list_of_commonly_used_passwords( - client, + client_request, mock_get_user_by_email, mock_login, ): - response = client.post(url_for('main.register'), - data={'name': 'Bad Mobile', - 'email_address': 'bad_mobile@example.gov.uk', - 'mobile_number': '+44123412345', - 'password': 'password'}) + client_request.logout() + page = client_request.post( + 'main.register', + _data={ + 'name': 'Bad Mobile', + 'email_address': 'bad_mobile@example.gov.uk', + 'mobile_number': '+44123412345', + 'password': 'password', + }, + _expected_status=200, + ) - assert response.status_code == 200 - assert 'Choose a password that’s harder to guess' in response.get_data(as_text=True) + assert 'Choose a password that’s harder to guess' in page.text def test_register_with_existing_email_sends_emails( - client, + client_request, api_user_active, mock_get_user_by_email, mock_send_already_registered_email, ): + client_request.logout() user_data = { 'name': 'Already Hasaccount', 'email_address': api_user_active['email_address'], @@ -177,10 +193,11 @@ def test_register_with_existing_email_sends_emails( 'password': 'validPassword!' } - response = client.post(url_for('main.register'), - data=user_data) - assert response.status_code == 302 - assert response.location == url_for('main.registration_continue', _external=True) + client_request.post( + 'main.register', + _data=user_data, + _expected_redirect=url_for('main.registration_continue', _external=True), + ) @pytest.mark.parametrize('email_address, expected_value', [ @@ -254,7 +271,7 @@ def test_shows_hidden_email_address_on_registration_page_from_invite( {'username': 'anythingelse@example.com'}, )) def test_register_from_invite( - client, + client_request, fake_uuid, mock_email_is_not_already_in_use, mock_register_user, @@ -264,11 +281,12 @@ def test_register_from_invite( sample_invite, extra_data, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] - response = client.post( - url_for('main.register_from_invite'), - data=dict( + client_request.post( + 'main.register_from_invite', + _data=dict( name='Registered in another Browser', email_address=sample_invite['email_address'], mobile_number='+4407700900460', @@ -277,9 +295,8 @@ def test_register_from_invite( auth_type='sms_auth', **extra_data ), + _expected_redirect=url_for('main.verify', _external=True), ) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) mock_register_user.assert_called_once_with( 'Registered in another Browser', sample_invite['email_address'], @@ -291,34 +308,34 @@ def test_register_from_invite( def test_register_from_invite_when_user_registers_in_another_browser( - client, + client_request, api_user_active, mock_get_user_by_email, mock_accept_invite, mock_get_invited_user_by_id, sample_invite, ): + client_request.logout() sample_invite['email_address'] = api_user_active['email_address'] - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] - response = client.post( - url_for('main.register_from_invite'), - data={ + client_request.post( + 'main.register_from_invite', + _data={ 'name': 'Registered in another Browser', 'email_address': api_user_active['email_address'], 'mobile_number': api_user_active['mobile_number'], 'service': sample_invite['service'], 'password': 'somreallyhardthingtoguess', 'auth_type': 'sms_auth' - } + }, + _expected_redirect=url_for('main.verify', _external=True), ) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) @pytest.mark.parametrize('invite_email_address', ['gov-user@gov.uk', 'non-gov-user@example.com']) def test_register_from_email_auth_invite( - client, + client_request, sample_invite, mock_email_is_not_already_in_use, mock_register_user, @@ -335,10 +352,11 @@ def test_register_from_email_auth_invite( fake_uuid, mocker, ): + client_request.logout() mock_login_user = mocker.patch('app.models.user.login_user') sample_invite['auth_type'] = 'email_auth' sample_invite['email_address'] = invite_email_address - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] # Prove that the user isn’t already signed in assert 'user_id' not in session @@ -352,9 +370,15 @@ def test_register_from_email_auth_invite( 'auth_type': 'email_auth', } - resp = client.post(url_for('main.register_from_invite'), data=data) - assert resp.status_code == 302 - assert resp.location == url_for('main.service_dashboard', service_id=sample_invite['service'], _external=True) + client_request.post( + 'main.register_from_invite', + _data=data, + _expected_redirect=url_for( + 'main.service_dashboard', + service_id=sample_invite['service'], + _external=True, + ), + ) # doesn't send any 2fa code assert not mock_send_verify_email.called @@ -383,7 +407,7 @@ def test_register_from_email_auth_invite( [], ) - with client.session_transaction() as session: + with client_request.session_transaction() as session: # The user is signed in assert 'user_id' in session # invited user details are still there so they can get added to the service @@ -391,7 +415,7 @@ def test_register_from_email_auth_invite( def test_can_register_email_auth_without_phone_number( - client, + client_request, sample_invite, mock_email_is_not_already_in_use, mock_register_user, @@ -404,8 +428,9 @@ def test_can_register_email_auth_without_phone_number( mock_get_service, mock_get_invited_user_by_id, ): + client_request.logout() sample_invite['auth_type'] = 'email_auth' - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] data = { @@ -417,9 +442,15 @@ def test_can_register_email_auth_without_phone_number( 'auth_type': 'email_auth' } - resp = client.post(url_for('main.register_from_invite'), data=data) - assert resp.status_code == 302 - assert resp.location == url_for('main.service_dashboard', service_id=sample_invite['service'], _external=True) + client_request.post( + 'main.register_from_invite', + _data=data, + _expected_redirect=url_for( + 'main.service_dashboard', + service_id=sample_invite['service'], + _external=True, + ), + ) mock_register_user.assert_called_once_with( ANY, @@ -431,35 +462,38 @@ def test_can_register_email_auth_without_phone_number( def test_cannot_register_with_sms_auth_and_missing_mobile_number( - client, + client_request, mock_send_verify_code, mock_get_user_by_email_not_found, mock_login, ): - response = client.post(url_for('main.register'), - data={'name': 'Missing Mobile', - 'email_address': 'missing_mobile@example.gov.uk', - 'password': 'validPassword!'}) + client_request.logout() + page = client_request.post( + 'main.register', + _data={ + 'name': 'Missing Mobile', + 'email_address': 'missing_mobile@example.gov.uk', + 'password': 'validPassword!', + }, + _expected_status=200, + ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') err = page.select_one('.govuk-error-message') assert err.text.strip() == 'Error: Cannot be empty' assert err.attrs['data-error-label'] == 'mobile_number' def test_register_from_invite_form_doesnt_show_mobile_number_field_if_email_auth( - client, + client_request, sample_invite, mock_get_invited_user_by_id, ): + client_request.logout() sample_invite['auth_type'] = 'email_auth' - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] - response = client.get(url_for('main.register_from_invite')) + page = client_request.get('main.register_from_invite') - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.find('input', attrs={'name': 'auth_type'}).attrs['value'] == 'email_auth' assert page.find('input', attrs={'name': 'mobile_number'}) is None diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 2ddf888c3..6d63781d2 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -40,10 +40,10 @@ def test_render_sign_in_template_with_next_link_for_password_reset( assert forgot_password_link['href'] == url_for('main.forgot_password', next=f'/services/{SERVICE_ONE_ID}/templates') -def test_sign_in_explains_session_timeout(client): - response = client.get(url_for('main.sign_in', next='/foo')) - assert response.status_code == 200 - assert 'We signed you out because you have not used Notify for a while.' in response.get_data(as_text=True) +def test_sign_in_explains_session_timeout(client_request): + client_request.logout() + page = client_request.get('main.sign_in', next='/foo') + assert 'We signed you out because you have not used Notify for a while.' in page.text def test_sign_in_explains_other_browser(client_request, api_user_active, mocker): @@ -137,7 +137,7 @@ def test_logged_in_user_doesnt_do_evil_redirect( (' valid@example.gov.uk ', ' val1dPassw0rd! '), ]) def test_process_sms_auth_sign_in_return_2fa_template( - client, + client_request, api_user_active, mock_send_verify_code, mock_get_user, @@ -147,12 +147,16 @@ def test_process_sms_auth_sign_in_return_2fa_template( password, redirect_url ): - response = client.post( - url_for('main.sign_in', next=redirect_url), data={ + client_request.logout() + client_request.post( + 'main.sign_in', + next=redirect_url, + _data={ 'email_address': email_address, - 'password': password}) - assert response.status_code == 302 - assert response.location == url_for('.two_factor_sms', next=redirect_url, _external=True) + 'password': password, + }, + _expected_redirect=url_for('.two_factor_sms', next=redirect_url, _external=True), + ) mock_verify_password.assert_called_with(api_user_active['id'], password) mock_get_user_by_email.assert_called_with('valid@example.gov.uk') @@ -162,22 +166,27 @@ def test_process_sms_auth_sign_in_return_2fa_template( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_process_email_auth_sign_in_return_2fa_template( - client, + client_request, api_user_active_email_auth, mock_send_verify_code, mock_verify_password, mocker, redirect_url ): + client_request.logout() mocker.patch('app.user_api_client.get_user', return_value=api_user_active_email_auth) mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_active_email_auth) - response = client.post( - url_for('main.sign_in', next=redirect_url), data={ + client_request.post( + 'main.sign_in', + next=redirect_url, + _data={ 'email_address': 'valid@example.gov.uk', - 'password': 'val1dPassw0rd!'}) - assert response.status_code == 302 - assert response.location == url_for('.two_factor_email_sent', _external=True, next=redirect_url) + 'password': 'val1dPassw0rd!', + }, + _expected_redirect=url_for('.two_factor_email_sent', _external=True, next=redirect_url), + ) + mock_send_verify_code.assert_called_with(api_user_active_email_auth['id'], 'email', None, redirect_url) mock_verify_password.assert_called_with(api_user_active_email_auth['id'], 'val1dPassw0rd!') @@ -187,68 +196,78 @@ def test_process_email_auth_sign_in_return_2fa_template( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_process_webauthn_auth_sign_in_redirects_to_webauthn_with_next_redirect( - client, + client_request, api_user_active, mocker, mock_verify_password, redirect_url ): + client_request.logout() api_user_active['auth_type'] = 'webauthn_auth' mock_get_user_by_email = mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_active) - response = client.post( - url_for( - 'main.sign_in', next=redirect_url - ), - data={ + client_request.post( + 'main.sign_in', + next=redirect_url, + _data={ 'email_address': 'valid@example.gov.uk', - 'password': 'val1dPassw0rd!' - } + 'password': 'val1dPassw0rd!', + }, + _expected_redirect=url_for('.two_factor_webauthn', _external=True, next=redirect_url) ) + mock_get_user_by_email.assert_called_once_with('valid@example.gov.uk') - assert response.status_code == 302 - assert response.location == url_for('.two_factor_webauthn', _external=True, next=redirect_url) def test_should_return_locked_out_true_when_user_is_locked( - client, + client_request, mock_get_user_by_email_locked, ): - resp = client.post( - url_for('main.sign_in'), data={ + client_request.logout() + page = client_request.post( + 'main.sign_in', + _data={ 'email_address': 'valid@example.gov.uk', - 'password': 'whatIsMyPassword!'}) - assert resp.status_code == 200 - assert 'The email address or password you entered is incorrect' in resp.get_data(as_text=True) + 'password': 'whatIsMyPassword!', + }, + _expected_status=200, + ) + assert 'The email address or password you entered is incorrect' in page.text def test_should_return_200_when_user_does_not_exist( - client, + client_request, mock_get_user_by_email_not_found, ): - response = client.post( - url_for('main.sign_in'), data={ + client_request.logout() + page = client_request.post( + 'main.sign_in', + _data={ 'email_address': 'notfound@gov.uk', - 'password': 'doesNotExist!'}) - assert response.status_code == 200 - assert 'The email address or password you entered is incorrect' in response.get_data(as_text=True) + 'password': 'doesNotExist!' + }, + _expected_status=200, + ) + + assert 'The email address or password you entered is incorrect' in page.text def test_should_return_redirect_when_user_is_pending( - client, + client_request, mock_get_user_by_email_pending, api_user_pending, mock_verify_password, ): - response = client.post( - url_for('main.sign_in'), - data={ + client_request.logout() + client_request.post( + 'main.sign_in', + _data={ 'email_address': 'pending_user@example.gov.uk', 'password': 'val1dPassw0rd!' - } + }, + _expected_redirect=url_for('main.resend_email_verification', _external=True), ) - assert response.location == url_for('main.resend_email_verification', _external=True) - with client.session_transaction() as s: + with client_request.session_transaction() as s: assert s['user_details'] == { 'email': api_user_pending['email_address'], 'id': api_user_pending['id'] @@ -260,21 +279,25 @@ def test_should_return_redirect_when_user_is_pending( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_should_attempt_redirect_when_user_is_pending( - client, + client_request, mock_get_user_by_email_pending, mock_verify_password, redirect_url ): - response = client.post( - url_for('main.sign_in', next=redirect_url), data={ + client_request.logout() + client_request.post( + 'main.sign_in', + next=redirect_url, + _data={ 'email_address': 'pending_user@example.gov.uk', - 'password': 'val1dPassw0rd!'}) - assert response.location == url_for('main.resend_email_verification', _external=True, next=redirect_url) - assert response.status_code == 302 + 'password': 'val1dPassw0rd!' + }, + _expected_redirect=url_for('main.resend_email_verification', _external=True, next=redirect_url) + ) def test_email_address_is_treated_case_insensitively_when_signing_in_as_invited_user( - client, + client_request, mocker, mock_verify_password, api_user_active, @@ -283,6 +306,7 @@ def test_email_address_is_treated_case_insensitively_when_signing_in_as_invited_ mock_send_verify_code, mock_get_invited_user_by_id, ): + client_request.logout() sample_invite['email_address'] = 'TEST@user.gov.uk' mocker.patch( @@ -290,16 +314,18 @@ def test_email_address_is_treated_case_insensitively_when_signing_in_as_invited_ return_value=User(api_user_active), ) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['invited_user_id'] = sample_invite['id'] - response = client.post( - url_for('main.sign_in'), data={ + client_request.post( + 'main.sign_in', + _data={ 'email_address': 'test@user.gov.uk', - 'password': 'val1dPassw0rd!'}) + 'password': 'val1dPassw0rd!' + }, + ) assert mock_accept_invite.called - assert response.status_code == 302 assert mock_send_verify_code.called mock_get_invited_user_by_id.assert_called_once_with(sample_invite['id']) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 14e966827..6d36e819c 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,5 +1,4 @@ import pytest -from bs4 import BeautifulSoup from flask import url_for from tests.conftest import ( @@ -22,16 +21,19 @@ def mock_email_validated_recently(mocker): (True, 'Email resent') ]) def test_two_factor_email_sent_page( - client, + client_request, email_resent, page_title, redirect_url, request_url ): - response = client.get(url_for(f'main.{request_url}', next=redirect_url, email_resent=email_resent)) - assert response.status_code == 200 + client_request.logout() + page = client_request.get( + f'main.{request_url}', + next=redirect_url, + email_resent=email_resent, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string == page_title # there shouldn't be a form for updating mobile number assert page.find('form') is None @@ -45,22 +47,22 @@ def test_two_factor_email_sent_page( f'/services/{SERVICE_ONE_ID}/templates', ]) def test_should_render_two_factor_page( - client, + client_request, api_user_active, mock_get_user_by_email, mocker, redirect_url ): + client_request.logout() # TODO this lives here until we work out how to # reassign the session after it is lost mid register process - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - response = client.get(url_for('main.two_factor_sms', next=redirect_url)) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get('main.two_factor_sms', next=redirect_url) + assert page.select_one('main p').text.strip() == ( 'We’ve sent you a text message with a security code.' ) @@ -76,7 +78,7 @@ def test_should_render_two_factor_page( def test_should_login_user_and_should_redirect_to_next_url( - client, + client_request, api_user_active, mock_get_user, mock_get_user_by_email, @@ -84,23 +86,27 @@ def test_should_login_user_and_should_redirect_to_next_url( mock_create_event, mock_email_validated_recently, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.post(url_for('main.two_factor_sms', next='/services/{}'.format(SERVICE_ONE_ID)), - data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id=SERVICE_ONE_ID, - _external=True + client_request.post( + 'main.two_factor_sms', + next='/services/{}'.format(SERVICE_ONE_ID), + _data={'sms_code': '12345'}, + _expected_redirect=url_for( + 'main.service_dashboard', + service_id=SERVICE_ONE_ID, + _external=True + ), ) def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate_email( - client, + client_request, api_user_active, mock_get_user, mock_check_verify_code, @@ -108,26 +114,30 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate mock_send_verify_code, mocker ): + client_request.logout() + mocker.patch('app.user_api_client.get_user', return_value=api_user_active) mocker.patch('app.main.views.two_factor.email_needs_revalidating', return_value=True) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.post(url_for('main.two_factor_sms', next=f'/services/{SERVICE_ONE_ID}'), - data={'sms_code': '12345'}) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.revalidate_email_sent', - _external=True, - next=f'/services/{SERVICE_ONE_ID}' + client_request.post( + 'main.two_factor_sms', + next=f'/services/{SERVICE_ONE_ID}', + _data={'sms_code': '12345'}, + _expected_redirect=url_for( + 'main.revalidate_email_sent', + _external=True, + next=f'/services/{SERVICE_ONE_ID}' + ), ) + mock_send_verify_code.assert_called_with(api_user_active['id'], 'email', None, mocker.ANY) def test_should_login_user_and_not_redirect_to_external_url( - client, + client_request, api_user_active, mock_get_user, mock_get_user_by_email, @@ -136,22 +146,26 @@ def test_should_login_user_and_not_redirect_to_external_url( mock_create_event, mock_email_validated_recently, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.post(url_for('main.two_factor_sms', next='http://www.google.com'), - data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) + client_request.post( + 'main.two_factor_sms', + next='http://www.google.com', + _data={'sms_code': '12345'}, + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True) + ) @pytest.mark.parametrize('platform_admin', ( True, False, )) def test_should_login_user_and_redirect_to_show_accounts( - client, + client_request, api_user_active, mock_get_user, mock_get_user_by_email, @@ -160,40 +174,46 @@ def test_should_login_user_and_redirect_to_show_accounts( mock_email_validated_recently, platform_admin, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} api_user_active['platform_admin'] = platform_admin - response = client.post(url_for('main.two_factor_sms'), - data={'sms_code': '12345'}) - - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) + client_request.post( + 'main.two_factor_sms', + _data={'sms_code': '12345'}, + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True) + ) def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong( - client, + client_request, api_user_active, mock_get_user_by_email, mock_check_verify_code_code_not_found, mocker ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - response = client.post(url_for('main.two_factor_sms'), - data={'sms_code': '23456'}) - assert response.status_code == 200 - assert 'Code not found' in response.get_data(as_text=True) + page = client_request.post( + 'main.two_factor_sms', + _data={'sms_code': '23456'}, + _expected_status=200, + ) + assert 'Code not found' in page.text def test_should_login_user_when_multiple_valid_codes_exist( - client, + client_request, api_user_active, mock_get_user, mock_get_user_by_email, @@ -202,18 +222,22 @@ def test_should_login_user_when_multiple_valid_codes_exist( mock_create_event, mock_email_validated_recently, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - response = client.post(url_for('main.two_factor_sms'), - data={'sms_code': '23456'}) - assert response.status_code == 302 + client_request.post( + 'main.two_factor_sms', + _data={'sms_code': '23456'}, + _expected_status=302, + ) def test_two_factor_sms_should_set_password_when_new_password_exists_in_session( - client, + client_request, api_user_active, mock_get_user, mock_check_verify_code, @@ -222,16 +246,19 @@ def test_two_factor_sms_should_set_password_when_new_password_exists_in_session( mock_create_event, mock_email_validated_recently, ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address'], 'password': 'changedpassword'} - response = client.post(url_for('main.two_factor_sms'), - data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) + client_request.post( + 'main.two_factor_sms', + _data={'sms_code': '12345'}, + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True), + ) mock_update_user_password.assert_called_once_with( api_user_active['id'], 'changedpassword', @@ -239,21 +266,25 @@ def test_two_factor_sms_should_set_password_when_new_password_exists_in_session( def test_two_factor_sms_returns_error_when_user_is_locked( - client, + client_request, api_user_locked, mock_get_locked_user, mock_check_verify_code_code_not_found, mock_get_services_with_one_service ): - with client.session_transaction() as session: + client_request.logout() + + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_locked['id'], 'email': api_user_locked['email_address'], } - response = client.post(url_for('main.two_factor_sms'), - data={'sms_code': '12345'}) - assert response.status_code == 200 - assert 'Code not found' in response.get_data(as_text=True) + page = client_request.post( + 'main.two_factor_sms', + _data={'sms_code': '12345'}, + _expected_status=200, + ) + assert 'Code not found' in page.text def test_two_factor_sms_post_should_redirect_to_sign_in_if_user_not_in_session( @@ -278,18 +309,17 @@ def test_two_factor_endpoints_get_should_redirect_to_sign_in_if_user_not_in_sess def test_two_factor_webauthn_should_have_auth_signin_button( - client, + client_request, platform_admin_user, mocker, ): + client_request.logout() mock_get_user = mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'id': platform_admin_user['id'], 'email': platform_admin_user['email_address']} - response = client.get(url_for('main.two_factor_webauthn')) + page = client_request.get('main.two_factor_webauthn') - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') button = page.select_one("button[data-module=authenticate-security-key]") assert button.text.strip() == 'Check security key' @@ -299,22 +329,24 @@ def test_two_factor_webauthn_should_have_auth_signin_button( def test_two_factor_webauthn_should_reject_non_webauthn_auth_users( - client, + client_request, platform_admin_user, mocker, ): + client_request.logout() platform_admin_user['auth_type'] = 'sms_auth' mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'id': platform_admin_user['id'], 'email': platform_admin_user['email_address']} - response = client.get(url_for('main.two_factor_webauthn')) - - assert response.status_code == 403 + client_request.get( + 'main.two_factor_webauthn', + _expected_status=403, + ) def test_two_factor_sms_should_activate_pending_user( - client, + client_request, mocker, api_user_pending, mock_check_verify_code, @@ -322,14 +354,15 @@ def test_two_factor_sms_should_activate_pending_user( mock_activate_user, mock_email_validated_recently, ): + client_request.logout() mocker.patch('app.user_api_client.get_user', return_value=api_user_pending) mocker.patch('app.service_api_client.get_services', return_value={'data': []}) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': api_user_pending['id'], 'email_address': api_user_pending['email_address'] } - client.post(url_for('main.two_factor_sms'), data={'sms_code': '12345'}) + client_request.post('main.two_factor_sms', _data={'sms_code': '12345'}) assert mock_activate_user.called @@ -377,7 +410,7 @@ def test_valid_two_factor_email_link_shows_interstitial( def test_valid_two_factor_email_link_logs_in_user( - client, + client_request, valid_token, mock_get_user, mock_get_services_with_one_service, @@ -386,13 +419,11 @@ def test_valid_two_factor_email_link_logs_in_user( ): mocker.patch('app.user_api_client.check_verify_code', return_value=(True, '')) - response = client.post( + client_request.post_url( url_for_endpoint_with_token('main.two_factor_email', token=valid_token), + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True) ) - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) - @pytest.mark.parametrize('redirect_url', [ None, @@ -401,21 +432,19 @@ def test_valid_two_factor_email_link_logs_in_user( def test_two_factor_email_link_has_expired( notify_admin, valid_token, - client, + client_request, mock_send_verify_code, fake_uuid, redirect_url ): + client_request.logout() with set_config(notify_admin, 'EMAIL_2FA_EXPIRY_SECONDS', -1): - response = client.post( + page = client_request.post_url( url_for_endpoint_with_token('main.two_factor_email', token=valid_token, next=redirect_url), - follow_redirects=True, + _follow_redirects=True, ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.text.strip() == 'The link has expired' assert page.select_one('a:contains("Sign in again")')['href'] == url_for('main.sign_in', next=redirect_url) @@ -423,43 +452,42 @@ def test_two_factor_email_link_has_expired( def test_two_factor_email_link_is_invalid( - client + client_request ): + client_request.logout() token = 12345 - response = client.post( - url_for('main.two_factor_email', token=token), - follow_redirects=True + page = client_request.post( + 'main.two_factor_email', + token=token, + _follow_redirects=True, + _expected_status=404, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert normalize_spaces( page.select_one('.banner-dangerous').text ) == "There’s something wrong with the link you’ve used." - assert response.status_code == 404 - @pytest.mark.parametrize('redirect_url', [ None, f'/services/{SERVICE_ONE_ID}/templates', ]) def test_two_factor_email_link_is_already_used( - client, + client_request, valid_token, mocker, mock_send_verify_code, redirect_url ): + client_request.logout() mocker.patch('app.user_api_client.check_verify_code', return_value=(False, 'Code has expired')) - response = client.post( + page = client_request.post_url( url_for_endpoint_with_token('main.two_factor_email', token=valid_token, next=redirect_url), - follow_redirects=True + _follow_redirects=True, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert page.h1.text.strip() == 'The link has expired' assert page.select_one('a:contains("Sign in again")')['href'] == url_for('main.sign_in', next=redirect_url) @@ -467,21 +495,19 @@ def test_two_factor_email_link_is_already_used( def test_two_factor_email_link_when_user_is_locked_out( - client, + client_request, valid_token, mocker, mock_send_verify_code ): + client_request.logout() mocker.patch('app.user_api_client.check_verify_code', return_value=(False, 'Code not found')) - response = client.post( + page = client_request.post_url( url_for_endpoint_with_token('main.two_factor_email', token=valid_token), - follow_redirects=True + _follow_redirects=True, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert page.h1.text.strip() == 'The link has expired' assert mock_send_verify_code.called is False diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 98107e3b6..e84ebae0a 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -2,7 +2,6 @@ import json import uuid from unittest.mock import Mock -from bs4 import BeautifulSoup from flask import session as flask_session from flask import url_for from itsdangerous import SignatureExpired @@ -12,25 +11,24 @@ from app.main.views.verify import activate_user def test_should_return_verify_template( - client, + client_request, api_user_active, mock_send_verify_code, ): + client_request.logout() # TODO this lives here until we work out how to # reassign the session after it is lost mid register process - with client.session_transaction() as session: + with client_request.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')) - assert response.status_code == 200 + page = client_request.get('main.verify') - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text == 'Check your phone' message = page.select('main p')[0].text assert message == "We’ve sent you a text message with a security code." def test_should_redirect_to_add_service_when_sms_code_is_correct( - client, + client_request, api_user_active, mocker, mock_update_user_attribute, @@ -41,25 +39,26 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct( api_user_active['current_session_id'] = str(uuid.UUID(int=1)) mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'email_address': api_user_active['email_address'], 'id': api_user_active['id']} # user's only just created their account so no session in the cookie session.pop('current_session_id', None) - response = client.post(url_for('main.verify'), - data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for('main.add_service', first='first', _external=True) + client_request.post( + 'main.verify', + _data={'sms_code': '12345'}, + _expected_redirect=url_for('main.add_service', first='first', _external=True), + ) # make sure the current_session_id has changed to what the API returned - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert session['current_session_id'] == str(uuid.UUID(int=1)) mock_check_verify_code.assert_called_once_with(api_user_active['id'], '12345', 'sms') def test_should_activate_user_after_verify( - client, + client_request, mocker, api_user_pending, mock_send_verify_code, @@ -67,14 +66,14 @@ def test_should_activate_user_after_verify( mock_create_event, mock_activate_user, ): + client_request.logout() mocker.patch('app.user_api_client.get_user', return_value=api_user_pending) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'email_address': api_user_pending['email_address'], 'id': api_user_pending['id'] } - client.post(url_for('main.verify'), - data={'sms_code': '12345'}) + client_request.post('main.verify', _data={'sms_code': '12345'}) assert mock_activate_user.called @@ -100,7 +99,7 @@ def test_should_return_200_when_sms_code_is_wrong( def test_verify_email_redirects_to_verify_if_token_valid( - client, + client_request, mocker, api_user_pending, mock_get_user_pending, @@ -110,67 +109,72 @@ def test_verify_email_redirects_to_verify_if_token_valid( 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: + with client_request.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.verify', _external=True) + client_request.get( + 'main.verify_email', + token='notreal', + _expected_redirect=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: + with client_request.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, + client_request, mocker, api_user_pending, ): + client_request.logout() mocker.patch('app.main.views.verify.check_token', side_effect=SignatureExpired('expired')) - 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) + client_request.get( + 'main.verify_email', + token='notreal', + _expected_redirect=url_for('main.resend_email_verification', _external=True), + ) def test_verify_email_redirects_to_sign_in_if_user_active( - client, + client_request, mocker, api_user_active, mock_get_user, mock_send_verify_code, mock_check_verify_code, ): + client_request.logout() 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)) - response = client.get(url_for('main.verify_email', token='notreal'), follow_redirects=True) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get('main.verify_email', token='notreal', _follow_redirects=True) + assert page.h1.text == 'Sign in' flash_banner = page.find('div', class_='banner-dangerous').string.strip() assert flash_banner == "That verification link has expired." def test_verify_redirects_to_sign_in_if_not_logged_in( - client + client_request ): - response = client.get(url_for('main.verify')) - - assert response.location == url_for('main.sign_in', _external=True) - assert response.status_code == 302 + client_request.logout() + client_request.get( + 'main.verify', + _expected_redirect=url_for('main.sign_in', _external=True), + ) def test_activate_user_redirects_to_service_dashboard_if_user_already_belongs_to_service( mocker, - client, + client_request, service_one, sample_invite, api_user_active, diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index 82e11961e..e711d2430 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -10,18 +10,8 @@ from app.models.webauthn_credential import RegistrationError, WebAuthnCredential @pytest.fixture def webauthn_authentication_post_data(fake_uuid, webauthn_credential, client): - """ - Sets up session, challenge, etc as if a user with uuid `fake_uuid` has logged in and touched the webauthn token - as found in the `webauthn_credential` fixture. Sets up the session as if `begin_authentication` had been called - so that the challenge matches and the credential will validate (provided that the key belongs to the user referenced - in the session). - """ - with client.session_transaction() as session: - session['user_details'] = {'id': fake_uuid} - session['webauthn_authentication_state'] = { - "challenge": "e-g-nXaRxMagEiqTJSyD82RsEc5if_6jyfJDy8bNKlw", - "user_verification": None - } + + _set_up_webauthn_session(fake_uuid, client) credential_id = WebAuthnCredential(webauthn_credential).to_credential_data().credential_id @@ -33,6 +23,21 @@ def webauthn_authentication_post_data(fake_uuid, webauthn_credential, client): }) +def _set_up_webauthn_session(user_id, client): + """ + Sets up session, challenge, etc as if a user with uuid `fake_uuid` has logged in and touched the webauthn token + as found in the `webauthn_credential` fixture. Sets up the session as if `begin_authentication` had been called + so that the challenge matches and the credential will validate (provided that the key belongs to the user referenced + in the session). + """ + with client.session_transaction() as session: + session['user_details'] = {'id': user_id} + session['webauthn_authentication_state'] = { + "challenge": "e-g-nXaRxMagEiqTJSyD82RsEc5if_6jyfJDy8bNKlw", + "user_verification": None + } + + def test_begin_register_forbidden_unless_can_use_webauthn( client_request, platform_admin_user, @@ -206,28 +211,36 @@ def test_complete_register_handles_missing_state( assert cbor.decode(response.data) == 'No registration in progress' -def test_begin_authentication_forbidden_for_users_without_webauthn(client, mocker, platform_admin_user): +def test_begin_authentication_forbidden_for_users_without_webauthn(client_request, mocker, platform_admin_user): platform_admin_user['auth_type'] = 'sms_auth' + client_request.logout() mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'id': '1'} - response = client.get(url_for('main.webauthn_begin_authentication')) - assert response.status_code == 403 + client_request.get( + 'main.webauthn_begin_authentication', + _expected_status=403, + ) -def test_begin_authentication_returns_encoded_options(client, mocker, webauthn_credential, platform_admin_user): - mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) +def test_begin_authentication_returns_encoded_options( + client_request, + mocker, + webauthn_credential, + platform_admin_user, +): + client_request.login(platform_admin_user) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'id': platform_admin_user['id']} get_creds_mock = mocker.patch( 'app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[webauthn_credential] ) - response = client.get(url_for('main.webauthn_begin_authentication')) + response = client_request.get_response('main.webauthn_begin_authentication') decoded_data = cbor.decode(response.data) allowed_credentials = decoded_data['publicKey']['allowCredentials'] @@ -237,24 +250,29 @@ def test_begin_authentication_returns_encoded_options(client, mocker, webauthn_c get_creds_mock.assert_called_once_with(platform_admin_user['id']) -def test_begin_authentication_stores_state_in_session(client, mocker, webauthn_credential, platform_admin_user): - mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) +def test_begin_authentication_stores_state_in_session( + client_request, + mocker, + webauthn_credential, + platform_admin_user, +): + client_request.login(platform_admin_user) - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = {'id': platform_admin_user['id']} mocker.patch( 'app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[webauthn_credential] ) - client.get(url_for('main.webauthn_begin_authentication')) + client_request.get_response('main.webauthn_begin_authentication') - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'challenge' in session['webauthn_authentication_state'] def test_complete_authentication_checks_credentials( - client, + client_request, mocker, webauthn_credential, webauthn_dev_server, @@ -262,6 +280,9 @@ def test_complete_authentication_checks_credentials( webauthn_authentication_post_data, platform_admin_user ): + client_request.logout() + _set_up_webauthn_session(platform_admin_user['id'], client_request) + mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[webauthn_credential]) mocker.patch( @@ -269,30 +290,39 @@ def test_complete_authentication_checks_credentials( return_value=Mock(location='/foo') ) - response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data) + response = client_request.post_response( + 'main.webauthn_complete_authentication', + _data=webauthn_authentication_post_data, + _expected_status=200, + ) - assert response.status_code == 200 assert cbor.decode(response.data) == {'redirect_url': '/foo'} def test_complete_authentication_403s_if_key_isnt_in_users_credentials( - client, + client_request, mocker, webauthn_credential, webauthn_dev_server, webauthn_authentication_post_data, platform_admin_user ): + client_request.logout() + _set_up_webauthn_session(platform_admin_user['id'], client_request) + mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) # user has no keys in the database mocker.patch('app.models.webauthn_credential.WebAuthnCredentials.client_method', return_value=[]) mock_verify_webauthn_login = mocker.patch('app.main.views.webauthn_credentials._complete_webauthn_login_attempt') mock_unsuccesful_login_api_call = mocker.patch('app.user_api_client.complete_webauthn_login_attempt') - response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data) - assert response.status_code == 403 + client_request.post_response( + 'main.webauthn_complete_authentication', + _data=webauthn_authentication_post_data, + _expected_status=403, + ) - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert session['user_details']['id'] == platform_admin_user['id'] # user not logged in assert 'user_id' not in session @@ -305,7 +335,7 @@ def test_complete_authentication_403s_if_key_isnt_in_users_credentials( def test_complete_authentication_clears_session( - client, + client_request, mocker, webauthn_credential, webauthn_dev_server, @@ -313,6 +343,7 @@ def test_complete_authentication_clears_session( mock_create_event, platform_admin_user ): + client_request.logout() mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential]) mocker.patch( @@ -320,9 +351,9 @@ def test_complete_authentication_clears_session( return_value=Mock(location='/foo') ) - client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data) + client_request.post('main.webauthn_complete_authentication', _data=webauthn_authentication_post_data) - with client.session_transaction() as session: + with client_request.session_transaction() as session: # it's important that we clear the session to ensure that we don't re-use old login artifacts in future assert 'webauthn_authentication_state' not in session @@ -332,56 +363,61 @@ def test_complete_authentication_clears_session( ({'next': '/bar'}, '/bar'), ]) def test_verify_webauthn_login_signs_user_in( - client, + client_request, mocker, mock_create_event, platform_admin_user, url_kwargs, expected_redirect, ): - with client.session_transaction() as session: + client_request.logout() + with client_request.session_transaction() as session: session['user_details'] = { 'id': platform_admin_user['id'], 'email': platform_admin_user['email_address'] } - mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) + client_request.login(platform_admin_user) mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication') mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None)) mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=False) - resp = client.post(url_for('main.webauthn_complete_authentication', **url_kwargs)) + resp = client_request.post_response( + 'main.webauthn_complete_authentication', + _expected_status=200, + **url_kwargs + ) - assert resp.status_code == 200 assert cbor.decode(resp.data)['redirect_url'] == expected_redirect # removes stuff from session - with client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'user_details' not in session mock_create_event.assert_called_once_with('sucessful_login', ANY) def test_verify_webauthn_login_signs_user_in_doesnt_sign_user_in_if_api_rejects( - client, + client_request, mocker, platform_admin_user, ): - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = { 'id': platform_admin_user['id'], 'email': platform_admin_user['email_address'] } - mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) + client_request.login(platform_admin_user) mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication') mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(False, None)) - resp = client.post(url_for('main.webauthn_complete_authentication')) - - assert resp.status_code == 403 + client_request.post( + 'main.webauthn_complete_authentication', + _expected_status=403, + ) def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( - client, + client_request, mocker, mock_send_verify_code, platform_admin_user, @@ -391,7 +427,7 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( 'email': platform_admin_user['email_address'] } - with client.session_transaction() as session: + with client_request.session_transaction() as session: session['user_details'] = user_details mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) @@ -399,12 +435,14 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None)) mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=True) - resp = client.post(url_for('main.webauthn_complete_authentication')) + resp = client_request.post_response( + 'main.webauthn_complete_authentication', + _expected_status=200, + ) - assert resp.status_code == 200 assert cbor.decode(resp.data)['redirect_url'] == url_for('main.revalidate_email_sent') - with client.session_transaction() as session: + with client_request.session_transaction() as session: # stuff stays in session so we can log them in later when they validate their email assert session['user_details'] == user_details diff --git a/tests/app/test_assets.py b/tests/app/test_assets.py index 9a4c532e8..772926545 100644 --- a/tests/app/test_assets.py +++ b/tests/app/test_assets.py @@ -1,10 +1,15 @@ -def test_crown_logo(client): +def test_crown_logo(client_request): # This image is used by the email templates, so we should be really careful to make # sure that its always there. - response = client.get('/static/images/email-template/crown-32px.gif') - assert response.status_code == 200 + client_request.logout() + client_request.get_response_from_url( + '/static/images/email-template/crown-32px.gif', + _expected_status=200, + ) -def test_static_404s_return(client): - response = client.get('/static/images/some-image-that-doesnt-exist.png') - assert response.status_code == 404 +def test_static_404s_return(client_request): + client_request.get_response_from_url( + '/static/images/some-image-that-doesnt-exist.png', + _expected_status=404, + ) diff --git a/tests/conftest.py b/tests/conftest.py index d5c03cd73..a72e5126b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2904,9 +2904,17 @@ def client_request(_logged_in_client, mocker, service_one): # noqa (C901 too co _optional_args="", **endpoint_kwargs ): - resp = _logged_in_client.get( + return ClientRequest.get_response_from_url( url_for(endpoint, **(endpoint_kwargs or {})) + _optional_args, + _expected_status=_expected_status, ) + + @staticmethod + def get_response_from_url( + url, + _expected_status=200, + ): + resp = _logged_in_client.get(url) assert resp.status_code == _expected_status return resp