From f7efceba44ff4c8d60b9b4216154c8bd8395960a Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Mon, 11 Dec 2023 10:47:21 -0500 Subject: [PATCH 1/2] Fix permissions check for inviting users to a service This changeset reverts a change we had made previously where we accidentally locked down the ability for service admins to invite other users to their own service. This removes the platform admin user check and reverts it back to the proper permissions check (including adjusting the tests to account for this). Signed-off-by: Carlo Costino --- app/main/views/manage_users.py | 4 ++-- tests/app/main/views/test_manage_users.py | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index d23c27d2f..3daaa38d2 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -21,7 +21,7 @@ from app.main.forms import ( SearchUsersForm, ) from app.models.user import InvitedUser, User -from app.utils.user import is_gov_user, user_has_permissions, user_is_platform_admin +from app.utils.user import is_gov_user, user_has_permissions from app.utils.user_permissions import permission_options @@ -42,7 +42,7 @@ def manage_users(service_id): @main.route( "/services//users/invite/", methods=["GET", "POST"] ) -@user_is_platform_admin +@user_has_permissions("manage_service") def invite_user(service_id, user_id=None): form_class = InviteUserForm form = form_class( diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 2a93dbef0..6474cc13d 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -800,10 +800,13 @@ def test_should_show_page_for_inviting_user_with_email_prefilled( user_id=fake_uuid, # We have the user’s name in the H1 but don’t want it duplicated # in the page title - _test_page_title=False, - _expected_status=403, + _test_page_title=False ) - assert "not allowed to see this page" in page.h1.string.strip() + assert normalize_spaces(page.select_one("title").text).startswith( + "Invite a team member" + ) + assert normalize_spaces(page.select_one("h1").text) == ("Invite Service Two User") + assert not page.select("input#email_address") or page.select("input[type=email]") def test_should_show_page_if_prefilled_user_is_already_a_team_member( @@ -1280,9 +1283,11 @@ def test_user_cant_invite_themselves( "permissions_field": ["send_messages", "manage_service", "manage_api_keys"], }, _follow_redirects=True, - _expected_status=403, + _expected_status=200, ) - assert "not allowed to see this page" in page.h1.string.strip() + assert page.h1.string.strip() == "Invite a team member" + form_error = page.find("span", class_="usa-error-message").text.strip() + assert form_error == "Error: You cannot send an invitation to yourself" assert not mock_create_invite.called From b0d2de703bdd9d09976ac25779de1ed5bb6027b6 Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Mon, 11 Dec 2023 16:59:04 -0500 Subject: [PATCH 2/2] Updated tests with correct permissions and added test for no permissions h/t @stvnrlly for the suggestions! Signed-off-by: Carlo Costino --- tests/app/main/views/test_manage_users.py | 24 ++++++++++++++++++----- tests/conftest.py | 5 +++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 6474cc13d..abc23b563 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -762,9 +762,9 @@ def test_edit_user_permissions_shows_authentication_for_email_auth_service( def test_should_show_page_for_inviting_user( client_request, mock_get_template_folders, - platform_admin_user, + active_user_with_permissions, ): - client_request.login(platform_admin_user) + client_request.login(active_user_with_permissions) page = client_request.get( "main.invite_user", service_id=SERVICE_ONE_ID, @@ -774,6 +774,21 @@ def test_should_show_page_for_inviting_user( assert not page.find("div", class_="checkboxes-nested") +def test_should_not_show_page_for_inviting_user_without_permissions( + client_request, + mock_get_template_folders, + active_user_empty_permissions +): + client_request.login(active_user_empty_permissions) + page = client_request.get( + "main.invite_user", + service_id=SERVICE_ONE_ID, + _expected_status=403 + ) + + assert "not allowed to see this page" in page.h1.string.strip() + + def test_should_show_page_for_inviting_user_with_email_prefilled( client_request, mocker, @@ -815,10 +830,9 @@ def test_should_show_page_if_prefilled_user_is_already_a_team_member( mock_get_template_folders, fake_uuid, active_user_with_permissions, - active_caseworking_user, - platform_admin_user, + active_caseworking_user ): - client_request.login(platform_admin_user) + client_request.login(active_user_with_permissions) mocker.patch( "app.models.user.user_api_client.get_user", side_effect=[ diff --git a/tests/conftest.py b/tests/conftest.py index 0c43770d3..9d55b796b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1067,6 +1067,11 @@ def active_user_with_permissions(fake_uuid): return create_active_user_with_permissions() +@pytest.fixture() +def active_user_empty_permissions(fake_uuid): + return create_active_user_empty_permissions() + + @pytest.fixture() def active_user_with_permission_to_two_services(fake_uuid): permissions = [