mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-05-26 08:09:51 -04:00
Audit when user permissions are changed
I've used the term "admin_roles" in the event data to try and show that these are not the permissions we store in the DB. This is the name we use for the abstracted form of permissions in the Admin app. While we could store the DB permissions, that would be a bit more effort and arguably it's clearer to keep the event data consistent with the options the user actually saw / chose.
This commit is contained in:
@@ -8,6 +8,7 @@ EVENT_SCHEMAS = {
|
||||
"update_user_mobile_number": {"user_id", "updated_by_id", "original_mobile_number", "new_mobile_number"},
|
||||
"remove_user_from_service": {"user_id", "removed_by_id", "service_id"},
|
||||
"add_user_to_service": {"user_id", "invited_by_id", "service_id"},
|
||||
"set_user_permissions": {"user_id", "service_id", "original_admin_roles", "new_admin_roles", "set_by_id"},
|
||||
"archive_user": {"user_id", "archived_by_id"},
|
||||
"change_broadcast_account_type": {"service_id", "changed_by_id", "service_mode", "broadcast_channel", "provider_restriction"}, # noqa: E501 (length)
|
||||
"archive_service": {"service_id", "archived_by_id"},
|
||||
@@ -36,6 +37,10 @@ def create_add_user_to_service_event(**kwargs):
|
||||
_send_event('add_user_to_service', **kwargs)
|
||||
|
||||
|
||||
def create_set_user_permissions_event(**kwargs):
|
||||
_send_event('set_user_permissions', **kwargs)
|
||||
|
||||
|
||||
def create_archive_user_event(**kwargs):
|
||||
_send_event('archive_user', **kwargs)
|
||||
|
||||
|
||||
@@ -141,6 +141,7 @@ def edit_user_permissions(service_id, user_id):
|
||||
service_id,
|
||||
permissions=form.permissions,
|
||||
folder_permissions=form.folder_permissions.data,
|
||||
set_by_id=current_user.id,
|
||||
)
|
||||
# Only change the auth type if this is supported for a service. If a user logs in with a
|
||||
# security key, we generally don't want them to be able to use something less secure.
|
||||
|
||||
@@ -4,7 +4,10 @@ from notifications_python_client.errors import HTTPError
|
||||
from notifications_utils.timezones import utc_string_to_aware_gmt_datetime
|
||||
from werkzeug.utils import cached_property
|
||||
|
||||
from app.event_handlers import create_add_user_to_service_event
|
||||
from app.event_handlers import (
|
||||
create_add_user_to_service_event,
|
||||
create_set_user_permissions_event,
|
||||
)
|
||||
from app.models import JSONModel, ModelList
|
||||
from app.models.organisation import Organisation
|
||||
from app.models.roles_and_permissions import (
|
||||
@@ -122,13 +125,20 @@ class User(JSONModel, UserMixin):
|
||||
datetime_string
|
||||
)
|
||||
|
||||
def set_permissions(self, service_id, permissions, folder_permissions):
|
||||
def set_permissions(self, service_id, permissions, folder_permissions, set_by_id):
|
||||
user_api_client.set_user_permissions(
|
||||
self.id,
|
||||
service_id,
|
||||
permissions=permissions,
|
||||
folder_permissions=folder_permissions,
|
||||
)
|
||||
create_set_user_permissions_event(
|
||||
user_id=self.id,
|
||||
service_id=service_id,
|
||||
original_admin_roles=self.permissions_for_service(service_id),
|
||||
new_admin_roles=permissions,
|
||||
set_by_id=set_by_id,
|
||||
)
|
||||
|
||||
def logged_in_elsewhere(self):
|
||||
return session.get('current_session_id') != self.current_session_id
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import pytest
|
||||
|
||||
from app.models.user import AnonymousUser, InvitedOrgUser, InvitedUser, User
|
||||
from tests.conftest import USER_ONE_ID
|
||||
from tests.conftest import SERVICE_ONE_ID, USER_ONE_ID
|
||||
|
||||
|
||||
def test_anonymous_user(notify_admin):
|
||||
@@ -135,3 +135,24 @@ def test_invited_org_user_from_session_uses_id(client, mocker, mock_get_invited_
|
||||
def test_invited_org_user_from_session_returns_none_if_nothing_present(client, mocker):
|
||||
mocker.patch.dict('app.models.user.session', values={}, clear=True)
|
||||
assert InvitedOrgUser.from_session() is None
|
||||
|
||||
|
||||
def test_set_permissions(client, mocker, active_user_view_permissions, fake_uuid):
|
||||
mock_api = mocker.patch('app.models.user.user_api_client.set_user_permissions')
|
||||
mock_event = mocker.patch('app.models.user.create_set_user_permissions_event')
|
||||
|
||||
User(active_user_view_permissions).set_permissions(
|
||||
service_id=SERVICE_ONE_ID,
|
||||
permissions={'manage_templates'},
|
||||
folder_permissions=[],
|
||||
set_by_id=fake_uuid,
|
||||
)
|
||||
|
||||
mock_api.assert_called_once()
|
||||
mock_event.assert_called_once_with(
|
||||
service_id=SERVICE_ONE_ID,
|
||||
user_id=active_user_view_permissions['id'],
|
||||
original_admin_roles={'view_activity'},
|
||||
new_admin_roles={'manage_templates'},
|
||||
set_by_id=fake_uuid,
|
||||
)
|
||||
|
||||
@@ -10,6 +10,7 @@ from app.event_handlers import (
|
||||
create_mobile_number_change_event,
|
||||
create_remove_user_from_service_event,
|
||||
create_resume_service_event,
|
||||
create_set_user_permissions_event,
|
||||
create_suspend_service_event,
|
||||
on_user_logged_in,
|
||||
)
|
||||
@@ -129,3 +130,16 @@ def test_resume_service(client, mock_events):
|
||||
|
||||
create_resume_service_event(**kwargs)
|
||||
mock_events.assert_called_with('resume_service', event_dict(**kwargs))
|
||||
|
||||
|
||||
def test_set_user_permissions(client, mock_events):
|
||||
kwargs = {
|
||||
"user_id": str(uuid.uuid4()),
|
||||
"service_id": str(uuid.uuid4()),
|
||||
"original_admin_roles": set("manage_templates"),
|
||||
"new_admin_roles": set("view_activity"),
|
||||
"set_by_id": str(uuid.uuid4()),
|
||||
}
|
||||
|
||||
create_set_user_permissions_event(**kwargs)
|
||||
mock_events.assert_called_with('set_user_permissions', event_dict(**kwargs))
|
||||
|
||||
Reference in New Issue
Block a user