From f92167de71e68dc2a05bebc2259aa4bcf9e7636f Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 2 Feb 2022 16:21:36 +0000 Subject: [PATCH] Only show branding description pages if branding allowed for service It shouldn't be possible to view the page to confirm that you want a particular type of email branding if that branding is not allowed for your service. Although we don't show banned branding options on the branding form, it would have been possible to visit the relevant URLs directly. We now give a `404` status page if you visit a page to select branding that isn't allowed. --- app/main/views/service_settings.py | 16 ++++++ tests/app/main/views/test_service_settings.py | 52 ++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 0a5d3763b..2aeb4a9e5 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -1191,11 +1191,21 @@ def email_branding_request(service_id): ) +def check_branding_allowed_for_service(branding): + allowed_branding_for_service = dict( + BrandingOptions.get_available_choices(current_service, branding_type='email') + ) + if branding not in allowed_branding_for_service: + abort(404) + + @main.route("/services//service-settings/email-branding/govuk", methods=['GET', 'POST']) @user_has_permissions('manage_service') def email_branding_govuk(service_id): with_org = request.args.get('with_org') + check_branding_allowed_for_service('govuk_and_org' if with_org else 'govuk') + if request.method == 'POST': create_email_branding_zendesk_ticket(request.form['branding_choice']) @@ -1208,6 +1218,8 @@ def email_branding_govuk(service_id): @main.route("/services//service-settings/email-branding/nhs", methods=['GET', 'POST']) @user_has_permissions('manage_service') def email_branding_nhs(service_id): + check_branding_allowed_for_service('nhs') + if request.method == 'POST': create_email_branding_zendesk_ticket('nhs') @@ -1220,6 +1232,8 @@ def email_branding_nhs(service_id): @main.route("/services//service-settings/email-branding/organisation", methods=['GET', 'POST']) @user_has_permissions('manage_service') def email_branding_organisation(service_id): + check_branding_allowed_for_service('organisation') + if request.method == 'POST': create_email_branding_zendesk_ticket('organisation') @@ -1232,6 +1246,8 @@ def email_branding_organisation(service_id): @main.route("/services//service-settings/email-branding/something-else", methods=['GET', 'POST']) @user_has_permissions('manage_service') def email_branding_something_else(service_id): + check_branding_allowed_for_service('something_else') + form = SomethingElseBrandingForm() if form.validate_on_submit(): diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 07dd59a76..9e5f4a06a 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5440,13 +5440,32 @@ def test_submit_branding_when_something_else_is_only_option( ) in mock_create_ticket.call_args_list[0][1]['message'] -@pytest.mark.parametrize('endpoint, query_param, expected_heading', [ - ('main.email_branding_govuk', False, 'Before you request new branding'), - ('main.email_branding_govuk', True, 'Before you request new branding'), - ('main.email_branding_nhs', False, 'Before you request new branding'), - ('main.email_branding_organisation', False, 'When you request new branding'), +@pytest.mark.parametrize('endpoint, query_param, service_org_type, expected_heading', [ + ('main.email_branding_govuk', False, 'central', 'Before you request new branding'), + ('main.email_branding_govuk', True, 'central', 'Before you request new branding'), + ('main.email_branding_nhs', False, 'nhs_local', 'Before you request new branding'), + ('main.email_branding_organisation', 'central', False, 'When you request new branding'), ]) -def test_get_email_branding_description_pages(client_request, endpoint, query_param, expected_heading): +def test_get_email_branding_description_pages( + client_request, + mocker, + service_one, + organisation_one, + mock_get_email_branding, + endpoint, + query_param, + service_org_type, + expected_heading, +): + organisation_one['organisation_type'] = service_org_type + service_one['email_branding'] = sample_uuid() + service_one['organisation'] = organisation_one + + mocker.patch( + 'app.organisations_client.get_organisation', + return_value=organisation_one, + ) + page = client_request.get( endpoint, service_id=SERVICE_ONE_ID, @@ -5466,6 +5485,27 @@ def test_get_email_branding_something_else_page(client_request): assert normalize_spaces(page.select_one('.page-footer button').text) == 'Request new branding' +@pytest.mark.parametrize('endpoint, query_param', [ + ('main.email_branding_govuk', False), + ('main.email_branding_govuk', True), + ('main.email_branding_nhs', False), + ('main.email_branding_organisation', False), +]) +def test_get_email_branding_description_pages_give_404_if_selected_branding_not_allowed( + client_request, + endpoint, + query_param, +): + # The only email branding allowed is 'something_else', so trying to visit any of the other + # endpoints gives a 404 status code. + client_request.get( + endpoint, + service_id=SERVICE_ONE_ID, + with_org=(True if query_param else None), + _expected_status=404 + ) + + @pytest.mark.parametrize('branding_choice, branding_description', [ ('govuk', 'GOV.UK'), ('govuk_and_org', 'GOV.UK and organisation one'),