From 84c0765ccca0fbfa6d4d19e87377265920ec22ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Tue, 7 Jan 2025 17:12:16 +0100 Subject: [PATCH] fix serializer selection for routes containing 'roles' substring 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//roles/') are matched --- rbac/management/group/view.py | 4 ++-- .../integration/test_integration_views.py | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/rbac/management/group/view.py b/rbac/management/group/view.py index 2d5773b3..6d295fd4 100644 --- a/rbac/management/group/view.py +++ b/rbac/management/group/view.py @@ -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 diff --git a/tests/internal/integration/test_integration_views.py b/tests/internal/integration/test_integration_views.py index d0b3b6c0..506f6e20 100644 --- a/tests/internal/integration/test_integration_views.py +++ b/tests/internal/integration/test_integration_views.py @@ -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() @@ -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//principal/groups/ request from an external account fails.""" + """Test that a /tenant//principal//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( @@ -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//principal//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={