Skip to content

Commit a92b7c0

Browse files
committed
enhance group sync handling for public groups
1 parent b7faa92 commit a92b7c0

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

backend/ee/onyx/external_permissions/sharepoint/permission_utils.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def process_users(users: list[Any]) -> None:
234234
nonlocal groups, user_emails
235235

236236
for user in users:
237-
logger.info(f"User: {user.to_json()}")
237+
logger.debug(f"User: {user.to_json()}")
238238
if user.principal_type == USER_PRINCIPAL_TYPE and hasattr(
239239
user, "user_principal_name"
240240
):
@@ -289,7 +289,7 @@ def process_members(members: list[Any]) -> None:
289289

290290
for member in members:
291291
member_data = member.to_json()
292-
logger.info(f"Member: {member_data}")
292+
logger.debug(f"Member: {member_data}")
293293
# Check for user-specific attributes
294294
user_principal_name = member_data.get("userPrincipalName")
295295
mail = member_data.get("mail")
@@ -370,13 +370,15 @@ def _get_groups_and_members_recursively(
370370
client_context: ClientContext,
371371
graph_client: GraphClient,
372372
groups: set[SharepointGroup],
373+
is_group_sync: bool = False,
373374
) -> GroupsResult:
374375
"""
375376
Get all groups and their members recursively.
376377
"""
377378
group_queue: deque[SharepointGroup] = deque(groups)
378379
visited_groups: set[str] = set()
379380
visited_group_name_to_emails: dict[str, set[str]] = {}
381+
found_public_group = False
380382
while group_queue:
381383
group = group_queue.popleft()
382384
if group.login_name in visited_groups:
@@ -397,8 +399,14 @@ def _get_groups_and_members_recursively(
397399
try:
398400
# if the site is public, we have default groups assigned to it, so we return early
399401
if _is_public_login_name(group.login_name):
400-
return GroupsResult(groups_to_emails={}, found_public_group=True)
401-
402+
found_public_group = True
403+
if not is_group_sync:
404+
return GroupsResult(
405+
groups_to_emails={}, found_public_group=True
406+
)
407+
else:
408+
# we don't want to sync public groups, so we skip them
409+
continue
402410
group_info, user_emails = _get_azuread_groups(
403411
graph_client, group.login_name
404412
)
@@ -415,7 +423,8 @@ def _get_groups_and_members_recursively(
415423
raise e
416424

417425
return GroupsResult(
418-
groups_to_emails=visited_group_name_to_emails, found_public_group=False
426+
groups_to_emails=visited_group_name_to_emails,
427+
found_public_group=found_public_group,
419428
)
420429

421430

@@ -440,7 +449,7 @@ def add_user_and_group_to_sets(
440449
) -> None:
441450
nonlocal user_emails, groups
442451
for assignment in role_assignments:
443-
logger.info(f"Assignment: {assignment.to_json()}")
452+
logger.debug(f"Assignment: {assignment.to_json()}")
444453
if assignment.role_definition_bindings:
445454
is_limited_access = True
446455
for role_definition_binding in assignment.role_definition_bindings:
@@ -616,13 +625,9 @@ def add_group_to_sets(role_assignments: RoleAssignmentCollection) -> None:
616625
"get_sharepoint_external_groups",
617626
)
618627
groups_and_members: GroupsResult = _get_groups_and_members_recursively(
619-
client_context, graph_client, groups
628+
client_context, graph_client, groups, is_group_sync=True
620629
)
621630

622-
# We don't have any direct way to check if the site is public, so we check if any public group is present
623-
if groups_and_members.found_public_group:
624-
return []
625-
626631
# get all Azure AD groups because if any group is assigned to the drive item, we don't want to miss them
627632
# We can't assign sharepoint groups to drive items or drives, so we don't need to get all sharepoint groups
628633
azure_ad_groups = sleep_and_retry(

0 commit comments

Comments
 (0)