Skip to content

Commit

Permalink
fix serializer selection for routes containing 'roles' substring
Browse files Browse the repository at this point in the history
previously, the check for 'roles' in the route path used a simple
substring match, which could lead to incorrect serializer selection
if the path contained a username with 'roles' as part of it

- updated the condition to split the route by '/' and check each part
  for the 'roles' substring
- ensures that only valid 'roles' routes (e.g., '/groups/<uuid>/roles/')
  are matched
  • Loading branch information
petracihalova committed Jan 7, 2025
1 parent 69c3565 commit 84c0765
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
4 changes: 2 additions & 2 deletions rbac/management/group/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ def get_serializer_class(self):
"""Get serializer based on route."""
if "principals" in self.request.path:
return GroupPrincipalInputSerializer
if ROLES_KEY in self.request.path and self.request.method == "GET":
if ROLES_KEY in self.request.path.split("/") and self.request.method == "GET":
return GroupRoleSerializerOut
if ROLES_KEY in self.request.path:
if ROLES_KEY in self.request.path.split("/"):
return GroupRoleSerializerIn
if self.request.method in ("POST", "PUT"):
return GroupInputSerializer
Expand Down
18 changes: 16 additions & 2 deletions tests/internal/integration/test_integration_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class IntegrationViewsTests(IdentityRequest):
def setUp(self):
"""Set up the integration view tests."""
test_roles = ["Role Admin", "Role A", "Role B"]
test_principals = ["user_admin", "user_a", "user_b"]
test_principals = ["user_admin", "user_a", "user_b", "roles_test"]

super().setUp()
self.client = APIClient()
Expand Down Expand Up @@ -193,7 +193,7 @@ def test_groups_for_principal_valid_account(self):
self.assertEqual(response.data.get("meta").get("count"), 3)

def test_groups_for_principal_invalid_account(self):
"""Test that a /tenant/<id>/principal/<username>groups/ request from an external account fails."""
"""Test that a /tenant/<id>/principal/<username>/groups/ request from an external account fails."""
external_request_context = self._create_request_context(self.customer, self.user_data, is_internal=False)
request = external_request_context["request"]
response = self.client.get(
Expand All @@ -203,6 +203,20 @@ def test_groups_for_principal_invalid_account(self):
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_groups_for_principal_roles_in_username(self):
"""
Test that a request to /tenant/<id>/principal/<username>/groups/ from an internal account works
with "roles" substring in the principal's username (RHCLOUD-32144).
"""
response = self.client.get(
f"/_private/api/v1/integrations/tenant/{self.tenant.org_id}/principal/roles_test/groups/",
**self.request.META,
follow=True,
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
# Expecting ["Group All"]
self.assertEqual(response.data.get("meta").get("count"), 1)

@patch(
"management.principal.proxy.PrincipalProxy.request_filtered_principals",
return_value={
Expand Down

0 comments on commit 84c0765

Please sign in to comment.