From 128719a3668744b9210e77f4b4265c3a8087dc1f Mon Sep 17 00:00:00 2001 From: Guillaume Charest <1690085+gcharest@users.noreply.github.com> Date: Tue, 1 Oct 2024 14:04:35 +0000 Subject: [PATCH 1/4] feat: install pandas --- app/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/requirements.txt b/app/requirements.txt index 965e6d53..aa593332 100644 --- a/app/requirements.txt +++ b/app/requirements.txt @@ -14,6 +14,7 @@ google-api-core==2.19.2 google-auth==2.33.0 httpx==0.27.2 itsdangerous==2.2.0 +pandas==2.2.3 PyJWT==2.9.0 PyYAML!=6.0.0,!=5.4.0,!=5.4.1 python-dotenv==0.21.1 From 98f61ca17e3b263ad6b475c20e0e3d1aab2fdab4 Mon Sep 17 00:00:00 2001 From: Guillaume Charest <1690085+gcharest@users.noreply.github.com> Date: Tue, 1 Oct 2024 14:31:49 +0000 Subject: [PATCH 2/4] fix: add error handling for list groups with members --- .../google_workspace/google_directory.py | 28 +++++++---- .../google_workspace/test_google_directory.py | 50 +++++++++++++++++++ 2 files changed, 69 insertions(+), 9 deletions(-) diff --git a/app/integrations/google_workspace/google_directory.py b/app/integrations/google_workspace/google_directory.py index 94945851..55b33a80 100644 --- a/app/integrations/google_workspace/google_directory.py +++ b/app/integrations/google_workspace/google_directory.py @@ -203,16 +203,26 @@ def list_groups_with_members( groups_with_members = [] for group in groups: logger.info(f"Getting members for group: {group['email']}") - members = list_group_members( - group["email"], fields="members(email, role, type, status)" - ) + try: + members = list_group_members( + group["email"], fields="members(email, role, type, status)" + ) + except Exception as e: + logger.warning(f"Error getting members for group {group['email']}: {e}") + continue if members and members_details: detailed_members = [] - for member in members: - logger.info(f"Getting user details for member: {member['email']}") - detailed_members.append( - get_user(member["email"], fields="name, primaryEmail") + try: + for member in members: + logger.info(f"Getting user details for member: {member['email']}") + detailed_members.append( + get_user(member["email"], fields="name, primaryEmail") + ) + group["members"] = detailed_members + groups_with_members.append(group) + except Exception as e: + logger.warning( + f"Error getting user details for group {member['email']}: {e}" ) - group["members"] = detailed_members - groups_with_members.append(group) + continue return groups_with_members diff --git a/app/tests/integrations/google_workspace/test_google_directory.py b/app/tests/integrations/google_workspace/test_google_directory.py index 43782cd9..cd413e1b 100644 --- a/app/tests/integrations/google_workspace/test_google_directory.py +++ b/app/tests/integrations/google_workspace/test_google_directory.py @@ -393,6 +393,33 @@ def test_list_groups_with_members_filtered( assert mock_get_user.call_count == 2 +@patch("integrations.google_workspace.google_directory.list_groups") +@patch("integrations.google_workspace.google_directory.list_group_members") +@patch("integrations.google_workspace.google_directory.get_user") +def test_list_groups_with_members_error_in_list_group_members( + mock_get_user, + mock_list_group_members, + mock_list_groups, + google_groups, + google_group_members, + google_users, + google_groups_w_users +): + groups = google_groups(2) + group_members = [Exception("Error fetching group members"), google_group_members(2)] + users = google_users(2) + + mock_list_groups.return_value = groups + mock_list_group_members.side_effect = group_members + mock_get_user.side_effect = users + + # Only the second group should be processed + expected_groups_with_users = [groups[1]] + expected_groups_with_users[0]["members"] = users + + assert google_directory.list_groups_with_members() == expected_groups_with_users + + @patch("integrations.google_workspace.google_directory.list_groups") @patch("integrations.google_workspace.google_directory.list_group_members") @patch("integrations.google_workspace.google_directory.get_user") @@ -437,6 +464,29 @@ def test_list_groups_with_members_without_members_enabled( mock_get_user.assert_not_called() +@patch("integrations.google_workspace.google_directory.list_groups") +@patch("integrations.google_workspace.google_directory.list_group_members") +@patch("integrations.google_workspace.google_directory.get_user") +def test_list_groups_with_members_error_in_get_user( + mock_get_user, + mock_list_group_members, + mock_list_groups, + google_groups, + google_group_members, + google_users, +): + groups = google_groups(2) + group_members = [google_group_members(2), google_group_members(2)] + users = [Exception("Error fetching user details"), google_users(1)[0]] + + mock_list_groups.return_value = groups + mock_list_group_members.side_effect = group_members + mock_get_user.side_effect = users + + # No groups should be processed due to error in get_user + assert google_directory.list_groups_with_members() == [] + + @patch("integrations.google_workspace.google_directory.list_groups") def test_list_groups_with_members_skips_when_no_groups(mock_list_groups): mock_list_groups.return_value = [] From 2f1547b47157934d0abfb3086e6bba127f622301 Mon Sep 17 00:00:00 2001 From: Guillaume Charest <1690085+gcharest@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:26:59 +0000 Subject: [PATCH 3/4] feat: add tolerate errors support --- .../google_workspace/google_directory.py | 24 ++++++++----- .../google_workspace/test_google_directory.py | 36 ++++++++++++++++++- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/app/integrations/google_workspace/google_directory.py b/app/integrations/google_workspace/google_directory.py index 55b33a80..314e2349 100644 --- a/app/integrations/google_workspace/google_directory.py +++ b/app/integrations/google_workspace/google_directory.py @@ -175,6 +175,7 @@ def list_groups_with_members( members_details: bool = True, groups_filters: list = [], query: str | None = None, + tolerate_errors: bool = False, ): """List all groups in the Google Workspace domain with their members. @@ -183,6 +184,7 @@ def list_groups_with_members( members_details (bool): Include the members details in the response. groups_filters (list): List of filters to apply to the groups. query (str): The query to search for groups. + tolerate_errors (bool): Whether to include groups that encountered errors during member detail retrieval. Returns: list: A list of group objects with members. Any group without members will not be included. @@ -210,19 +212,25 @@ def list_groups_with_members( except Exception as e: logger.warning(f"Error getting members for group {group['email']}: {e}") continue + if members and members_details: detailed_members = [] - try: - for member in members: + error_occurred = False + for member in members: + try: logger.info(f"Getting user details for member: {member['email']}") detailed_members.append( get_user(member["email"], fields="name, primaryEmail") ) - group["members"] = detailed_members - groups_with_members.append(group) - except Exception as e: - logger.warning( - f"Error getting user details for group {member['email']}: {e}" - ) + except Exception as e: + logger.warning( + f"Error getting user details for member {member['email']}: {e}" + ) + error_occurred = True + if not tolerate_errors: + break + if error_occurred and not tolerate_errors: continue + group["members"] = detailed_members + groups_with_members.append(group) return groups_with_members diff --git a/app/tests/integrations/google_workspace/test_google_directory.py b/app/tests/integrations/google_workspace/test_google_directory.py index cd413e1b..e483a28c 100644 --- a/app/tests/integrations/google_workspace/test_google_directory.py +++ b/app/tests/integrations/google_workspace/test_google_directory.py @@ -403,7 +403,7 @@ def test_list_groups_with_members_error_in_list_group_members( google_groups, google_group_members, google_users, - google_groups_w_users + google_groups_w_users, ): groups = google_groups(2) group_members = [Exception("Error fetching group members"), google_group_members(2)] @@ -487,6 +487,40 @@ def test_list_groups_with_members_error_in_get_user( assert google_directory.list_groups_with_members() == [] +@patch("integrations.google_workspace.google_directory.list_groups") +@patch("integrations.google_workspace.google_directory.list_group_members") +@patch("integrations.google_workspace.google_directory.get_user") +def test_list_groups_with_members_tolerate_errors( + mock_get_user, + mock_list_group_members, + mock_list_groups, + google_groups, + google_group_members, + google_users, +): + groups = google_groups(2) + group_members = [google_group_members(2), google_group_members(2)] + users = [ + Exception("Error fetching user details"), + google_users(1)[0], + Exception("Error fetching user details"), + google_users(1)[0], + ] + + mock_list_groups.return_value = groups + mock_list_group_members.side_effect = group_members + mock_get_user.side_effect = users + + # Expected result should include both groups, with the second group having one member + expected_groups_with_users = [groups[0], groups[1]] + expected_groups_with_users[0]["members"] = [] + expected_groups_with_users[1]["members"] = [google_users(1)[0]] + + result = google_directory.list_groups_with_members(tolerate_errors=True) + + assert result == expected_groups_with_users + + @patch("integrations.google_workspace.google_directory.list_groups") def test_list_groups_with_members_skips_when_no_groups(mock_list_groups): mock_list_groups.return_value = [] From 42e02f649ea46a7c924e0031b0749a046c60219e Mon Sep 17 00:00:00 2001 From: Guillaume Charest <1690085+gcharest@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:57:51 +0000 Subject: [PATCH 4/4] feat: specify arguments instead of kwargs --- app/modules/aws/identity_center.py | 62 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/app/modules/aws/identity_center.py b/app/modules/aws/identity_center.py index 382795ef..e99cb58e 100644 --- a/app/modules/aws/identity_center.py +++ b/app/modules/aws/identity_center.py @@ -10,25 +10,30 @@ DRY_RUN = True -def synchronize(**kwargs): +def synchronize( + enable_users_sync: bool = True, + enable_user_create: bool = True, + enable_user_delete: bool = False, + enable_groups_sync: bool = True, + enable_membership_create: bool = True, + enable_membership_delete: bool = False, + query: str = "email:aws-*", + pre_processing_filters: list = [], +): """Sync the AWS Identity Center with the Google Workspace. Args: - enable_users_sync (bool): Toggle to sync users. - enable_groups_sync (bool): Toggle to sync groups. - query (str): The query to filter the Google Groups. - + enable_users_sync (bool): Enable the synchronization of users. Default is True. + enable_user_create (bool): Enable the creation of users. Default is True. + enable_user_delete (bool): Enable the deletion of users. Default is False. + enable_groups_sync (bool): Enable the synchronization of groups. Default is True. + enable_membership_create (bool): Enable the creation of group memberships. Default is True. + enable_membership_delete (bool): Enable the deletion of group memberships. Default is False. + query (str): The query to search for groups. + pre_processing_filters (list): List of filters to apply to the groups before processing the members. Returns: tuple: A tuple containing the users sync status and groups sync status. """ - enable_users_sync = kwargs.pop("enable_users_sync", True) - enable_user_create = kwargs.pop("enable_user_create", True) - enable_user_delete = kwargs.pop("enable_user_delete", False) - enable_groups_sync = kwargs.pop("enable_groups_sync", True) - enable_membership_create = kwargs.pop("enable_membership_create", True) - enable_membership_delete = kwargs.pop("enable_membership_delete", False) - query = kwargs.pop("query", "email:aws-*") - pre_processing_filters = kwargs.pop("pre_processing_filters", []) users_sync_status = None groups_sync_status = None @@ -56,7 +61,7 @@ def synchronize(**kwargs): if enable_users_sync: users_sync_status = sync_users( - source_users, target_users, enable_user_create, enable_user_delete, **kwargs + source_users, target_users, enable_user_create, enable_user_delete ) target_users = identity_store.list_users() @@ -67,7 +72,6 @@ def synchronize(**kwargs): target_users, enable_membership_create, enable_membership_delete, - **kwargs, ) logger.info("synchronize:Sync Completed") @@ -78,11 +82,11 @@ def synchronize(**kwargs): def sync_users( - source_users, - target_users, - enable_user_create=True, - enable_user_delete=False, - **kwargs, + source_users: list, + target_users: list, + enable_user_create: bool = True, + enable_user_delete: bool = False, + delete_target_all: bool = False, ): """Sync the users in the identity store. @@ -90,14 +94,13 @@ def sync_users( source_users (list): A list of users from the source system. target_users (list): A list of users in the identity store. - enable_user_create (bool): Enable creation of users. - enable_user_delete (bool): Enable deletion of users. - delete_target_all (bool): Mark all target users for deletion. + enable_user_create (bool): Enable creation of users. Default is True. + enable_user_delete (bool): Enable deletion of users. Default is False. + delete_target_all (bool): Mark all target users for deletion. Default is False. Returns: tuple: A tuple containing the users created and deleted. """ - delete_target_all = kwargs.get("delete_target_all", False) if delete_target_all: users_to_delete = target_users @@ -150,12 +153,11 @@ def sync_users( def sync_groups( - source_groups, - target_groups, - target_users, - enable_membership_create=True, - enable_membership_delete=False, - **kwargs, + source_groups: list, + target_groups: list, + target_users: list, + enable_membership_create: bool = True, + enable_membership_delete: bool = False, ): """Sync the groups in the identity store.