Audit permissions when adding a user to a service

This is useful information to store for the event, which would be
lost if someone subsequently changed them.

Rather than updating lots of mock assertions, I've replaced them
with a single test / assert at a lower level, which is consistent
with auditing being a non-critical function.
This commit is contained in:
Ben Thorner
2021-07-15 12:13:42 +01:00
parent 171f911237
commit 9fafc092f7
6 changed files with 24 additions and 23 deletions

View File

@@ -7,7 +7,7 @@ EVENT_SCHEMAS = {
"update_user_email": {"user_id", "updated_by_id", "original_email_address", "new_email_address"},
"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"},
"add_user_to_service": {"user_id", "invited_by_id", "service_id", "admin_roles"},
"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)

View File

@@ -425,6 +425,7 @@ class User(JSONModel, UserMixin):
user_id=self.id,
invited_by_id=invited_by_id,
service_id=service_id,
admin_roles=permissions,
)
except HTTPError as exception:
if exception.status_code == 400 and 'already part of service' in exception.message:

View File

@@ -46,7 +46,6 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(
):
expected_service = service_one['id']
expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'}
mock_audit_event = mocker.patch('app.models.user.create_add_user_to_service_event')
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'))
@@ -62,11 +61,6 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(
assert response.status_code == 302
assert response.location == url_for('main.service_dashboard', service_id=expected_service, _external=True)
mock_audit_event.assert_called_once_with(
invited_by_id=service_one['users'][0],
service_id=SERVICE_ONE_ID,
user_id=api_user_active['id'],
)
@pytest.mark.parametrize('trial_mode, expected_endpoint', (
@@ -270,8 +264,6 @@ def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_pa
mock_no_users_for_service,
mock_get_user,
):
mock_audit_event = mocker.patch('app.models.user.create_add_user_to_service_event')
mocker.patch('app.user_api_client.add_user_to_service', side_effect=HTTPError(
response=Mock(
status_code=400,
@@ -285,7 +277,6 @@ def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_pa
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=False)
assert response.location == url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True)
assert not mock_audit_event.called
def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(
@@ -574,8 +565,6 @@ def test_new_invited_user_verifies_and_added_to_service(
mock_create_event,
mocker,
):
mock_audit_event = mocker.patch('app.models.user.create_add_user_to_service_event')
# visit accept token page
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'))
assert response.status_code == 302
@@ -611,10 +600,6 @@ def test_new_invited_user_verifies_and_added_to_service(
mock_check_verify_code.assert_called_once_with(new_user_id, '12345', 'sms')
assert service_one['id'] == session['service_id']
mock_audit_event.assert_called_once_with(invited_by_id=service_one['users'][0],
service_id=service_one['id'],
user_id=new_user_id)
raw_html = response.data.decode('utf-8')
page = BeautifulSoup(raw_html, 'html.parser')
assert page.find('h1').text == 'Dashboard'

View File

@@ -334,8 +334,6 @@ def test_register_from_email_auth_invite(
fake_uuid,
mocker,
):
mock_audit_event = mocker.patch('app.models.user.create_add_user_to_service_event')
sample_invite['auth_type'] = 'email_auth'
sample_invite['email_address'] = invite_email_address
with client.session_transaction() as session:
@@ -373,10 +371,6 @@ def test_register_from_email_auth_invite(
assert current_user.is_authenticated
assert mock_add_user_to_service.called
mock_audit_event.assert_called_once_with(invited_by_id=service_one['users'][0],
service_id=sample_invite['service'],
user_id=fake_uuid)
with client.session_transaction() as session:
# invited user details are still there so they can get added to the service
assert session['invited_user_id'] == sample_invite['id']

View File

@@ -156,3 +156,23 @@ def test_set_permissions(client, mocker, active_user_view_permissions, fake_uuid
new_admin_roles={'manage_templates'},
set_by_id=fake_uuid,
)
def test_add_to_service(client, mocker, api_user_active, fake_uuid):
mock_api = mocker.patch('app.models.user.user_api_client.add_user_to_service')
mock_event = mocker.patch('app.models.user.create_add_user_to_service_event')
User(api_user_active).add_to_service(
service_id=SERVICE_ONE_ID,
permissions={'manage_templates'},
folder_permissions=[],
invited_by_id=fake_uuid,
)
mock_api.assert_called_once()
mock_event.assert_called_once_with(
service_id=SERVICE_ONE_ID,
user_id=api_user_active['id'],
invited_by_id=fake_uuid,
admin_roles={'manage_templates'},
)

View File

@@ -49,7 +49,8 @@ def test_create_add_user_to_service_event_calls_events_api(client, mock_events):
kwargs = {
"user_id": str(uuid.uuid4()),
"invited_by_id": str(uuid.uuid4()),
"service_id": str(uuid.uuid4())
"service_id": str(uuid.uuid4()),
"admin_roles": {'manage_templates'},
}
create_add_user_to_service_event(**kwargs)