Fix access request completed notification parameter for ModifyGroupRo…

…les (#42)

api/operations/modify_role_groups.py

@@ -517,7 +517,7 @@ async def _notify_access_request(self, access_request: AccessRequest) -> None:
 
         self.notification_hook.access_request_completed(
             access_request=access_request,
-            group=access_request.requested_group.name,
+            group=access_request.requested_group,
             requester=requester,
             approvers=approvers,
             notify_requester=True,

api/plugins/conditional_access.py

@@ -48,7 +48,7 @@ def access_request_created(
         # Log and do not raise since request failures should not
         # break the flow. The access request can still be manually
         # approved or denied
-        logger.error("Failed to execute request created callback")
+        logger.exception("Failed to execute request created callback")
 
     return []
 

api/plugins/notifications.py

@@ -62,7 +62,7 @@ def access_request_created(access_request: AccessRequest,
         # Log and do not raise since notification failures should not
         # break the flow. Users can still manually ping approvers
         # to process their request from the UI
-        logger.error("Failed to execute access request created notification callback")
+        logger.exception("Failed to execute access request created notification callback")
 
 
 @hookimpl(wrapper=True)
@@ -77,7 +77,7 @@ def access_request_completed(access_request: AccessRequest,
         # Log and do not raise since notification failures should not
         # break the flow. Users can still manually ping approvers
         # to process their request from the UI
-        logger.error("Failed to execute access request completed notification callback")
+        logger.exception("Failed to execute access request completed notification callback")
 
 
 @hookimpl(wrapper=True)
@@ -90,7 +90,7 @@ def access_expiring_user(groups: list[OktaGroup],
         # Log and do not raise since notification failures should not
         # break the flow. Users can still manually ping approvers
         # to process their request from the UI
-        logger.error("Failed to execute access expiring for user notification callback")
+        logger.exception("Failed to execute access expiring for user notification callback")
 
 @hookimpl(wrapper=True)
 def access_expiring_owner(owner: OktaUser,
@@ -104,7 +104,7 @@ def access_expiring_owner(owner: OktaUser,
         # Log and do not raise since notification failures should not
         # break the flow. Users can still manually ping approvers
         # to process their request from the UI
-        logger.error("Failed to execute access expiring for owner notification callback")
+        logger.exception("Failed to execute access expiring for owner notification callback")
 
 
 def get_notification_hook() -> pluggy.HookRelay:

tests/test_access_request.py

@@ -415,6 +415,10 @@ def test_create_app_access_request_notification(
 
     assert access_request is not None
     assert request_created_notification_spy.call_count == 1
+    _, kwargs = request_created_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
 
     access_owner = OktaUser.query.filter(
         OktaUser.email == app.config["CURRENT_OKTA_USER_EMAIL"]
@@ -426,6 +430,10 @@ def test_create_app_access_request_notification(
 
     assert add_membership_spy.call_count == 1
     assert request_completed_notification_spy.call_count == 1
+    _, kwargs = request_completed_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
 
     access_request.status = AccessRequestStatus.PENDING
     access_request.resolved_at = None
@@ -440,6 +448,10 @@ def test_create_app_access_request_notification(
 
     assert add_membership_spy.call_count == 0
     assert request_completed_notification_spy.call_count == 1
+    _, kwargs = request_completed_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
 
 def test_get_all_possible_request_approvers(app: Flask, mocker: MockerFixture, db: SQLAlchemy) -> None:
     access_admin = OktaUser.query.filter(OktaUser.email == app.config["CURRENT_OKTA_USER_EMAIL"]).first()
@@ -528,6 +540,9 @@ def test_resolve_app_access_request_notification(
     assert access_request is not None
     assert request_created_notification_spy.call_count == 1
     _, kwargs = request_created_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
     assert len(kwargs['approvers']) == 2
     assert app_owner_user1 in kwargs['approvers']
     assert app_owner_user2 in kwargs['approvers']
@@ -539,6 +554,9 @@ def test_resolve_app_access_request_notification(
     assert add_ownership_spy.call_count == 1
     assert request_completed_notification_spy.call_count == 1
     _, kwargs = request_completed_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
     assert len(kwargs['approvers']) == 4
     assert access_admin in kwargs['approvers']
     assert app_owner_user1 in kwargs['approvers']
@@ -560,6 +578,9 @@ def test_resolve_app_access_request_notification(
     assert add_ownership_spy.call_count == 0
     assert request_completed_notification_spy.call_count == 1
     _, kwargs = request_completed_notification_spy.call_args
+    assert kwargs['access_request'] == access_request
+    assert kwargs['group'] == app_group
+    assert kwargs['requester'] == user
     assert len(kwargs['approvers']) == 4
     assert access_admin in kwargs['approvers']
     assert app_owner_user1 in kwargs['approvers']