Skip to content

Commit

Permalink
Merge pull request RedHatInsights#1160 from petracihalova/service-acc…
Browse files Browse the repository at this point in the history
…ounts-removal

RHCLOUD-30376 Service accounts removal from a group
  • Loading branch information
petracihalova authored Aug 9, 2024
2 parents 9cdbf12 + 386d4f8 commit 55b6bd8
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion docs/source/specs/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
17 changes: 9 additions & 8 deletions rbac/management/group/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -1100,27 +1100,28 @@ 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():
for service_account in valid_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")
10 changes: 5 additions & 5 deletions tests/management/group/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 55b6bd8

Please sign in to comment.