fix: #1367 Fix and simplify audit log (#1498)

server/admin_management/api/app/routers/router_access_control_privilege.py

@@ -2,7 +2,7 @@
 from typing import List
 
 from api.app import jwt_validation, schemas
-from api.app.models import model as models
+from api.app.models.model import FamUser
 from api.app.routers.router_guards import (
     authorize_by_app_id,
     authorize_by_application_role,
@@ -76,34 +76,36 @@ def create_access_control_privilege_many(
     )
 
     try:
-        audit_event_log.requesting_user = user_service.get_user_by_cognito_user_id(
-            requester.cognito_user_id
-        )
+        audit_event_log.requesting_user = requester
         audit_event_log.role = role_service.get_role_by_id(
             access_control_privilege_request.role_id
         )
         audit_event_log.application = audit_event_log.role.application
 
-        return access_control_privilege_service.create_access_control_privilege_many(
+        response = access_control_privilege_service.create_access_control_privilege_many(
             access_control_privilege_request, requester.cognito_user_id, target_user
         )
+        # get target user from database, so for existing user, we can get the cognito user id
+        audit_event_log.target_user = user_service.get_user_by_domain_and_guid(
+            access_control_privilege_request.user_type_code,
+            access_control_privilege_request.user_guid,
+        )
+
+        return  response
 
     except Exception as e:
         audit_event_log.event_outcome = AuditEventOutcome.FAIL
         audit_event_log.exception = e
         raise e
 
     finally:
-        audit_event_log.target_user = user_service.get_user_by_domain_and_guid(
-            access_control_privilege_request.user_type_code,
-            access_control_privilege_request.user_guid,
-        )
+        # if failed to get target user from database, use the information from request
         if audit_event_log.target_user is None:
-            audit_event_log.target_user = models.FamUser(
-                user_type_code=access_control_privilege_request.user_type_code,
-                user_name=access_control_privilege_request.user_name,
-                user_guid="unknown",
-                cognito_user_id="unknown",
+            audit_event_log.target_user = FamUser(
+                user_type_code=target_user.user_type_code,
+                user_name=target_user.user_name,
+                user_guid=target_user.user_guid,
+                cognito_user_id=target_user.cognito_user_id,
             )
 
         audit_event_log.log_event()
@@ -141,7 +143,6 @@ def get_access_control_privileges_by_application_id(
 def delete_access_control_privilege(
     access_control_privilege_id: int,
     request: Request,
-    user_service: UserService = Depends(user_service_instance),
     access_control_privilege_service: AccessControlPrivilegeService = Depends(
         access_control_privilege_service_instance
     ),
@@ -158,9 +159,7 @@ def delete_access_control_privilege(
     )
 
     try:
-        audit_event_log.requesting_user = user_service.get_user_by_cognito_user_id(
-            requester.cognito_user_id
-        )
+        audit_event_log.requesting_user = requester
         access_control_privilege = access_control_privilege_service.get_acp_by_id(
             access_control_privilege_id
         )

server/admin_management/api/app/routers/router_application_admin.py

@@ -2,23 +2,26 @@
 from typing import List
 
 from api.app import database, jwt_validation, schemas
-from api.app.models import model as models
-from api.app.routers.router_guards import (authorize_by_fam_admin,
-                                           enforce_self_grant_guard,
-                                           get_current_requester,
-                                           get_verified_target_user,
-                                           validate_param_application_admin_id,
-                                           validate_param_application_id,
-                                           validate_param_user_type)
-from api.app.routers.router_utils import (application_admin_service_instance,
-                                          application_service_instance,
-                                          user_service_instance)
+from api.app.models.model import FamUser
+from api.app.routers.router_guards import (
+    authorize_by_fam_admin,
+    enforce_self_grant_guard,
+    get_current_requester,
+    get_verified_target_user,
+    validate_param_application_admin_id,
+    validate_param_application_id,
+    validate_param_user_type,
+)
+from api.app.routers.router_utils import (
+    application_admin_service_instance,
+    application_service_instance,
+    user_service_instance,
+)
 from api.app.schemas import Requester, TargetUser
 from api.app.services.application_admin_service import ApplicationAdminService
 from api.app.services.application_service import ApplicationService
 from api.app.services.user_service import UserService
-from api.app.utils.audit_util import (AuditEventLog, AuditEventOutcome,
-                                      AuditEventType)
+from api.app.utils.audit_util import AuditEventLog, AuditEventOutcome, AuditEventType
 from fastapi import APIRouter, Depends, Request, Response
 from sqlalchemy.orm import Session
 
@@ -75,33 +78,36 @@ def create_application_admin(
     )
 
     try:
-        audit_event_log.requesting_user = user_service.get_user_by_cognito_user_id(
-            requester.cognito_user_id
-        )
+        audit_event_log.requesting_user = requester
         audit_event_log.application = application_service.get_application(
             application_admin_request.application_id
         )
 
-        return application_admin_service.create_application_admin(
+        response = application_admin_service.create_application_admin(
             application_admin_request, target_user, requester.cognito_user_id
         )
 
+        # get target user from database, so for existing user, we can get the cognito user id
+        audit_event_log.target_user = user_service.get_user_by_domain_and_guid(
+            application_admin_request.user_type_code,
+            application_admin_request.user_guid,
+        )
+
+        return response
+
     except Exception as e:
         audit_event_log.event_outcome = AuditEventOutcome.FAIL
         audit_event_log.exception = e
         raise e
 
     finally:
-        audit_event_log.target_user = user_service.get_user_by_domain_and_guid(
-            application_admin_request.user_type_code,
-            application_admin_request.user_guid,
-        )
+        # if failed to get target user from database, use the information from request
         if audit_event_log.target_user is None:
-            audit_event_log.target_user = models.FamUser(
-                user_type_code=application_admin_request.user_type_code,
-                user_name=application_admin_request.user_name,
-                user_guid="unknown",
-                cognito_user_id="unknown",
+            audit_event_log.target_user = FamUser(
+                user_type_code=target_user.user_type_code,
+                user_name=target_user.user_name,
+                user_guid=target_user.user_guid,
+                cognito_user_id=target_user.cognito_user_id,
             )
 
         audit_event_log.log_event()
@@ -134,14 +140,10 @@ def delete_application_admin(
 
     try:
         application_admin_service = ApplicationAdminService(db)
-        user_service = UserService(db)
-
         application_admin = application_admin_service.get_application_admin_by_id(
             application_admin_id
         )
-        audit_event_log.requesting_user = user_service.get_user_by_cognito_user_id(
-            requester.cognito_user_id
-        )
+        audit_event_log.requesting_user = requester
         audit_event_log.application = application_admin.application
         audit_event_log.target_user = application_admin.user
 

server/backend/api/app/routers/router_user_role_assignment.py

@@ -2,15 +2,19 @@
 from http import HTTPStatus
 
 from api.app.crud import crud_role, crud_user, crud_user_role
-from api.app.models import model as models
 from api.app.routers.router_guards import (
-    authorize_by_application_role, authorize_by_privilege,
-    authorize_by_user_type, enforce_bceid_by_same_org_guard,
-    enforce_bceid_terms_conditions_guard, enforce_self_grant_guard,
-    get_current_requester, get_verified_target_user)
+    authorize_by_application_role,
+    authorize_by_privilege,
+    authorize_by_user_type,
+    enforce_bceid_by_same_org_guard,
+    enforce_bceid_terms_conditions_guard,
+    enforce_self_grant_guard,
+    get_current_requester,
+    get_verified_target_user,
+)
 from api.app.schemas import Requester, TargetUser
-from api.app.utils.audit_util import (AuditEventLog, AuditEventOutcome,
-                                      AuditEventType)
+from api.app.models.model import FamUser
+from api.app.utils.audit_util import AuditEventLog, AuditEventOutcome, AuditEventType
 from fastapi import APIRouter, Depends, Request, Response
 from sqlalchemy.orm import Session
 
@@ -67,41 +71,39 @@ def create_user_role_assignment(
     )
 
     try:
-
-        requesting_user = get_requesting_user(db, requester.cognito_user_id)
         role = crud_role.get_role(db, role_assignment_request.role_id)
 
         audit_event_log.role = role
         audit_event_log.application = role.application
-        audit_event_log.requesting_user = requesting_user
+        audit_event_log.requesting_user = requester
 
-        return crud_user_role.create_user_role(
+        response = crud_user_role.create_user_role(
+            db, role_assignment_request, target_user, requester.cognito_user_id
+        )
+        # get target user from database, so for existing user, we can get the cognito user id
+        audit_event_log.target_user = crud_user.get_user_by_domain_and_guid(
             db,
-            role_assignment_request,
-            target_user,
-            requesting_user.cognito_user_id
+            role_assignment_request.user_type_code,
+            role_assignment_request.user_guid,
         )
 
+        return response
+
     except Exception as e:
         audit_event_log.event_outcome = AuditEventOutcome.FAIL
         audit_event_log.exception = e
 
         raise e
 
     finally:
-        audit_event_log.target_user = crud_user.get_user_by_domain_and_name(
-            db,
-            role_assignment_request.user_type_code,
-            role_assignment_request.user_name,
-        )
+        # if failed to get target user from database, use the information from request
         if audit_event_log.target_user is None:
-            audit_event_log.target_user = models.FamUser(
-                user_type_code=role_assignment_request.user_type_code,
-                user_name=role_assignment_request.user_name,
-                user_guid="unknown",
-                cognito_user_id="unknown",
+            audit_event_log.target_user = FamUser(
+                user_type_code=target_user.user_type_code,
+                user_name=target_user.user_name,
+                user_guid=target_user.user_guid,
+                cognito_user_id=target_user.cognito_user_id,
             )
-
         audit_event_log.log_event()
 
 
@@ -144,13 +146,12 @@ def delete_user_role_assignment(
     )
 
     try:
-        requesting_user = get_requesting_user(db, requester.cognito_user_id)
         user_role = crud_user_role.find_by_id(db, user_role_xref_id)
 
         audit_event_log.role = user_role.role
         audit_event_log.target_user = user_role.user
         audit_event_log.application = user_role.role.application
-        audit_event_log.requesting_user = requesting_user
+        audit_event_log.requesting_user = requester
 
         crud_user_role.delete_fam_user_role_assignment(db, user_role_xref_id)
 
@@ -165,8 +166,3 @@ def delete_user_role_assignment(
                 user_role.role.client_number.forest_client_number
             )
         audit_event_log.log_event()
-
-
-def get_requesting_user(db: Session, cognito_user_id: str) -> models.FamUser:
-    requester = crud_user.get_user_by_cognito_user_id(db, cognito_user_id)
-    return requester

server/backend/api/app/utils/audit_util.py

@@ -2,6 +2,7 @@
 from enum import Enum
 import json
 from api.app.models import model as models
+from api.app.schemas import Requester
 from fastapi import Request, HTTPException
 
 
@@ -25,7 +26,7 @@ class AuditEventLog:
     application: models.FamApplication
     role: models.FamRole
     forest_client_number: str
-    requesting_user: models.FamUser
+    requesting_user: Requester
     target_user: models.FamUser
     exception: Exception
 
@@ -37,7 +38,7 @@ def __init__(
         application: models.FamApplication = None,
         role: models.FamRole = None,
         forest_client_number: str = None,
-        requesting_user: models.FamUser = None,
+        requesting_user: Requester = None,
         target_user: models.FamUser = None,
         exception: Exception = None,
     ):