From cd7c27925ced46a7dba24d928a0468ca6191e2f7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 22 May 2017 11:49:15 +0100 Subject: [PATCH] =?UTF-8?q?Check=20that=20users=20can=E2=80=99t=20skip=20s?= =?UTF-8?q?teps=20in=20send=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur. --- app/main/views/send.py | 12 +++- tests/app/main/views/test_send.py | 115 ++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 64fb70eb1..638f67cc0 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -177,6 +177,11 @@ def send_test(service_id, template_id): @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_test_step(service_id, template_id, step_index): + if 'send_test_values' not in session: + return redirect(url_for( + '.send_test', service_id=service_id, template_id=template_id + )) + template = service_api_client.get_service_template(service_id, template_id)['data'] if not session.get('send_test_letter_page_count'): @@ -201,7 +206,12 @@ def send_test_step(service_id, template_id, step_index): if len(placeholders) == 0: return make_and_upload_csv_file(service_id, template) - current_placeholder = placeholders[step_index] + try: + current_placeholder = placeholders[step_index] + except IndexError: + return redirect(url_for( + '.send_test', service_id=service_id, template_id=template_id + )) optional_placeholder = (current_placeholder in optional_address_columns) form = get_placeholder_form_instance( current_placeholder, diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 8e616e1bf..a90d690f1 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -21,6 +21,8 @@ from tests.conftest import ( mock_get_service_letter_template, mock_get_service, mock_get_international_service, + mock_get_service_template, + mock_get_service_email_template, ) template_types = ['email', 'sms'] @@ -320,6 +322,119 @@ def test_send_test_sms_message( mock_s3_upload.assert_called_with(service_one['id'], expected_data, 'eu-west-1') +def test_send_test_step_redirects_if_session_not_setup( + logged_in_client, + service_one, + fake_uuid, + mock_get_service_email_template, +): + + with logged_in_client.session_transaction() as session: + assert 'send_test_values' not in session + + response = logged_in_client.get( + url_for('main.send_test_step', service_id=service_one['id'], template_id=fake_uuid, step_index=0), + follow_redirects=True + ) + assert response.status_code == 200 + + with logged_in_client.session_transaction() as session: + assert session['send_test_values'] == {'email address': 'test@user.gov.uk'} + + +def test_send_test_redirects_to_end_if_step_out_of_bounds( + logged_in_client, + service_one, + fake_uuid, + mock_get_service_template, + mock_s3_upload, + mock_get_users_by_service, + mock_get_detailed_service_for_today, +): + + with logged_in_client.session_transaction() as session: + session['send_test_values'] = {'name': 'foo'} + + response = logged_in_client.get(url_for( + 'main.send_test_step', + service_id=service_one['id'], + template_id=fake_uuid, + step_index=999, + )) + + assert response.status_code == 302 + expected_url = url_for( + 'main.check_messages', + service_id=service_one['id'], + upload_id=fake_uuid, + template_type='sms', + _external=True, + ) + assert response.location in ( + expected_url + '?help=0&from_test=True', + expected_url + '?from_test=True&help=0', + ) + + +def test_send_test_redirects_to_start_if_you_skip_steps( + logged_in_platform_admin_client, + service_one, + fake_uuid, + mock_get_service_letter_template, + mock_s3_upload, + mock_get_users_by_service, + mock_get_detailed_service_for_today, + mocker, +): + + with logged_in_platform_admin_client.session_transaction() as session: + session['send_test_letter_page_count'] = 1 + session['send_test_values'] = {'address_line_1': 'foo'} + + response = logged_in_platform_admin_client.get(url_for( + 'main.send_test_step', + service_id=service_one['id'], + template_id=fake_uuid, + step_index=7, # letter template has 7 placeholders – we’re at the end + )) + assert response.status_code == 302 + assert response.location == url_for( + 'main.send_test', + service_id=service_one['id'], + template_id=fake_uuid, + _external=True, + ) + + +def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholders_empty( + logged_in_client, + service_one, + fake_uuid, + mock_get_service_email_template, + mock_s3_download, + mock_get_users_by_service, + mock_get_detailed_service_for_today, +): + + with logged_in_client.session_transaction() as session: + session['send_test_values'] = {'name': 'foo'} + + response = logged_in_client.get(url_for( + 'main.send_test_step', + service_id=service_one['id'], + template_id=fake_uuid, + step_index=999, + )) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.send_test', + service_id=service_one['id'], + template_id=fake_uuid, + _external=True, + ) + + def test_send_test_sms_message_redirects_with_help_argument( logged_in_client, service_one,