Add audit event when a service manager changes someones profile

We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2026-04-27 12:40:55 -04:00 · 2019-04-03 11:47:46 +01:00
parent a249382e69
commit 88e9a0ff61
4 changed files with 123 additions and 17 deletions
--- a/app/event_handlers.py
+++ b/app/event_handlers.py
@@ -3,18 +3,43 @@ from flask import request
 from app import events_api_client


-def on_user_logged_in(sender, user):
-    _send_event(sender, event_type='sucessful_login', user=user)
+def on_user_logged_in(_sender, user):
+    _send_event(event_type='sucessful_login', user_id=user.id)


-def _send_event(sender, **kwargs):
+def create_email_change_event(user_id, updated_by_id, original_email_address, new_email_address):
+    _send_event(
+        event_type='update_user_email',
+        user_id=user_id,
+        updated_by_id=updated_by_id,
+        original_email_address=original_email_address,
+        new_email_address=new_email_address)
+
+
+def create_mobile_number_change_event(user_id, updated_by_id, original_mobile_number, new_mobile_number):
+    _send_event(
+        event_type='update_user_mobile_number',
+        user_id=user_id,
+        updated_by_id=updated_by_id,
+        original_mobile_number=original_mobile_number,
+        new_mobile_number=new_mobile_number)
+
+
+def _send_event(**kwargs):
    if not kwargs.get('event_type'):
        return

    event_data = _construct_event_data(request)
+    event_fields = ('user_id',
+                    'updated_by_id',
+                    'original_email_address',
+                    'new_email_address',
+                    'original_mobile_number',
+                    'new_mobile_number')

-    if kwargs.get('user'):
-        event_data['user_id'] = kwargs.get('user').id
+    for field in event_fields:
+        if kwargs.get(field):
+            event_data[field] = kwargs[field]

    events_api_client.create_event(kwargs['event_type'], event_data)

--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -16,6 +16,10 @@ from app import (
    service_api_client,
    user_api_client,
 )
+from app.event_handlers import (
+    create_email_change_event,
+    create_mobile_number_change_event,
+)
 from app.main import main
 from app.main.forms import (
    ChangeEmailForm,
@@ -196,6 +200,8 @@ def confirm_edit_user_email(service_id, user_id):
            user_api_client.update_user_attribute(str(user_id), email_address=new_email, updated_by=current_user.id)
        except HTTPError as e:
            abort(500, e)
+        else:
+            create_email_change_event(user.id, current_user.id, user.email_address, new_email)
        finally:
            session.pop('team_member_email_change', None)

@@ -254,6 +260,8 @@ def confirm_edit_user_mobile_number(service_id, user_id):
            user_api_client.update_user_attribute(str(user_id), mobile_number=new_number, updated_by=current_user.id)
        except HTTPError as e:
            abort(500, e)
+        else:
+            create_mobile_number_change_event(user.id, current_user.id, user.mobile_number, new_number)
        finally:
            session.pop('team_member_mobile_change', None)

--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -1112,19 +1112,30 @@ def test_confirm_edit_user_email_page_404s_for_non_team_member(
 def test_confirm_edit_user_email_changes_user_email(
    client_request,
    active_user_with_permissions,
-    mock_get_users_by_service,
+    api_user_active,
    service_one,
    mocker,
-    mock_get_user,
-    mock_update_user_attribute
+    mock_update_user_attribute,
 ):
+    # We want active_user_with_permissions (the current user) to update the email address for api_user_active
+    # By default both users would have the same id, so we change the id of api_user_active
+    api_user_active.id = str(uuid.uuid4())
+    mocker.patch('app.user_api_client.get_users_for_service',
+                 return_value=[api_user_active, active_user_with_permissions])
+    # get_user gets called twice - first to check if current user can see the page, then to see if the team member
+    # whose email address we're changing belongs to the service
+    mocker.patch('app.models.service.user_api_client.get_user',
+                 side_effect=[active_user_with_permissions, api_user_active])
+    mock_event_handler = mocker.patch('app.main.views.manage_users.create_email_change_event')
+
    new_email = 'new_email@gov.uk'
    with client_request.session_transaction() as session:
        session['team_member_email_change'] = new_email
+
    client_request.post(
        'main.confirm_edit_user_email',
        service_id=service_one['id'],
-        user_id=active_user_with_permissions.id,
+        user_id=api_user_active.id,
        _expected_status=302,
        _expected_redirect=url_for(
            'main.manage_users',
@@ -1132,11 +1143,17 @@ def test_confirm_edit_user_email_changes_user_email(
            _external=True,
        ),
    )
+
    mock_update_user_attribute.assert_called_once_with(
-        active_user_with_permissions.id,
+        api_user_active.id,
        email_address=new_email,
-        updated_by=mocker.ANY
+        updated_by=active_user_with_permissions.id
    )
+    mock_event_handler.assert_called_once_with(
+        api_user_active.id,
+        active_user_with_permissions.id,
+        api_user_active.email_address,
+        new_email)


 def test_confirm_edit_user_email_doesnt_change_user_email_for_non_team_member(
@@ -1313,19 +1330,31 @@ def test_confirm_edit_user_mobile_number_page_redirects_if_session_empty(
 def test_confirm_edit_user_mobile_number_changes_user_mobile_number(
    client_request,
    active_user_with_permissions,
-    mock_get_users_by_service,
+    api_user_active,
    service_one,
    mocker,
-    mock_get_user,
    mock_update_user_attribute
 ):
+    # We want active_user_with_permissions (the current user) to update the mobile number for api_user_active
+    # By default both users would have the same id, so we change the id of api_user_active
+    api_user_active.id = str(uuid.uuid4())
+
+    mocker.patch('app.user_api_client.get_users_for_service',
+                 return_value=[api_user_active, active_user_with_permissions])
+    # get_user gets called twice - first to check if current user can see the page, then to see if the team member
+    # whose mobile number we're changing belongs to the service
+    mocker.patch('app.models.service.user_api_client.get_user',
+                 side_effect=[active_user_with_permissions, api_user_active])
+    mock_event_handler = mocker.patch('app.main.views.manage_users.create_mobile_number_change_event')
+
    new_number = '07554080636'
    with client_request.session_transaction() as session:
        session['team_member_mobile_change'] = new_number
+
    client_request.post(
        'main.confirm_edit_user_mobile_number',
        service_id=SERVICE_ONE_ID,
-        user_id=active_user_with_permissions.id,
+        user_id=api_user_active.id,
        _expected_status=302,
        _expected_redirect=url_for(
            'main.manage_users',
@@ -1334,10 +1363,15 @@ def test_confirm_edit_user_mobile_number_changes_user_mobile_number(
        ),
    )
    mock_update_user_attribute.assert_called_once_with(
-        active_user_with_permissions.id,
+        api_user_active.id,
        mobile_number=new_number,
-        updated_by=mocker.ANY
+        updated_by=active_user_with_permissions.id
    )
+    mock_event_handler.assert_called_once_with(
+        api_user_active.id,
+        active_user_with_permissions.id,
+        api_user_active.mobile_number,
+        new_number)


 def test_confirm_edit_user_mobile_number_doesnt_change_user_mobile_for_non_team_member(
--- a/tests/app/test_event_handlers.py
+++ b/tests/app/test_event_handlers.py
@@ -1,6 +1,11 @@
+import uuid
 from unittest.mock import ANY

-from app.event_handlers import on_user_logged_in
+from app.event_handlers import (
+    create_email_change_event,
+    create_mobile_number_change_event,
+    on_user_logged_in,
+)


 def test_on_user_logged_in_calls_events_api(app_, api_user_active, mock_events):
@@ -11,3 +16,37 @@ def test_on_user_logged_in_calls_events_api(app_, api_user_active, mock_events):
                                       {'browser_fingerprint':
                                        {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''},
                                        'ip_address': ANY, 'user_id': str(api_user_active.id)})
+
+
+def test_create_email_change_event_calls_events_api(app_, mock_events):
+    user_id = str(uuid.uuid4())
+    updated_by_id = str(uuid.uuid4())
+
+    with app_.test_request_context():
+        create_email_change_event(user_id, updated_by_id, 'original@example.com', 'new@example.com')
+
+        mock_events.assert_called_with('update_user_email',
+                                       {'browser_fingerprint':
+                                        {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''},
+                                        'ip_address': ANY,
+                                        'user_id': user_id,
+                                        'updated_by_id': updated_by_id,
+                                        'original_email_address': 'original@example.com',
+                                        'new_email_address': 'new@example.com'})
+
+
+def test_create_mobile_number_change_event_calls_events_api(app_, mock_events):
+    user_id = str(uuid.uuid4())
+    updated_by_id = str(uuid.uuid4())
+
+    with app_.test_request_context():
+        create_mobile_number_change_event(user_id, updated_by_id, '07700900000', '07700900999')
+
+        mock_events.assert_called_with('update_user_mobile_number',
+                                       {'browser_fingerprint':
+                                        {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''},
+                                        'ip_address': ANY,
+                                        'user_id': user_id,
+                                        'updated_by_id': updated_by_id,
+                                        'original_mobile_number': '07700900000',
+                                        'new_mobile_number': '07700900999'})