Merge pull request #912 from GSA/notify-admin-571

Notify admin 571 Add a permissions check to the create user, create organizations, and create services views

app/main/views/add_service.py

@@ -6,7 +6,7 @@ from app import service_api_client
 from app.formatters import email_safe
 from app.main import main
 from app.main.forms import CreateServiceForm
-from app.utils.user import user_is_gov_user, user_is_logged_in
+from app.utils.user import user_is_gov_user, user_is_platform_admin
 
 
 def _create_service(service_name, organization_type, email_from, form):
@@ -41,8 +41,8 @@ def _create_example_template(service_id):
 
 
 @main.route("/add-service", methods=["GET", "POST"])
-@user_is_logged_in
 @user_is_gov_user
+@user_is_platform_admin
 def add_service():
     default_organization_type = current_user.default_organization_type
     if default_organization_type is None:

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
+from app.utils.user import is_gov_user, user_has_permissions, user_is_platform_admin
 from app.utils.user_permissions import permission_options
 
 
@@ -42,10 +42,9 @@ def manage_users(service_id):
 @main.route(
     "/services/<uuid:service_id>/users/invite/<uuid:user_id>", methods=["GET", "POST"]
 )
-@user_has_permissions("manage_service")
+@user_is_platform_admin
 def invite_user(service_id, user_id=None):
     form_class = InviteUserForm
-
     form = form_class(
         inviter_email_address=current_user.email_address,
         all_template_folders=current_service.all_template_folders,

tests/app/main/views/test_add_service.py

@@ -31,7 +31,9 @@ def test_get_should_render_add_service_template(
     client_request,
     mocker,
     org_json,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.organizations_client.get_organization_by_domain",
         return_value=org_json,
@@ -42,9 +44,9 @@ def test_get_should_render_add_service_template(
 
 
 def test_get_should_not_render_radios_if_org_type_known(
-    client_request,
+    client_request, mocker, platform_admin_user
-    mocker,
 ):
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.organizations_client.get_organization_by_domain",
         return_value=organization_json(organization_type="central"),
@@ -56,9 +58,9 @@ def test_get_should_not_render_radios_if_org_type_known(
 
 
 def test_show_different_page_if_user_org_type_is_local(
-    client_request,
+    client_request, mocker, platform_admin_user
-    mocker,
 ):
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.organizations_client.get_organization_by_domain",
         return_value=organization_json(organization_type="local"),
@@ -101,9 +103,10 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(
     posted,
     persisted,
     sms_limit,
+    platform_admin_user,
 ):
     api_user_active["email_address"] = email_address
-    client_request.login(api_user_active)
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.organizations_client.get_organization_by_domain",
         return_value=organization_json(organization_type=inherited),
@@ -151,7 +154,9 @@ def test_add_service_has_to_choose_org_type(
     mock_get_services_with_no_services,
     api_user_active,
     mock_get_all_email_branding,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.organizations_client.get_organization_by_domain",
         return_value=None,
@@ -223,7 +228,9 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
     organization_type,
     free_allowance,
     mock_get_all_email_branding,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     client_request.post(
         "main.add_service",
         _data={
@@ -252,7 +259,9 @@ def test_add_service_fails_if_service_name_fails_validation(
     mock_get_organization_by_domain,
     name,
     error_message,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     page = client_request.post(
         "main.add_service",
         _data={"name": name},
@@ -263,9 +272,7 @@ def test_add_service_fails_if_service_name_fails_validation(
 
 @freeze_time("2021-01-01")
 def test_should_return_form_errors_with_duplicate_service_name_regardless_of_case(
-    client_request,
+    client_request, mock_get_organization_by_domain, mocker, platform_admin_user
-    mock_get_organization_by_domain,
-    mocker,
 ):
     def _create(**_kwargs):
         json_mock = mocker.Mock(
@@ -276,7 +283,7 @@ def test_should_return_form_errors_with_duplicate_service_name_regardless_of_cas
         raise http_error
 
     mocker.patch("app.service_api_client.create_service", side_effect=_create)
-
+    client_request.login(platform_admin_user)
     page = client_request.post(
         "main.add_service",
         _data={

tests/app/main/views/test_manage_users.py

@@ -15,6 +15,7 @@ from tests.conftest import (
     create_active_user_manage_template_permissions,
     create_active_user_view_permissions,
     create_active_user_with_permissions,
+    create_platform_admin_user,
     normalize_spaces,
     sample_uuid,
 )
@@ -296,9 +297,11 @@ def test_service_with_no_email_auth_hides_auth_type_options(
     service_one,
     mock_get_users_by_service,
     mock_get_template_folders,
+    platform_admin_user,
 ):
     if service_has_email_auth:
         service_one["permissions"].append("email_auth")
+    client_request.login(platform_admin_user)
     page = client_request.get(endpoint, service_id=service_one["id"], **extra_args)
     assert (
         page.find("input", attrs={"name": "login_authentication"}) is None
@@ -326,7 +329,9 @@ def test_service_without_caseworking_doesnt_show_admin_vs_caseworker(
     endpoint,
     service_has_caseworking,
     extra_args,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     page = client_request.get(endpoint, service_id=SERVICE_ONE_ID, **extra_args)
     permission_checkboxes = page.select("input[type=checkbox]")
 
@@ -443,7 +448,9 @@ def test_should_show_page_for_one_user(
     endpoint,
     extra_args,
     expected_checkboxes,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     page = client_request.get(endpoint, service_id=SERVICE_ONE_ID, **extra_args)
     checkboxes = page.select("input[type=checkbox]")
 
@@ -461,8 +468,10 @@ def test_invite_user_allows_to_choose_auth(
     mock_get_users_by_service,
     mock_get_template_folders,
     service_one,
+    platform_admin_user,
 ):
     service_one["permissions"].append("email_auth")
+    client_request.login(platform_admin_user)
     page = client_request.get("main.invite_user", service_id=SERVICE_ONE_ID)
 
     radio_buttons = page.select("input[name=login_authentication]")
@@ -476,7 +485,9 @@ def test_invite_user_has_correct_email_field(
     client_request,
     mock_get_users_by_service,
     mock_get_template_folders,
+    platform_admin_user,
 ):
+    client_request.login(platform_admin_user)
     email_field = client_request.get(
         "main.invite_user", service_id=SERVICE_ONE_ID
     ).select_one("#email_address")
@@ -790,9 +801,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,
-    active_user_with_permissions,
+    platform_admin_user,
 ):
-    client_request.login(active_user_with_permissions)
+    client_request.login(platform_admin_user)
     page = client_request.get(
         "main.invite_user",
         service_id=SERVICE_ONE_ID,
@@ -829,15 +840,9 @@ def test_should_show_page_for_inviting_user_with_email_prefilled(
         # 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,
     )
-    assert normalize_spaces(page.select_one("title").text).startswith(
+    assert "not allowed to see this page" in page.h1.string.strip()
-        "Invite a team member"
-    )
-    assert normalize_spaces(page.select_one("h1").text) == ("Invite Service Two User")
-    # assert normalize_spaces(page.select_one('main .gov-uk').text) == (
-    #     'service-two-user@test.gsa.gov'
-    # )
-    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(
@@ -847,8 +852,9 @@ def test_should_show_page_if_prefilled_user_is_already_a_team_member(
     fake_uuid,
     active_user_with_permissions,
     active_caseworking_user,
+    platform_admin_user,
 ):
-    client_request.login(active_user_with_permissions)
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.models.user.user_api_client.get_user",
         side_effect=[
@@ -878,14 +884,14 @@ def test_should_show_page_if_prefilled_user_is_already_invited(
     client_request,
     mock_get_template_folders,
     fake_uuid,
-    active_user_with_permissions,
     active_user_with_permission_to_other_service,
     mock_get_invites_for_service,
+    platform_admin_user,
 ):
     active_user_with_permission_to_other_service[
         "email_address"
     ] = "user_1@testnotify.gsa.gov"
-    client_request.login(active_user_with_permissions)
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.models.user.user_api_client.get_user",
         side_effect=[
@@ -966,8 +972,9 @@ def test_should_403_if_trying_to_prefill_email_address_for_user_from_other_organ
 
 
 def test_should_show_folder_permission_form_if_service_has_folder_permissions_enabled(
-    client_request, mocker, mock_get_template_folders, service_one
+    client_request, mocker, mock_get_template_folders, service_one, platform_admin_user
 ):
+    client_request.login(platform_admin_user)
     mock_get_template_folders.return_value = [
         {
             "id": "folder-id-1",
@@ -1005,7 +1012,7 @@ def test_should_show_folder_permission_form_if_service_has_folder_permissions_en
 )
 def test_invite_user(
     client_request,
-    active_user_with_permissions,
+    platform_admin_user,
     mocker,
     sample_invite,
     email_address,
@@ -1021,9 +1028,10 @@ def test_invite_user(
     )
     mocker.patch(
         "app.models.user.Users.client_method",
-        return_value=[active_user_with_permissions],
+        return_value=[platform_admin_user],
     )
     mocker.patch("app.invite_api_client.create_invite", return_value=sample_invite)
+    client_request.login(platform_admin_user)
     page = client_request.post(
         "main.invite_user",
         service_id=SERVICE_ONE_ID,
@@ -1064,7 +1072,7 @@ def test_invite_user(
 def test_invite_user_when_email_address_is_prefilled(
     client_request,
     service_one,
-    active_user_with_permissions,
+    platform_admin_user,
     active_user_with_permission_to_other_service,
     fake_uuid,
     mocker,
@@ -1074,7 +1082,7 @@ def test_invite_user_when_email_address_is_prefilled(
     mock_get_organization_by_domain,
 ):
     service_one["organization"] = ORGANISATION_ID
-    client_request.login(active_user_with_permissions)
+    client_request.login(platform_admin_user)
     mocker.patch(
         "app.models.user.user_api_client.get_user",
         side_effect=[
@@ -1095,7 +1103,7 @@ def test_invite_user_when_email_address_is_prefilled(
     )
 
     app.invite_api_client.create_invite.assert_called_once_with(
-        active_user_with_permissions["id"],
+        platform_admin_user["id"],
         SERVICE_ONE_ID,
         active_user_with_permission_to_other_service["email_address"],
         {"send_messages"},
@@ -1112,7 +1120,7 @@ def test_invite_user_when_email_address_is_prefilled(
 def test_invite_user_with_email_auth_service(
     client_request,
     service_one,
-    active_user_with_permissions,
+    platform_admin_user,
     sample_invite,
     email_address,
     gov_user,
@@ -1130,10 +1138,11 @@ def test_invite_user_with_email_auth_service(
     )
     mocker.patch(
         "app.models.user.Users.client_method",
-        return_value=[active_user_with_permissions],
+        return_value=[platform_admin_user],
     )
     mocker.patch("app.invite_api_client.create_invite", return_value=sample_invite)
 
+    client_request.login(platform_admin_user)
     page = client_request.post(
         "main.invite_user",
         service_id=SERVICE_ONE_ID,
@@ -1316,12 +1325,27 @@ def test_user_cant_invite_themselves(
             "permissions_field": ["send_messages", "manage_service", "manage_api_keys"],
         },
         _follow_redirects=True,
+        _expected_status=403,
+    )
+    assert "not allowed to see this page" in page.h1.string.strip()
+    assert not mock_create_invite.called
+
+
+def test_user_cant_invite_themselves_platform_admin(
+    client_request,
+    mocker,
+    mock_create_invite,
+    mock_get_template_folders,
+):
+    platform_admin = create_platform_admin_user()
+    client_request.login(platform_admin)
+    page = client_request.post(
+        "main.invite_user",
+        service_id=SERVICE_ONE_ID,
+        _follow_redirects=True,
         _expected_status=200,
     )
-    assert page.h1.string.strip() == "Invite a team member"
+    assert "Invite a team member" in page.h1.string.strip()
-    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
 
 
 def test_no_permission_manage_users_page(

tests/app/main/views/test_sign_in.py

@@ -4,7 +4,7 @@ import pytest
 from flask import url_for
 
 from app.models.user import User
-from tests.conftest import SERVICE_ONE_ID, normalize_spaces
+from tests.conftest import SERVICE_ONE_ID, create_platform_admin_user, normalize_spaces
 
 
 def test_render_sign_in_template_for_new_user(client_request):
@@ -69,6 +69,22 @@ def test_doesnt_redirect_to_sign_in_if_no_session_info(
 ):
     api_user_active["current_session_id"] = str(uuid.UUID(int=1))
 
+    with client_request.session_transaction() as session:
+        session["current_session_id"] = None
+
+    # This returns a 403 now
+    with pytest.raises(AssertionError):
+        client_request.get("main.add_service")
+
+
+def test_doesnt_redirect_to_sign_in_if_no_session_info_platform_admin(
+    client_request,
+    mock_get_organization_by_domain,
+):
+    platform_admin = create_platform_admin_user()
+    client_request.login(platform_admin)
+    platform_admin["current_session_id"] = str(uuid.UUID(int=1))
+
     with client_request.session_transaction() as session:
         session["current_session_id"] = None