From 9d44c45e90b2635c7808169a2d50121f3e1ae768 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 2 Apr 2019 12:07:11 +0100 Subject: [PATCH 1/2] Only users who can manage team members see folder permissions --- .../templates/manage-template-folder.html | 4 +- tests/app/main/views/test_template_folders.py | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/templates/views/templates/manage-template-folder.html b/app/templates/views/templates/manage-template-folder.html index 457676a1b..5b8f97a7d 100644 --- a/app/templates/views/templates/manage-template-folder.html +++ b/app/templates/views/templates/manage-template-folder.html @@ -26,7 +26,9 @@ {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %} {{ textbox(form.name) }} {% if current_service.has_permission("edit_folder_permissions") %} - {{ checkboxes(form.users_with_permission) }} + {% if current_user.has_permissions("manage_service") %} + {{ checkboxes(form.users_with_permission) }} + {% endif %} {% endif %} {{ page_footer( diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 6cfe9f23d..d8ba3414c 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -477,6 +477,48 @@ def test_get_manage_folder_viewing_permissions_for_users( assert "Test User" in page.findAll('label', {'for': 'users_with_permission-0'})[0].text +def test_get_manage_folder_viewing_permissions_for_users_not_visible_when_no_manage_users_permission( + client_request, + active_user_with_permissions, + service_one, + mock_get_template_folders, + mocker +): + active_user_with_permissions.permissions[SERVICE_ONE_ID] = [ + 'send_texts', + 'send_emails', + 'send_letters', + 'manage_templates', + 'manage_settings', + 'manage_api_keys', + 'view_activity', + ] + folder_id = str(uuid.uuid4()) + team_member = active_user_view_permissions(str(uuid.uuid4())) + team_member_2 = active_user_view_permissions(str(uuid.uuid4())) + service_one["permissions"] += ["edit_folder_permissions"] + mock_get_template_folders.return_value = [ + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id, team_member_2.id + ]}, + ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member, team_member_2]) + + page = client_request.get( + 'main.manage_template_folder', + service_id=service_one['id'], + template_folder_id=folder_id, + _test_page_title=False, + ) + assert normalize_spaces(page.select_one('title').text) == ( + 'folder_two – Templates – service one – GOV.UK Notify' + ) + form_labels = page.select('legend[class=form-label]') + assert len(form_labels) == 0 + checkboxes = page.select('input[name=users_with_permission]') + assert len(checkboxes) == 0 + + def test_manage_folder_page_404s(client_request, service_one, mock_get_template_folders): client_request.get( 'main.manage_template_folder', From bd66cc220b24eea3f0632f38e7c3a2276d97dc61 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 2 Apr 2019 12:26:41 +0100 Subject: [PATCH 2/2] User permissions section for a folder noy visible if service only has one user If no manage users permission then don't pass user folder permissions to api --- app/main/views/templates.py | 5 +- .../templates/manage-template-folder.html | 2 +- tests/app/main/views/test_template_folders.py | 72 ++++++++++++++++++- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 19af79fc7..6c0ddd2de 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -460,7 +460,10 @@ def manage_template_folder(service_id, template_folder_id): all_service_users=[user for user in current_service.active_users if user.id != current_user.id] ) if form.validate_on_submit(): - users_with_permission = form.users_with_permission.data + [current_user.id] + if current_user.has_permissions("manage_service") and form.users_with_permission.all_service_users: + users_with_permission = form.users_with_permission.data + [current_user.id] + else: + users_with_permission = None template_folder_api_client.update_template_folder( current_service.id, template_folder_id, diff --git a/app/templates/views/templates/manage-template-folder.html b/app/templates/views/templates/manage-template-folder.html index 5b8f97a7d..f7bbb37a0 100644 --- a/app/templates/views/templates/manage-template-folder.html +++ b/app/templates/views/templates/manage-template-folder.html @@ -26,7 +26,7 @@ {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %} {{ textbox(form.name) }} {% if current_service.has_permission("edit_folder_permissions") %} - {% if current_user.has_permissions("manage_service") %} + {% if current_user.has_permissions("manage_service") and form.users_with_permission.all_service_users %} {{ checkboxes(form.users_with_permission) }} {% endif %} {% endif %} diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index d8ba3414c..651d9460f 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -519,6 +519,35 @@ def test_get_manage_folder_viewing_permissions_for_users_not_visible_when_no_man assert len(checkboxes) == 0 +def test_get_manage_folder_viewing_permissions_for_users_not_visible_for_services_with_one_user( + client_request, + active_user_with_permissions, + service_one, + mock_get_template_folders, + mocker +): + folder_id = str(uuid.uuid4()) + service_one["permissions"] += ["edit_folder_permissions"] + mock_get_template_folders.return_value = [ + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id + ]}, + ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions]) + + page = client_request.get( + 'main.manage_template_folder', + service_id=service_one['id'], + template_folder_id=folder_id, + _test_page_title=False, + ) + assert normalize_spaces(page.select_one('title').text) == ( + 'folder_two – Templates – service one – GOV.UK Notify' + ) + form_labels = page.select('legend[class=form-label]') + assert len(form_labels) == 0 + + def test_manage_folder_page_404s(client_request, service_one, mock_get_template_folders): client_request.get( 'main.manage_template_folder', @@ -634,7 +663,7 @@ def test_rename_folder(client_request, active_user_with_permissions, service_one service_one['id'], folder_id, name="new beautiful name", - users_with_permission=[active_user_with_permissions.id] + users_with_permission=None ) @@ -670,6 +699,47 @@ def test_manage_folder_users( ) +def test_manage_folder_users_doesnt_change_permissions_current_user_cannot_manage_users( + client_request, active_user_with_permissions, service_one, mock_get_template_folders, mocker +): + active_user_with_permissions.permissions[SERVICE_ONE_ID] = [ + 'send_texts', + 'send_emails', + 'send_letters', + 'manage_templates', + 'manage_settings', + 'manage_api_keys', + 'view_activity', + ] + team_member = active_user_view_permissions(str(uuid.uuid4())) + mock_update = mocker.patch('app.template_folder_api_client.update_template_folder') + folder_id = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': folder_id, 'name': 'folder_two', 'parent_id': None, 'users_with_permission': [ + active_user_with_permissions.id, team_member.id + ]} + ] + mocker.patch('app.models.service.Service.active_users', [active_user_with_permissions, team_member]) + + client_request.post( + 'main.manage_template_folder', + service_id=service_one['id'], + template_folder_id=folder_id, + _data={"name": "new beautiful name", "users_with_permission": []}, + _expected_redirect=url_for("main.choose_template", + service_id=service_one['id'], + template_folder_id=folder_id, + _external=True) + ) + + mock_update.assert_called_once_with( + service_one['id'], + folder_id, + name="new beautiful name", + users_with_permission=None + ) + + def test_delete_template_folder_should_request_confirmation( client_request, service_one, mock_get_template_folders, mocker, ):