From 96a87e7cf2e60319101ec1acff358123a2f232dc Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Wed, 14 Jul 2021 14:43:04 +0100
Subject: [PATCH] Fix and test archive service permissions

Previously the backend would never validate permissions because the
"not service.active" part would (usually) fail. I've updated it to
match the (inverse of the) conditional we have in the HTML [1].

[1]: https://github.com/alphagov/notifications-admin/blob/6ac593aa5f176fe9dacb4be51ebda818bb9e8e1b/app/templates/views/service-settings.html#L455
---
 app/main/views/service_settings.py            |  2 +-
 tests/app/main/views/test_service_settings.py | 30 ++++++++++++-------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 9705d695f..e638117e9 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -421,7 +421,7 @@ def service_confirm_broadcast_account_type(service_id, account_type):
 @main.route("/services/<uuid:service_id>/service-settings/archive", methods=['GET', 'POST'])
 @user_has_permissions('manage_service')
 def archive_service(service_id):
-    if not current_service.active and (
+    if not current_service.active or not (
         current_service.trial_mode or current_user.platform_admin
     ):
         abort(403)
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 05af7453a..06375d6da 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -4111,10 +4111,12 @@ def test_switch_service_enable_international_sms_and_letters(
     assert mocked_fn.call_args[0][0] == service_one['id']
 
 
-@pytest.mark.parametrize('user', (
-    create_platform_admin_user(),
-    create_active_user_with_permissions(),
-    pytest.param(create_active_user_no_settings_permission(), marks=pytest.mark.xfail),
+@pytest.mark.parametrize('user, is_trial_service', (
+    [create_platform_admin_user(), True],
+    [create_platform_admin_user(), False],
+    [create_active_user_with_permissions(), True],
+    pytest.param(create_active_user_with_permissions(), False, marks=pytest.mark.xfail),
+    pytest.param(create_active_user_no_settings_permission(), True, marks=pytest.mark.xfail),
 ))
 def test_archive_service_after_confirm(
     client_request,
@@ -4124,8 +4126,11 @@ def test_archive_service_after_confirm(
     mock_get_organisations_and_services_for_user,
     mock_get_users_by_service,
     mock_get_service_templates,
+    service_one,
     user,
+    is_trial_service,
 ):
+    service_one['restricted'] = is_trial_service
     mock_api = mocker.patch('app.service_api_client.post')
     mock_event = mocker.patch('app.main.views.service_settings.create_archive_service_event')
     redis_delete_mock = mocker.patch('app.notify_client.service_api_client.redis_client.delete')
@@ -4149,21 +4154,26 @@ def test_archive_service_after_confirm(
     assert call(f"user-{sample_uuid()}") in redis_delete_mock.call_args_list
 
 
-@pytest.mark.parametrize('user', (
-    create_platform_admin_user(),
-    create_active_user_with_permissions(),
-    pytest.param(create_active_user_no_settings_permission(), marks=pytest.mark.xfail),
+@pytest.mark.parametrize('user, is_trial_service', (
+    [create_platform_admin_user(), True],
+    [create_platform_admin_user(), False],
+    [create_active_user_with_permissions(), True],
+    pytest.param(create_active_user_with_permissions(), False, marks=pytest.mark.xfail),
+    pytest.param(create_active_user_no_settings_permission(), True, marks=pytest.mark.xfail),
 ))
 def test_archive_service_prompts_user(
     client_request,
     mocker,
     single_reply_to_email_address,
     single_letter_contact_block,
+    service_one,
     single_sms_sender,
     mock_get_service_settings_page_common,
     user,
+    is_trial_service,
 ):
-    mocked_fn = mocker.patch('app.service_api_client.post')
+    mock_api = mocker.patch('app.service_api_client.post')
+    service_one['restricted'] = is_trial_service
     client_request.login(user)
 
     settings_page = client_request.get(
@@ -4186,7 +4196,7 @@ def test_archive_service_prompts_user(
         'There’s no way to undo this. '
         'Yes, delete'
     )
-    assert mocked_fn.called is False
+    assert mock_api.called is False
 
 
 def test_cant_archive_inactive_service(