diff --git a/docs/source/specs/openapi.json b/docs/source/specs/openapi.json index 5f4b88896..2a4e15e99 100644 --- a/docs/source/specs/openapi.json +++ b/docs/source/specs/openapi.json @@ -1378,7 +1378,7 @@ { "name": "service-accounts", "in": "query", - "description": "A comma separated list of usernames for service accounts to remove from the group", + "description": "A comma separated list of client IDs for service accounts to remove from the group", "schema": { "type": "string" } diff --git a/rbac/management/group/view.py b/rbac/management/group/view.py index 2fa252a97..f956d2a83 100644 --- a/rbac/management/group/view.py +++ b/rbac/management/group/view.py @@ -1100,19 +1100,19 @@ def remove_service_accounts( # Get the group's service accounts that match the service accounts that the user specified. valid_service_accounts = Principal.objects.filter( - group=group, tenant=tenant, type="service-account", username__in=service_accounts + group=group, tenant=tenant, type="service-account", service_account_id__in=service_accounts ) - # Collect the usernames the user specified. - valid_usernames = valid_service_accounts.values_list("username", flat=True) + # Collect the service account IDs the user specified. + valid_service_account_ids = valid_service_accounts.values_list("service_account_id", flat=True) # If there is a difference in the sets, then we know that the user specified service accounts # that did not exist in the database. - usernames_diff = set(service_accounts).difference(valid_usernames) - if usernames_diff: - logger.info(f"Service accounts {usernames_diff} not found for org id {org_id}.") + service_account_ids_diff = set(service_accounts).difference(valid_service_account_ids) + if service_account_ids_diff: + logger.info(f"Service accounts {service_account_ids_diff} not found for org id {org_id}.") - raise ValueError(f"Service account(s) {usernames_diff} not found in the group '{group.name}'") + raise ValueError(f"Service account(s) {service_account_ids_diff} not found in the group '{group.name}'") # Remove service accounts from the group. with transaction.atomic(): @@ -1120,7 +1120,8 @@ def remove_service_accounts( group.principals.remove(service_account) logger.info( - f"[Request_id:{request_id}] {valid_usernames} removed from group {group.name} for org id {org_id}." + f"[Request_id:{request_id}] {valid_service_account_ids} " + f"removed from group {group.name} for org id {org_id}." ) for username in service_accounts: group_principal_change_notification_handler(self.request.user, group, username, "removed") diff --git a/tests/management/group/test_view.py b/tests/management/group/test_view.py index 38948aee5..db4ebf4a4 100644 --- a/tests/management/group/test_view.py +++ b/tests/management/group/test_view.py @@ -3065,7 +3065,7 @@ def test_group_service_account_with_user_administrator_role_remove_principals(se # Generate the URL for removing the principals from the regular group. group_principals_url = reverse("group-principals", kwargs={"uuid": regular_group.uuid}) - group_principals_url = f"{group_principals_url}?service-accounts={service_account_principal_to_delete.username}&usernames={user_principal_to_remove.username}" + group_principals_url = f"{group_principals_url}?service-accounts={service_account_principal_to_delete.service_account_id}&usernames={user_principal_to_remove.username}" api_client = APIClient() # Make sure that before calling the endpoint the group has the two principals. @@ -3355,7 +3355,7 @@ def test_group_user_with_user_administrator_role_remove_principals(self): # Generate the URL for removing the principals from the regular group. group_principals_url = reverse("group-principals", kwargs={"uuid": regular_group.uuid}) - group_principals_url = f"{group_principals_url}?service-accounts={service_account_principal_to_delete.username}&usernames={user_principal_to_remove.username}" + group_principals_url = f"{group_principals_url}?service-accounts={service_account_principal_to_delete.service_account_id}&usernames={user_principal_to_remove.username}" api_client = APIClient() # Make sure that before calling the endpoint the group has the two principals. @@ -4090,7 +4090,7 @@ def test_remove_service_account_principal_from_group_without_User_Access_Admin_f url = ( reverse("group-principals", kwargs={"uuid": test_group.uuid}) - + f"?service-accounts={sa_principal.username}" + + f"?service-accounts={sa_principal.service_account_id}" ) client = APIClient() @@ -4160,7 +4160,7 @@ def test_remove_service_account_principal_from_group_with_User_Access_Admin_succ url = ( reverse("group-principals", kwargs={"uuid": test_group.uuid}) - + f"?service-accounts={sa_principal.username}" + + f"?service-accounts={sa_principal.service_account_id}" ) client = APIClient() @@ -4252,7 +4252,7 @@ def test_remove_service_account_principal_from_group_with_User_Access_Admin_fail url = ( reverse("group-principals", kwargs={"uuid": test_group.uuid}) - + f"?service-accounts={sa_principal.username}" + + f"?service-accounts={sa_principal.service_account_id}" ) client = APIClient()