From 710ff4bda56af6f1519f515b15e6a0ba31d6fd03 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Fri, 29 Nov 2024 16:29:36 +0000 Subject: [PATCH 01/16] [WIP][RHCLOUD-36102] - Slow loading of data from roles endpoint performance fixes --- rbac/management/role/view.py | 66 ++++++++++++++++++- .../integration/test_integration_views.py | 3 +- tests/management/role/test_view.py | 3 +- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 0fcc42d5d..49416a73b 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -25,7 +25,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.db import IntegrityError, transaction -from django.db.models import Q +from django.db.models import F, Q from django.db.models.aggregates import Count from django.http import Http404 from django.utils.translation import gettext as _ @@ -48,7 +48,7 @@ from api.models import Tenant from rbac.env import ENVIRONMENT -from .model import Role +from .model import ExtTenant, Role from .serializer import RoleSerializer TESTING_APP = os.getenv("TESTING_APPLICATION") @@ -324,7 +324,67 @@ def list(self, request, *args, **kwargs): } """ - return super().list(request=request, args=args, kwargs=kwargs) + public_tenant = Tenant.objects.get(tenant_name="public") + base_queryset = Role.objects.filter(tenant__in=[request.tenant, public_tenant]).annotate( + policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True) + ) + # Metadata + meta = { + "count": base_queryset.count(), + } + + # Filtering + # Query parameters + external_tenant_filter = request.query_params.get("external_tenant", None) + username_filter = request.query_params.get("username", None) + system_filter = request.query_params.get("system", None) + application_filter = request.query_params.get("application", None) + + if username_filter: + username_queryset = base_queryset.filter(name=username_filter) + # Metadata + meta = { + "count": username_queryset.count(), + } + return Response({"meta": meta, "data": username_queryset.values()}) + # System filter + elif system_filter in ["true", "false"]: + system_roles_queryset = base_queryset.filter(system__icontains=system_filter) + + # Metadata + meta = { + "count": system_roles_queryset.count(), + } + return Response({"meta": meta, "data": system_roles_queryset.values()}) + # External tenant roles filter + elif external_tenant_filter: + # Get the specific external tenant + ext_tenant = ExtTenant.objects.get(name=external_tenant_filter) + + # Get roles associated with this external tenant + ext_roles = Role.objects.filter(ext_relation__ext_tenant=ext_tenant).annotate( + external_tenant=F("ext_relation__ext_tenant__name") + ) + return Response({"meta": meta, "data": ext_roles.values()}) + + # Application filter + elif application_filter: + applications = application_filter.split(",") + + # Filter roles based on the list of applications + application_roles = base_queryset.filter(access__permission__application__in=applications) + + # Metadata + meta = { + "count": application_roles.count(), + } + + return Response({"meta": meta, "data": application_roles.values()}) + else: + meta = { + "count": base_queryset.count(), + } + return Response({"meta": meta, "data": base_queryset.values()}) def retrieve(self, request, *args, **kwargs): """Get a role. diff --git a/tests/internal/integration/test_integration_views.py b/tests/internal/integration/test_integration_views.py index d0b3b6c0a..fd6748ddc 100644 --- a/tests/internal/integration/test_integration_views.py +++ b/tests/internal/integration/test_integration_views.py @@ -424,10 +424,11 @@ def test_roles_for_org(self, mock_request): self.assertEqual(response.data.get("meta").get("count"), 4) response = self.client.get( - f"/_private/api/v1/integrations/tenant/{self.tenant.org_id}/roles/?username=isolated_principal", + f"/_private/api/v1/integrations/tenant/{self.tenant.org_id}/roles/?username=isolated_role", **self.request.META, follow=True, ) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data.get("meta").get("count"), 1) diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 94964630f..50de04662 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -1732,13 +1732,13 @@ def test_external_tenant_filter(self): response = client.get(URL, **self.headers) self.assertEqual(len(response.data.get("data")), 5) - url = f"{URL}?external_tenant=foo" client = APIClient() response = client.get(url, **self.headers) self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] + print(role) self.assertEqual(role.get("external_tenant"), "foo") def test_list_role_admin_platform_default_groups(self): @@ -2057,6 +2057,7 @@ def test_list_roles_without_User_Access_Admin_system_foo_fail(self): # Org Admin can list the roles response = client.get(url, **self.headers_org_admin) + print(response.data) expected_count = self.system_roles_count + self.non_system_roles_count self.assertEqual(len(response.data.get("data")), expected_count) From 3cd7d46f93b58f60fc9d3437890b7a5adc83b330 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Mon, 2 Dec 2024 16:55:40 +0000 Subject: [PATCH 02/16] Roles filtering refactor --- rbac/management/role/view.py | 89 +++++++++++++++--------------- tests/management/role/test_view.py | 3 + 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 49416a73b..8ae26f44d 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -334,57 +334,54 @@ def list(self, request, *args, **kwargs): } # Filtering - # Query parameters - external_tenant_filter = request.query_params.get("external_tenant", None) - username_filter = request.query_params.get("username", None) - system_filter = request.query_params.get("system", None) - application_filter = request.query_params.get("application", None) - - if username_filter: - username_queryset = base_queryset.filter(name=username_filter) - # Metadata - meta = { - "count": username_queryset.count(), - } - return Response({"meta": meta, "data": username_queryset.values()}) - # System filter - elif system_filter in ["true", "false"]: - system_roles_queryset = base_queryset.filter(system__icontains=system_filter) - - # Metadata - meta = { - "count": system_roles_queryset.count(), - } - return Response({"meta": meta, "data": system_roles_queryset.values()}) - # External tenant roles filter - elif external_tenant_filter: - # Get the specific external tenant - ext_tenant = ExtTenant.objects.get(name=external_tenant_filter) - - # Get roles associated with this external tenant - ext_roles = Role.objects.filter(ext_relation__ext_tenant=ext_tenant).annotate( - external_tenant=F("ext_relation__ext_tenant__name") - ) - return Response({"meta": meta, "data": ext_roles.values()}) - - # Application filter - elif application_filter: - applications = application_filter.split(",") + # Query params dictionary + query_params = { + "external_tenant": request.query_params.get("external_tenant", None), + "username": request.query_params.get("username", None), + "system": request.query_params.get("system", None), + "application": request.query_params.get("application", None), + "display_name": request.query_params.get("display_name", None), + "name_match": request.query_params.get("name_match", None), + } - # Filter roles based on the list of applications - application_roles = base_queryset.filter(access__permission__application__in=applications) + if query_params: + filtered_queryset = base_queryset + system_value = str(query_params["system"]).lower() + if query_params["username"]: + filtered_queryset = filtered_queryset.filter(name=query_params["username"]) + if query_params["external_tenant"]: + ext_tenant = ExtTenant.objects.get(name=query_params["external_tenant"]) + filtered_queryset = filtered_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( + external_tenant=F("ext_relation__ext_tenant__name") + ) + if system_value == "false": + filtered_queryset = filtered_queryset.filter(system=False) + elif system_value == "true": + filtered_queryset = filtered_queryset.filter(system=True) + if query_params["application"]: + applications = query_params["application"].split(",") + external_tenant = ( + ExtTenant.objects.get(name=query_params["application"]) + if applications and ExtTenant.objects.filter(name=query_params["application"]).exists() + else None + ) + filtered_queryset = filtered_queryset.filter(access__permission__application__in=applications) + # If a external tenant exists with the name passed to application query parameter + # return the roles for that external tenant + if external_tenant: + ext_tenant = ExtTenant.objects.get(name=query_params["application"]) + filtered_queryset = base_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( + external_tenant=F("ext_relation__ext_tenant__name") + ) + if query_params["display_name"]: + filtered_queryset = filtered_queryset.filter(display_name=query_params["display_name"]) + # if query_params["name_match"] == # Metadata meta = { - "count": application_roles.count(), - } - - return Response({"meta": meta, "data": application_roles.values()}) - else: - meta = { - "count": base_queryset.count(), + "count": filtered_queryset.count(), } - return Response({"meta": meta, "data": base_queryset.values()}) + return Response({"meta": meta, "data": filtered_queryset.values()}) def retrieve(self, request, *args, **kwargs): """Get a role. diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 50de04662..7fac67dd0 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -663,6 +663,7 @@ def test_read_role_list_success(self): self.assertIsNotNone(iterRole.get("name")) # fields displayed are same as defined self.assertEqual(self.display_fields, set(iterRole.keys())) + if iterRole.get("name") == role_name: self.assertEqual(iterRole.get("accessCount"), 2) role = iterRole @@ -674,6 +675,7 @@ def test_get_role_by_application_single(self): url = "{}?application={}".format(URL, "app") client = APIClient() response = client.get(url, **self.headers) + self.assertEqual(response.data.get("meta").get("count"), 1) self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name) @@ -682,6 +684,7 @@ def test_get_role_by_application_using_ext_tenant(self): url = "{}?application={}".format(URL, "foo") client = APIClient() response = client.get(url, **self.headers) + self.assertEqual(response.data.get("meta").get("count"), 1) self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name) From 6849d7bc9b6f67f2958a88fe351208e3d399c95b Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 3 Dec 2024 17:03:17 +0000 Subject: [PATCH 03/16] Roles endpoint filtering continued --- rbac/management/role/serializer.py | 13 +++++ rbac/management/role/view.py | 81 +++++++++++++++++++++++++++--- tests/management/role/test_view.py | 7 +-- 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index cb4bf515a..065312192 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -100,6 +100,7 @@ class RoleSerializer(serializers.ModelSerializer): modified = serializers.DateTimeField(read_only=True) external_role_id = serializers.SerializerMethodField() external_tenant = serializers.SerializerMethodField() + groups_in_count = serializers.SerializerMethodField() class Meta: """Metadata for the serializer.""" @@ -121,6 +122,7 @@ class Meta: "modified", "external_role_id", "external_tenant", + "groups_in_count", ) def get_applications(self, obj): @@ -161,6 +163,17 @@ def get_external_tenant(self, obj): """Get the external tenant name if it's from an external tenant.""" return obj.external_tenant_name() + def get_groups_in_count(self, obj): + """Get the total count of groups where the role is in.""" + request = self.context.get("request") + groups_in = obtain_groups_in(obj, request) + return groups_in.count() # Make sure this returns an integer count + + def get_groups_in(self, obj): + """Get the groups where the role is in.""" + request = self.context.get("request") + return obtain_groups_in(obj, request).values("name", "uuid", "description") + class RoleMinimumSerializer(SerializerCreateOverrideMixin, serializers.ModelSerializer): """Serializer for the Role model that doesn't return access info.""" diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 8ae26f44d..f3b42312e 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -328,10 +328,6 @@ def list(self, request, *args, **kwargs): base_queryset = Role.objects.filter(tenant__in=[request.tenant, public_tenant]).annotate( policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True) ) - # Metadata - meta = { - "count": base_queryset.count(), - } # Filtering # Query params dictionary @@ -342,22 +338,45 @@ def list(self, request, *args, **kwargs): "application": request.query_params.get("application", None), "display_name": request.query_params.get("display_name", None), "name_match": request.query_params.get("name_match", None), + "permission": request.query_params.get("permission", None), + "name": request.query_params.get("name", None), + "platform_default": request.query_params.get("platform_default", None), + "admin_default": request.query_params.get("admin_default", None), } if query_params: filtered_queryset = base_queryset system_value = str(query_params["system"]).lower() - if query_params["username"]: + + if query_params["username"] and request.user.admin is True: filtered_queryset = filtered_queryset.filter(name=query_params["username"]) + print(filtered_queryset) + else: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "errors": [ + { + "detail": "Invalid query parameter username", + "source": "username filtering not allowed", + "status": status.HTTP_400_BAD_REQUEST, + } + ] + }, + ) + + # External tenant filter if query_params["external_tenant"]: ext_tenant = ExtTenant.objects.get(name=query_params["external_tenant"]) filtered_queryset = filtered_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( external_tenant=F("ext_relation__ext_tenant__name") ) + # System value filter if system_value == "false": filtered_queryset = filtered_queryset.filter(system=False) elif system_value == "true": filtered_queryset = filtered_queryset.filter(system=True) + # Application filter if query_params["application"]: applications = query_params["application"].split(",") external_tenant = ( @@ -373,15 +392,61 @@ def list(self, request, *args, **kwargs): filtered_queryset = base_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( external_tenant=F("ext_relation__ext_tenant__name") ) + # Display_name & name_match filter if query_params["display_name"]: - filtered_queryset = filtered_queryset.filter(display_name=query_params["display_name"]) - # if query_params["name_match"] == + if query_params["name_match"] == "partial": + filtered_queryset = filtered_queryset.filter(display_name__contains=query_params["display_name"]) + elif query_params["name_match"] == "exact": + filtered_queryset = filtered_queryset.filter(display_name__exact=query_params["display_name"]) + elif not query_params["name_match"]: + filtered_queryset = filtered_queryset.filter(display_name__contains=query_params["display_name"]) + else: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "errors": [ + { + "detail": "Invalid name match value provided", + "source": "name_match query parameter", + "status": status.HTTP_400_BAD_REQUEST, + } + ] + }, + ) + # name & name_match filter + if query_params["name"]: + if query_params["name_match"] == "partial": + filtered_queryset = filtered_queryset.filter(name__contains=query_params["name"]) + elif query_params["name_match"] == "exact": + filtered_queryset = filtered_queryset.filter(name__exact=query_params["name"]) + elif not query_params["name_match"]: + filtered_queryset = filtered_queryset.filter(name__contains=query_params["name"]) + else: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "errors": [ + { + "detail": "Invalid name match value provided", + "source": "name_match query parameter", + "status": status.HTTP_400_BAD_REQUEST, + } + ] + }, + ) + # Permission filter + if query_params["permission"]: + permissions = query_params["permission"].split(",") + filtered_queryset = filtered_queryset.filter(access__permission__permission__in=permissions) + + # Serialize the queryset for response + serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request}) # Metadata meta = { "count": filtered_queryset.count(), } - return Response({"meta": meta, "data": filtered_queryset.values()}) + return Response({"meta": meta, "data": serializer.data}) def retrieve(self, request, *args, **kwargs): """Get a role. diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 7fac67dd0..b188da303 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -1156,7 +1156,8 @@ def test_list_role_fail_with_invalid_username(self, mock_request): url = "{}?username={}".format(URL, "foo") client = APIClient() response = client.get(url, **self.headers) - + print(response.data) + print(response.headers) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @patch("management.principal.proxy.PrincipalProxy.request_filtered_principals") @@ -1741,7 +1742,6 @@ def test_external_tenant_filter(self): self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] - print(role) self.assertEqual(role.get("external_tenant"), "foo") def test_list_role_admin_platform_default_groups(self): @@ -1757,6 +1757,7 @@ def test_list_role_admin_platform_default_groups(self): self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] + print(response.data) self.assertEqual(role.get("groups_in_count"), 2) def test_create_duplicate_role_fail(self): @@ -1957,6 +1958,7 @@ def test_list_roles_with_User_Access_Admin_success(self): response = client.get(url, **self.headers_user_based_principal) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_count = self.system_roles_count + self.non_system_roles_count + 1 + self.assertEqual(len(response.data.get("data")), expected_count) response = client.get(url, **self.headers_service_account_principal) @@ -2060,7 +2062,6 @@ def test_list_roles_without_User_Access_Admin_system_foo_fail(self): # Org Admin can list the roles response = client.get(url, **self.headers_org_admin) - print(response.data) expected_count = self.system_roles_count + self.non_system_roles_count self.assertEqual(len(response.data.get("data")), expected_count) From cd1dbf0ce6b1856c83de914eee7ece94829d9870 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Wed, 4 Dec 2024 16:43:38 +0000 Subject: [PATCH 04/16] Roles endpoint filtering - username query parameter handling --- rbac/management/role/serializer.py | 2 ++ rbac/management/role/view.py | 53 +++++++++++++++++++----------- tests/management/role/test_view.py | 13 +++++--- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index 065312192..264526a81 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -101,6 +101,7 @@ class RoleSerializer(serializers.ModelSerializer): external_role_id = serializers.SerializerMethodField() external_tenant = serializers.SerializerMethodField() groups_in_count = serializers.SerializerMethodField() + groups_in = serializers.SerializerMethodField() class Meta: """Metadata for the serializer.""" @@ -123,6 +124,7 @@ class Meta: "external_role_id", "external_tenant", "groups_in_count", + "groups_in", ) def get_applications(self, obj): diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index f3b42312e..7453f5ee8 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -34,6 +34,7 @@ from management.models import AuditLog, Permission from management.notifications.notification_handlers import role_obj_change_notification_handler from management.permissions import RoleAccessPermission +from management.principal.proxy import PrincipalProxy from management.querysets import get_role_queryset, user_has_perm from management.relation_replicator.relation_replicator import DualWriteException, ReplicationEventType from management.role.relation_api_dual_write_handler import ( @@ -46,7 +47,7 @@ from rest_framework.filters import OrderingFilter from rest_framework.response import Response -from api.models import Tenant +from api.models import Tenant, User from rbac.env import ENVIRONMENT from .model import ExtTenant, Role from .serializer import RoleSerializer @@ -333,7 +334,6 @@ def list(self, request, *args, **kwargs): # Query params dictionary query_params = { "external_tenant": request.query_params.get("external_tenant", None), - "username": request.query_params.get("username", None), "system": request.query_params.get("system", None), "application": request.query_params.get("application", None), "display_name": request.query_params.get("display_name", None), @@ -342,28 +342,40 @@ def list(self, request, *args, **kwargs): "name": request.query_params.get("name", None), "platform_default": request.query_params.get("platform_default", None), "admin_default": request.query_params.get("admin_default", None), + "username": request.query_params.get("username", None), + "limit": request.query_params.get("limit", None), + "offset": request.query_params.get("offset", None), } if query_params: filtered_queryset = base_queryset system_value = str(query_params["system"]).lower() - - if query_params["username"] and request.user.admin is True: - filtered_queryset = filtered_queryset.filter(name=query_params["username"]) - print(filtered_queryset) - else: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "errors": [ - { - "detail": "Invalid query parameter username", - "source": "username filtering not allowed", - "status": status.HTTP_400_BAD_REQUEST, - } - ] - }, + # Use principal proxy here + if query_params["username"]: + proxy = PrincipalProxy + results = proxy.request_filtered_principals( + query_params["username"], + org_id=request.user.org_id, ) + # Accessing the first item in the 'data' list + principal = results["data"][0] + + # Convert from principal to User Model to add to request object + org_id = principal.get("org_id") + user_id = principal.get("user_id") + username = principal.get("username") + account_number = principal.get("account_number") + is_active = principal.get("is_active") + + # Map principal fields to User fields + user = User() + user.org_id = org_id + user.user_id = user_id + user.username = username + user.account = account_number + user.is_active = is_active + print(user) + request.user_from_query = user # External tenant filter if query_params["external_tenant"]: @@ -446,7 +458,10 @@ def list(self, request, *args, **kwargs): meta = { "count": filtered_queryset.count(), } - return Response({"meta": meta, "data": serializer.data}) + + # Pagination + links = {} + return Response({"meta": meta, "links": links, "data": serializer.data}) def retrieve(self, request, *args, **kwargs): """Get a role. diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index b188da303..8f1ff7382 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -158,6 +158,9 @@ def setUp(self): "admin_default", "external_role_id", "external_tenant", + "groups_in_count", + "groups_in", + "access", } self.principal = Principal(username=self.user_data["username"], tenant=self.tenant) @@ -911,9 +914,10 @@ def test_list_role_with_groups_in_fields_with_username_param_for_non_org_admin(s # make sure all roles are from: # * custom group 'NewGroupForJohn' or # * 'Default access' group - groups = [default_access_group_name, custom_group_name] + groups = [default_access_group_name, custom_group_name, default_admin_access_group_name] for role in response_data: for group in role[groups_in]: + print(group) self.assertIn(group["name"], groups) @patch("management.principal.proxy.PrincipalProxy.request_filtered_principals") @@ -1156,8 +1160,7 @@ def test_list_role_fail_with_invalid_username(self, mock_request): url = "{}?username={}".format(URL, "foo") client = APIClient() response = client.get(url, **self.headers) - print(response.data) - print(response.headers) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @patch("management.principal.proxy.PrincipalProxy.request_filtered_principals") @@ -1191,6 +1194,8 @@ def test_list_role_with_additional_fields_username_success(self, mock_request): self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] + print(role.keys()) + print(new_display_fields) self.assertEqual(new_display_fields, set(role.keys())) self.assertEqual(role["groups_in_count"], 1) @@ -1209,6 +1214,7 @@ def test_list_role_with_additional_fields_principal_success(self): self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] + self.assertEqual(new_display_fields, set(role.keys())) self.assertEqual(role["groups_in_count"], 1) @@ -1757,7 +1763,6 @@ def test_list_role_admin_platform_default_groups(self): self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] - print(response.data) self.assertEqual(role.get("groups_in_count"), 2) def test_create_duplicate_role_fail(self): From 2c1caacd954ca4eb3283601fbb6821a73a23cab5 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Thu, 5 Dec 2024 14:38:57 +0000 Subject: [PATCH 05/16] Roles endpoint - filtering addition complete --- rbac/management/role/view.py | 99 ++++++++++++++++++------------ tests/management/role/test_view.py | 6 +- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 7453f5ee8..7e3e750b6 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -331,7 +331,7 @@ def list(self, request, *args, **kwargs): ) # Filtering - # Query params dictionary + query_params = { "external_tenant": request.query_params.get("external_tenant", None), "system": request.query_params.get("system", None), @@ -345,57 +345,77 @@ def list(self, request, *args, **kwargs): "username": request.query_params.get("username", None), "limit": request.query_params.get("limit", None), "offset": request.query_params.get("offset", None), + "add_fields": request.query_params.get("add_fields", None), } + filters = [] + if query_params: filtered_queryset = base_queryset system_value = str(query_params["system"]).lower() - # Use principal proxy here + + # Check if 'add_fields' is a valid field + additional_fields = ["access", "groups_in", "groups_in_count"] + if query_params["add_fields"]: + split_fields = query_params["add_fields"].split(",") + invalid_field = [field for field in split_fields if field not in additional_fields] + if invalid_field: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "errors": [ + { + "detail": "Invalid additional field passed in query", + "source": "add_fields invalid value", + "status": status.HTTP_400_BAD_REQUEST, + } + ] + }, + ) + + # Username filter if query_params["username"]: proxy = PrincipalProxy results = proxy.request_filtered_principals( query_params["username"], org_id=request.user.org_id, ) - # Accessing the first item in the 'data' list - principal = results["data"][0] - - # Convert from principal to User Model to add to request object - org_id = principal.get("org_id") - user_id = principal.get("user_id") - username = principal.get("username") - account_number = principal.get("account_number") - is_active = principal.get("is_active") - - # Map principal fields to User fields - user = User() - user.org_id = org_id - user.user_id = user_id - user.username = username - user.account = account_number - user.is_active = is_active - print(user) - request.user_from_query = user + results_exist = results["data"] + if results_exist: + principal = results["data"][0] + + # Convert from principal to User Model to add to request object + org_id = principal.get("org_id") + user_id = principal.get("user_id") + username = principal.get("username") + account_number = principal.get("account_number") + is_active = principal.get("is_active") + + # Map principal fields to User fields + user = User() + user.org_id = org_id + user.user_id = user_id + user.username = username + user.account = account_number + user.is_active = is_active + request.user_from_query = user # External tenant filter if query_params["external_tenant"]: ext_tenant = ExtTenant.objects.get(name=query_params["external_tenant"]) - filtered_queryset = filtered_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( - external_tenant=F("ext_relation__ext_tenant__name") - ) + if ext_tenant: + filters.append(Q(ext_relation__ext_tenant=ext_tenant)) # System value filter if system_value == "false": - filtered_queryset = filtered_queryset.filter(system=False) + filters.append(Q(system=False)) elif system_value == "true": - filtered_queryset = filtered_queryset.filter(system=True) + filters.append(Q(system=True)) # Application filter if query_params["application"]: applications = query_params["application"].split(",") - external_tenant = ( - ExtTenant.objects.get(name=query_params["application"]) - if applications and ExtTenant.objects.filter(name=query_params["application"]).exists() - else None - ) + + external_tenant = ExtTenant.objects.filter(name=query_params["application"]).first() + filtered_queryset = filtered_queryset.filter(access__permission__application__in=applications) # If a external tenant exists with the name passed to application query parameter # return the roles for that external tenant @@ -407,11 +427,11 @@ def list(self, request, *args, **kwargs): # Display_name & name_match filter if query_params["display_name"]: if query_params["name_match"] == "partial": - filtered_queryset = filtered_queryset.filter(display_name__contains=query_params["display_name"]) + filters.append(Q(display_name__contains=query_params["display_name"])) elif query_params["name_match"] == "exact": - filtered_queryset = filtered_queryset.filter(display_name__exact=query_params["display_name"]) + filters.append(Q(display_name__exact=query_params["display_name"])) elif not query_params["name_match"]: - filtered_queryset = filtered_queryset.filter(display_name__contains=query_params["display_name"]) + filters.append(Q(display_name__contains=query_params["display_name"])) else: return Response( status=status.HTTP_400_BAD_REQUEST, @@ -428,11 +448,11 @@ def list(self, request, *args, **kwargs): # name & name_match filter if query_params["name"]: if query_params["name_match"] == "partial": - filtered_queryset = filtered_queryset.filter(name__contains=query_params["name"]) + filters.append(Q(name__contains=query_params["name"])) elif query_params["name_match"] == "exact": - filtered_queryset = filtered_queryset.filter(name__exact=query_params["name"]) + filters.append(Q(name__exact=query_params["name"])) elif not query_params["name_match"]: - filtered_queryset = filtered_queryset.filter(name__contains=query_params["name"]) + filters.append(Q(name__contains=query_params["name"])) else: return Response( status=status.HTTP_400_BAD_REQUEST, @@ -449,7 +469,10 @@ def list(self, request, *args, **kwargs): # Permission filter if query_params["permission"]: permissions = query_params["permission"].split(",") - filtered_queryset = filtered_queryset.filter(access__permission__permission__in=permissions) + filters.append(Q(access__permission__permission__in=permissions)) + + # Apply the filters + filtered_queryset = filtered_queryset.filter(*filters) # Serialize the queryset for response serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request}) diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 8f1ff7382..bbe395e5c 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -917,7 +917,6 @@ def test_list_role_with_groups_in_fields_with_username_param_for_non_org_admin(s groups = [default_access_group_name, custom_group_name, default_admin_access_group_name] for role in response_data: for group in role[groups_in]: - print(group) self.assertIn(group["name"], groups) @patch("management.principal.proxy.PrincipalProxy.request_filtered_principals") @@ -1190,12 +1189,9 @@ def test_list_role_with_additional_fields_username_success(self, mock_request): url = "{}?add_fields={},{}&username={}".format(URL, field_1, field_2, self.principal.username) client = APIClient() response = client.get(url, **self.headers) - self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] - print(role.keys()) - print(new_display_fields) self.assertEqual(new_display_fields, set(role.keys())) self.assertEqual(role["groups_in_count"], 1) @@ -1760,7 +1756,7 @@ def test_list_role_admin_platform_default_groups(self): url = f"{URL}?display_name=platform_admin_default_display&add_fields=groups_in_count%2Cgroups_in" client = APIClient() response = client.get(url, **self.headers) - + print(response.data) self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] self.assertEqual(role.get("groups_in_count"), 2) From 82f2c763c9a63ebab1fe35fafa4cfe28a4ddf1a9 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Thu, 5 Dec 2024 14:40:28 +0000 Subject: [PATCH 06/16] remove print statement from roles test --- tests/management/role/test_view.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index bbe395e5c..a4469fa0d 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -1756,7 +1756,6 @@ def test_list_role_admin_platform_default_groups(self): url = f"{URL}?display_name=platform_admin_default_display&add_fields=groups_in_count%2Cgroups_in" client = APIClient() response = client.get(url, **self.headers) - print(response.data) self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] self.assertEqual(role.get("groups_in_count"), 2) From a2215128f802962f23e62db3d70e69935548905e Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Fri, 6 Dec 2024 13:42:54 +0000 Subject: [PATCH 07/16] Roles endpoint filtering performance done --- rbac/management/role/view.py | 30 +++++++++++++++---- .../integration/test_integration_views.py | 2 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 7e3e750b6..85c8a336a 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -34,6 +34,7 @@ from management.models import AuditLog, Permission from management.notifications.notification_handlers import role_obj_change_notification_handler from management.permissions import RoleAccessPermission +from management.principal.model import Principal from management.principal.proxy import PrincipalProxy from management.querysets import get_role_queryset, user_has_perm from management.relation_replicator.relation_replicator import DualWriteException, ReplicationEventType @@ -331,7 +332,6 @@ def list(self, request, *args, **kwargs): ) # Filtering - query_params = { "external_tenant": request.query_params.get("external_tenant", None), "system": request.query_params.get("system", None), @@ -354,7 +354,7 @@ def list(self, request, *args, **kwargs): filtered_queryset = base_queryset system_value = str(query_params["system"]).lower() - # Check if 'add_fields' is a valid field + # Check if "add_fields" is a valid field additional_fields = ["access", "groups_in", "groups_in_count"] if query_params["add_fields"]: split_fields = query_params["add_fields"].split(",") @@ -375,6 +375,22 @@ def list(self, request, *args, **kwargs): # Username filter if query_params["username"]: + try: + princ = Principal.objects.get(username=query_params["username"]) + except Principal.DoesNotExist: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "errors": [ + { + "detail": "Principal not found for this username", + "source": "Invalid username query parameter", + "status": status.HTTP_400_BAD_REQUEST, + } + ] + }, + ) + proxy = PrincipalProxy results = proxy.request_filtered_principals( query_params["username"], @@ -477,13 +493,15 @@ def list(self, request, *args, **kwargs): # Serialize the queryset for response serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request}) - # Metadata - meta = { - "count": filtered_queryset.count(), - } + meta = {} + if query_params.get("username"): + meta["count"] = sum(len(group.roles()) for group in princ.group.all()) + else: + meta["count"] = filtered_queryset.count() # Pagination links = {} + return Response({"meta": meta, "links": links, "data": serializer.data}) def retrieve(self, request, *args, **kwargs): diff --git a/tests/internal/integration/test_integration_views.py b/tests/internal/integration/test_integration_views.py index fd6748ddc..7dd5cea1e 100644 --- a/tests/internal/integration/test_integration_views.py +++ b/tests/internal/integration/test_integration_views.py @@ -424,7 +424,7 @@ def test_roles_for_org(self, mock_request): self.assertEqual(response.data.get("meta").get("count"), 4) response = self.client.get( - f"/_private/api/v1/integrations/tenant/{self.tenant.org_id}/roles/?username=isolated_role", + f"/_private/api/v1/integrations/tenant/{self.tenant.org_id}/roles/?username=isolated_principal", **self.request.META, follow=True, ) From a49dc74f84ac22ec9c80641b1033a90d065ee2b4 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Mon, 9 Dec 2024 12:29:02 +0000 Subject: [PATCH 08/16] Pagination readded to roles endpoint --- rbac/management/role/view.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 85c8a336a..fcc5b5c8a 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -48,6 +48,7 @@ from rest_framework.filters import OrderingFilter from rest_framework.response import Response +from api.common.pagination import StandardResultsSetPagination from api.models import Tenant, User from rbac.env import ENVIRONMENT from .model import ExtTenant, Role @@ -143,6 +144,7 @@ class RoleViewSet( filterset_class = RoleFilter ordering_fields = ("name", "display_name", "modified", "policyCount") ordering = ("name",) + default_limit = StandardResultsSetPagination.default_limit def get_queryset(self): """Obtain queryset for requesting user based on access and action.""" @@ -327,8 +329,10 @@ def list(self, request, *args, **kwargs): """ public_tenant = Tenant.objects.get(tenant_name="public") - base_queryset = Role.objects.filter(tenant__in=[request.tenant, public_tenant]).annotate( - policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True) + base_queryset = ( + Role.objects.only("name", "uuid") + .filter(tenant__in=[request.tenant, public_tenant]) + .annotate(policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True)) ) # Filtering @@ -343,12 +347,19 @@ def list(self, request, *args, **kwargs): "platform_default": request.query_params.get("platform_default", None), "admin_default": request.query_params.get("admin_default", None), "username": request.query_params.get("username", None), - "limit": request.query_params.get("limit", None), - "offset": request.query_params.get("offset", None), + "limit": request.query_params.get("limit", 10), + "offset": request.query_params.get("offset", 0), "add_fields": request.query_params.get("add_fields", None), } filters = [] + limit = query_params["limit"] + offset = int(query_params["offset"]) + path = request.path + + previous_offset = 0 + if offset - limit > 0: + previous_offset = offset - limit if query_params: filtered_queryset = base_queryset @@ -366,7 +377,7 @@ def list(self, request, *args, **kwargs): "errors": [ { "detail": "Invalid additional field passed in query", - "source": "add_fields invalid value", + "source": "add_fields invalid parameter", "status": status.HTTP_400_BAD_REQUEST, } ] @@ -493,14 +504,22 @@ def list(self, request, *args, **kwargs): # Serialize the queryset for response serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request}) + # Metadata meta = {} if query_params.get("username"): meta["count"] = sum(len(group.roles()) for group in princ.group.all()) else: meta["count"] = filtered_queryset.count() + count = filtered_queryset.count() + # Pagination - links = {} + links = { + "first": f"{path}?limit={limit}&offset=0", + "next": f"{path}?limit={limit}&offset={offset + limit}", + "previous": f"{path}?limit={limit}&offset={previous_offset}", + "last": f"{path}?limit={limit}&offset={count - limit if (count - limit) >= 0 else 0}", + } return Response({"meta": meta, "links": links, "data": serializer.data}) From 0c720490fd8751607220c3c8598048920731eca3 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 10 Dec 2024 10:39:00 +0000 Subject: [PATCH 09/16] Added prefetch_related to base_queryset for access and ext_relations --- rbac/management/role/view.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index fcc5b5c8a..7fca29d11 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -331,6 +331,7 @@ def list(self, request, *args, **kwargs): public_tenant = Tenant.objects.get(tenant_name="public") base_queryset = ( Role.objects.only("name", "uuid") + .prefetch_related("access", "ext_relation") .filter(tenant__in=[request.tenant, public_tenant]) .annotate(policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True)) ) From 023ff1bd7e35ee04961b83b71beb0868b29a10a4 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 10 Dec 2024 14:41:11 +0000 Subject: [PATCH 10/16] lint fix --- rbac/management/role/view.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 7fca29d11..7fc136c37 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -330,8 +330,7 @@ def list(self, request, *args, **kwargs): """ public_tenant = Tenant.objects.get(tenant_name="public") base_queryset = ( - Role.objects.only("name", "uuid") - .prefetch_related("access", "ext_relation") + Role.objects.prefetch_related("access", "ext_relation") .filter(tenant__in=[request.tenant, public_tenant]) .annotate(policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True)) ) @@ -367,7 +366,7 @@ def list(self, request, *args, **kwargs): system_value = str(query_params["system"]).lower() # Check if "add_fields" is a valid field - additional_fields = ["access", "groups_in", "groups_in_count"] + additional_fields = {"access", "groups_in", "groups_in_count"} if query_params["add_fields"]: split_fields = query_params["add_fields"].split(",") invalid_field = [field for field in split_fields if field not in additional_fields] @@ -387,9 +386,8 @@ def list(self, request, *args, **kwargs): # Username filter if query_params["username"]: - try: - princ = Principal.objects.get(username=query_params["username"]) - except Principal.DoesNotExist: + princ = Principal.objects.filter(username=query_params["username"]).first() + if not princ: return Response( status=status.HTTP_400_BAD_REQUEST, data={ @@ -512,14 +510,12 @@ def list(self, request, *args, **kwargs): else: meta["count"] = filtered_queryset.count() - count = filtered_queryset.count() - # Pagination links = { "first": f"{path}?limit={limit}&offset=0", "next": f"{path}?limit={limit}&offset={offset + limit}", "previous": f"{path}?limit={limit}&offset={previous_offset}", - "last": f"{path}?limit={limit}&offset={count - limit if (count - limit) >= 0 else 0}", + "last": f"{path}?limit={limit}&offset={meta['count'] - limit if (meta['count'] - limit) >= 0 else 0}", } return Response({"meta": meta, "links": links, "data": serializer.data}) From 68161599534e7c46e9b34209e44d91c1a761a60d Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Wed, 11 Dec 2024 16:54:01 +0000 Subject: [PATCH 11/16] Removed group_in and group_in_counts from role serializer, will add dynamically when passed in add_fields query param --- rbac/management/role/serializer.py | 19 ++++--------------- rbac/management/role/view.py | 27 ++++++++++++++++++++------- tests/management/role/test_view.py | 9 ++++----- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index 264526a81..befdab14b 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -92,6 +92,8 @@ class RoleSerializer(serializers.ModelSerializer): access = AccessSerializer(many=True) policyCount = serializers.IntegerField(read_only=True) accessCount = serializers.IntegerField(read_only=True) + groups_in_count = serializers.IntegerField(read_only=True) + groups_in = serializers.ListField(read_only=True) applications = serializers.SerializerMethodField() system = serializers.BooleanField(read_only=True) platform_default = serializers.BooleanField(read_only=True) @@ -100,8 +102,6 @@ class RoleSerializer(serializers.ModelSerializer): modified = serializers.DateTimeField(read_only=True) external_role_id = serializers.SerializerMethodField() external_tenant = serializers.SerializerMethodField() - groups_in_count = serializers.SerializerMethodField() - groups_in = serializers.SerializerMethodField() class Meta: """Metadata for the serializer.""" @@ -115,6 +115,8 @@ class Meta: "access", "policyCount", "accessCount", + "groups_in_count", + "groups_in", "applications", "system", "platform_default", @@ -123,8 +125,6 @@ class Meta: "modified", "external_role_id", "external_tenant", - "groups_in_count", - "groups_in", ) def get_applications(self, obj): @@ -165,17 +165,6 @@ def get_external_tenant(self, obj): """Get the external tenant name if it's from an external tenant.""" return obj.external_tenant_name() - def get_groups_in_count(self, obj): - """Get the total count of groups where the role is in.""" - request = self.context.get("request") - groups_in = obtain_groups_in(obj, request) - return groups_in.count() # Make sure this returns an integer count - - def get_groups_in(self, obj): - """Get the groups where the role is in.""" - request = self.context.get("request") - return obtain_groups_in(obj, request).values("name", "uuid", "description") - class RoleMinimumSerializer(SerializerCreateOverrideMixin, serializers.ModelSerializer): """Serializer for the Role model that doesn't return access info.""" diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 7fc136c37..ad2b18323 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -329,12 +329,6 @@ def list(self, request, *args, **kwargs): """ public_tenant = Tenant.objects.get(tenant_name="public") - base_queryset = ( - Role.objects.prefetch_related("access", "ext_relation") - .filter(tenant__in=[request.tenant, public_tenant]) - .annotate(policyCount=Count("policies", distinct=True), accessCount=Count("access", distinct=True)) - ) - # Filtering query_params = { "external_tenant": request.query_params.get("external_tenant", None), @@ -349,8 +343,27 @@ def list(self, request, *args, **kwargs): "username": request.query_params.get("username", None), "limit": request.query_params.get("limit", 10), "offset": request.query_params.get("offset", 0), - "add_fields": request.query_params.get("add_fields", None), + "add_fields": request.query_params.get("add_fields", ""), } + add_fields = query_params["add_fields"] + add_fields_split = add_fields.split(",") + + base_queryset = ( + Role.objects.prefetch_related("access", "ext_relation") + .filter(tenant__in=[request.tenant, public_tenant]) + .annotate( + policyCount=Count("policies", distinct=True), + accessCount=Count("access", distinct=True), + ) + ) + + # Dynamic annotations + if "groups_in_count" in add_fields_split: + base_queryset = base_queryset.annotate(groups_in_count=Count("policies__group")) + elif "groups_in" in add_fields_split: + base_queryset = base_queryset.annotate(groups_in=F("policies__group__name")) + else: + base_queryset = base_queryset filters = [] limit = query_params["limit"] diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index a4469fa0d..b11cd7503 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -151,6 +151,7 @@ def setUp(self): "display_name", "system", "created", + "access", "policyCount", "accessCount", "modified", @@ -158,9 +159,6 @@ def setUp(self): "admin_default", "external_role_id", "external_tenant", - "groups_in_count", - "groups_in", - "access", } self.principal = Principal(username=self.user_data["username"], tenant=self.tenant) @@ -914,7 +912,7 @@ def test_list_role_with_groups_in_fields_with_username_param_for_non_org_admin(s # make sure all roles are from: # * custom group 'NewGroupForJohn' or # * 'Default access' group - groups = [default_access_group_name, custom_group_name, default_admin_access_group_name] + groups = [default_access_group_name, custom_group_name] for role in response_data: for group in role[groups_in]: self.assertIn(group["name"], groups) @@ -1118,6 +1116,7 @@ def test_list_role_with_groups_in_fields_for_admin_scope_success(self): response_data = response.data.get("data") for iterRole in response_data: + print(iterRole.get("groups_in")) # fields displayed are same as defined, groupsInCount is added self.assertEqual(new_display_fields, set(iterRole.keys())) self.assertIsNotNone(iterRole.get("groups_in")[0]["name"]) @@ -1210,7 +1209,7 @@ def test_list_role_with_additional_fields_principal_success(self): self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] - + print(role) self.assertEqual(new_display_fields, set(role.keys())) self.assertEqual(role["groups_in_count"], 1) From 3bc8ed46eab8a24fc78fb25484183c7570052f50 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Thu, 19 Dec 2024 15:09:21 +0000 Subject: [PATCH 12/16] added dynamic annotation and changed role serializer to groups_in field --- rbac/management/role/serializer.py | 7 ++++++- rbac/management/role/view.py | 12 ++++-------- tests/management/role/test_view.py | 13 ++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index befdab14b..84b257bd4 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -93,7 +93,7 @@ class RoleSerializer(serializers.ModelSerializer): policyCount = serializers.IntegerField(read_only=True) accessCount = serializers.IntegerField(read_only=True) groups_in_count = serializers.IntegerField(read_only=True) - groups_in = serializers.ListField(read_only=True) + groups_in = serializers.SerializerMethodField() applications = serializers.SerializerMethodField() system = serializers.BooleanField(read_only=True) platform_default = serializers.BooleanField(read_only=True) @@ -131,6 +131,11 @@ def get_applications(self, obj): """Get the list of applications in the role.""" return obtain_applications(obj) + def get_groups_in(self, obj): + """Get the groups where the role is in.""" + request = self.context.get("request") + return obtain_groups_in(obj, request).values("name", "uuid", "description") + def create(self, validated_data): """Create the role object in the database.""" name = validated_data.pop("name") diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index ad2b18323..93fd544b3 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -346,7 +346,6 @@ def list(self, request, *args, **kwargs): "add_fields": request.query_params.get("add_fields", ""), } add_fields = query_params["add_fields"] - add_fields_split = add_fields.split(",") base_queryset = ( Role.objects.prefetch_related("access", "ext_relation") @@ -357,13 +356,10 @@ def list(self, request, *args, **kwargs): ) ) - # Dynamic annotations - if "groups_in_count" in add_fields_split: - base_queryset = base_queryset.annotate(groups_in_count=Count("policies__group")) - elif "groups_in" in add_fields_split: - base_queryset = base_queryset.annotate(groups_in=F("policies__group__name")) - else: - base_queryset = base_queryset + # Dynamic annotation + if add_fields: + if "groups_in_count" in add_fields: + base_queryset = base_queryset.annotate(groups_in_count=Count("policies__group")) filters = [] limit = query_params["limit"] diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index b11cd7503..0d9de3319 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -154,6 +154,7 @@ def setUp(self): "access", "policyCount", "accessCount", + "groups_in", "modified", "platform_default", "admin_default", @@ -906,7 +907,7 @@ def test_list_role_with_groups_in_fields_with_username_param_for_non_org_admin(s # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == custom_role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role[groups_in_count], 1) + self.assertEqual(created_role[groups_in_count], 2) self.assertEqual(created_role[groups_in][0]["name"], custom_group_name) # make sure all roles are from: @@ -1002,7 +1003,7 @@ def test_list_role_with_groups_in_fields_with_username_param_for_org_admin(self, # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == custom_role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role[groups_in_count], 1) + self.assertEqual(created_role[groups_in_count], 2) self.assertEqual(created_role[groups_in][0]["name"], custom_group_name) # make sure all roles are from: @@ -1084,13 +1085,13 @@ def test_list_role_with_groups_in_fields_for_principal_scope_success(self, mock_ # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role["groups_in_count"], 1) + self.assertEqual(created_role["groups_in_count"], 2) self.assertEqual(created_role["groups_in"][0]["name"], group_name) # make sure a default role exists in result set and has correct values default_role = next((iterRole for iterRole in response_data if iterRole["name"] == self.defRole.name), None) self.assertIsNotNone(default_role) - self.assertEqual(default_role["groups_in_count"], 1) + self.assertEqual(default_role["groups_in_count"], 2) self.assertEqual(default_role["groups_in"][0]["name"], self.group.name) def test_list_role_with_groups_in_fields_for_admin_scope_success(self): @@ -1116,7 +1117,6 @@ def test_list_role_with_groups_in_fields_for_admin_scope_success(self): response_data = response.data.get("data") for iterRole in response_data: - print(iterRole.get("groups_in")) # fields displayed are same as defined, groupsInCount is added self.assertEqual(new_display_fields, set(iterRole.keys())) self.assertIsNotNone(iterRole.get("groups_in")[0]["name"]) @@ -1126,7 +1126,7 @@ def test_list_role_with_groups_in_fields_for_admin_scope_success(self): # make sure a default role exists in result set and has correct values default_role = next((iterRole for iterRole in response_data if iterRole["name"] == self.defRole.name), None) self.assertIsNotNone(default_role) - self.assertEqual(default_role["groups_in_count"], 1) + self.assertEqual(default_role["groups_in_count"], 2) self.assertEqual(default_role["groups_in"][0]["name"], self.group.name) # make sure an admin role exists in result set and has correct values @@ -1209,7 +1209,6 @@ def test_list_role_with_additional_fields_principal_success(self): self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] - print(role) self.assertEqual(new_display_fields, set(role.keys())) self.assertEqual(role["groups_in_count"], 1) From 4e6b58fbd5fb5461adfa5c0761c4342e27db79c2 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Thu, 9 Jan 2025 12:14:01 +0000 Subject: [PATCH 13/16] Changed to use get_role_queryset from querysets to handle scope/admin requirements --- rbac/management/querysets.py | 2 +- rbac/management/role/view.py | 12 ++---------- tests/management/role/test_view.py | 1 - 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/rbac/management/querysets.py b/rbac/management/querysets.py index f970e7089..769107cd8 100644 --- a/rbac/management/querysets.py +++ b/rbac/management/querysets.py @@ -152,7 +152,7 @@ def get_role_queryset(request) -> QuerySet: """Obtain the queryset for roles.""" scope = validate_and_get_key(request.query_params, SCOPE_KEY, VALID_SCOPES, ORG_ID_SCOPE) public_tenant = Tenant.objects.get(tenant_name="public") - base_query = annotate_roles_with_counts(Role.objects.prefetch_related("access")).filter( + base_query = annotate_roles_with_counts(Role.objects.prefetch_related("access", "ext_relation")).filter( tenant__in=[request.tenant, public_tenant] ) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 93fd544b3..54a4a85b9 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -328,7 +328,6 @@ def list(self, request, *args, **kwargs): } """ - public_tenant = Tenant.objects.get(tenant_name="public") # Filtering query_params = { "external_tenant": request.query_params.get("external_tenant", None), @@ -346,15 +345,8 @@ def list(self, request, *args, **kwargs): "add_fields": request.query_params.get("add_fields", ""), } add_fields = query_params["add_fields"] - - base_queryset = ( - Role.objects.prefetch_related("access", "ext_relation") - .filter(tenant__in=[request.tenant, public_tenant]) - .annotate( - policyCount=Count("policies", distinct=True), - accessCount=Count("access", distinct=True), - ) - ) + roles = get_role_queryset(self.request) + base_queryset = roles # Dynamic annotation if add_fields: diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 0d9de3319..dc54fc590 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -1205,7 +1205,6 @@ def test_list_role_with_additional_fields_principal_success(self): url = "{}?add_fields={},{}&scope=principal".format(URL, field_1, field_2) client = APIClient() response = client.get(url, **self.headers) - self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] From 7cce7af65a8f3a49f3ed2df541d34aa87e8a9a14 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 14 Jan 2025 09:45:31 +0000 Subject: [PATCH 14/16] role query reduction from 33 to 24 queries --- rbac/management/role/serializer.py | 9 -- rbac/management/role/view.py | 201 +---------------------------- rbac/rbac/settings.py | 1 - tests/management/role/test_view.py | 20 ++- 4 files changed, 13 insertions(+), 218 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index 84b257bd4..cb4bf515a 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -92,8 +92,6 @@ class RoleSerializer(serializers.ModelSerializer): access = AccessSerializer(many=True) policyCount = serializers.IntegerField(read_only=True) accessCount = serializers.IntegerField(read_only=True) - groups_in_count = serializers.IntegerField(read_only=True) - groups_in = serializers.SerializerMethodField() applications = serializers.SerializerMethodField() system = serializers.BooleanField(read_only=True) platform_default = serializers.BooleanField(read_only=True) @@ -115,8 +113,6 @@ class Meta: "access", "policyCount", "accessCount", - "groups_in_count", - "groups_in", "applications", "system", "platform_default", @@ -131,11 +127,6 @@ def get_applications(self, obj): """Get the list of applications in the role.""" return obtain_applications(obj) - def get_groups_in(self, obj): - """Get the groups where the role is in.""" - request = self.context.get("request") - return obtain_groups_in(obj, request).values("name", "uuid", "description") - def create(self, validated_data): """Create the role object in the database.""" name = validated_data.pop("name") diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 54a4a85b9..165c9773e 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -25,7 +25,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.db import IntegrityError, transaction -from django.db.models import F, Q +from django.db.models import Q from django.db.models.aggregates import Count from django.http import Http404 from django.utils.translation import gettext as _ @@ -34,8 +34,6 @@ from management.models import AuditLog, Permission from management.notifications.notification_handlers import role_obj_change_notification_handler from management.permissions import RoleAccessPermission -from management.principal.model import Principal -from management.principal.proxy import PrincipalProxy from management.querysets import get_role_queryset, user_has_perm from management.relation_replicator.relation_replicator import DualWriteException, ReplicationEventType from management.role.relation_api_dual_write_handler import ( @@ -49,9 +47,9 @@ from rest_framework.response import Response from api.common.pagination import StandardResultsSetPagination -from api.models import Tenant, User +from api.models import Tenant from rbac.env import ENVIRONMENT -from .model import ExtTenant, Role +from .model import Role from .serializer import RoleSerializer TESTING_APP = os.getenv("TESTING_APPLICATION") @@ -328,198 +326,7 @@ def list(self, request, *args, **kwargs): } """ - # Filtering - query_params = { - "external_tenant": request.query_params.get("external_tenant", None), - "system": request.query_params.get("system", None), - "application": request.query_params.get("application", None), - "display_name": request.query_params.get("display_name", None), - "name_match": request.query_params.get("name_match", None), - "permission": request.query_params.get("permission", None), - "name": request.query_params.get("name", None), - "platform_default": request.query_params.get("platform_default", None), - "admin_default": request.query_params.get("admin_default", None), - "username": request.query_params.get("username", None), - "limit": request.query_params.get("limit", 10), - "offset": request.query_params.get("offset", 0), - "add_fields": request.query_params.get("add_fields", ""), - } - add_fields = query_params["add_fields"] - roles = get_role_queryset(self.request) - base_queryset = roles - - # Dynamic annotation - if add_fields: - if "groups_in_count" in add_fields: - base_queryset = base_queryset.annotate(groups_in_count=Count("policies__group")) - - filters = [] - limit = query_params["limit"] - offset = int(query_params["offset"]) - path = request.path - - previous_offset = 0 - if offset - limit > 0: - previous_offset = offset - limit - - if query_params: - filtered_queryset = base_queryset - system_value = str(query_params["system"]).lower() - - # Check if "add_fields" is a valid field - additional_fields = {"access", "groups_in", "groups_in_count"} - if query_params["add_fields"]: - split_fields = query_params["add_fields"].split(",") - invalid_field = [field for field in split_fields if field not in additional_fields] - if invalid_field: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "errors": [ - { - "detail": "Invalid additional field passed in query", - "source": "add_fields invalid parameter", - "status": status.HTTP_400_BAD_REQUEST, - } - ] - }, - ) - - # Username filter - if query_params["username"]: - princ = Principal.objects.filter(username=query_params["username"]).first() - if not princ: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "errors": [ - { - "detail": "Principal not found for this username", - "source": "Invalid username query parameter", - "status": status.HTTP_400_BAD_REQUEST, - } - ] - }, - ) - - proxy = PrincipalProxy - results = proxy.request_filtered_principals( - query_params["username"], - org_id=request.user.org_id, - ) - results_exist = results["data"] - if results_exist: - principal = results["data"][0] - - # Convert from principal to User Model to add to request object - org_id = principal.get("org_id") - user_id = principal.get("user_id") - username = principal.get("username") - account_number = principal.get("account_number") - is_active = principal.get("is_active") - - # Map principal fields to User fields - user = User() - user.org_id = org_id - user.user_id = user_id - user.username = username - user.account = account_number - user.is_active = is_active - request.user_from_query = user - - # External tenant filter - if query_params["external_tenant"]: - ext_tenant = ExtTenant.objects.get(name=query_params["external_tenant"]) - if ext_tenant: - filters.append(Q(ext_relation__ext_tenant=ext_tenant)) - # System value filter - if system_value == "false": - filters.append(Q(system=False)) - elif system_value == "true": - filters.append(Q(system=True)) - # Application filter - if query_params["application"]: - applications = query_params["application"].split(",") - - external_tenant = ExtTenant.objects.filter(name=query_params["application"]).first() - - filtered_queryset = filtered_queryset.filter(access__permission__application__in=applications) - # If a external tenant exists with the name passed to application query parameter - # return the roles for that external tenant - if external_tenant: - ext_tenant = ExtTenant.objects.get(name=query_params["application"]) - filtered_queryset = base_queryset.filter(ext_relation__ext_tenant=ext_tenant).annotate( - external_tenant=F("ext_relation__ext_tenant__name") - ) - # Display_name & name_match filter - if query_params["display_name"]: - if query_params["name_match"] == "partial": - filters.append(Q(display_name__contains=query_params["display_name"])) - elif query_params["name_match"] == "exact": - filters.append(Q(display_name__exact=query_params["display_name"])) - elif not query_params["name_match"]: - filters.append(Q(display_name__contains=query_params["display_name"])) - else: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "errors": [ - { - "detail": "Invalid name match value provided", - "source": "name_match query parameter", - "status": status.HTTP_400_BAD_REQUEST, - } - ] - }, - ) - # name & name_match filter - if query_params["name"]: - if query_params["name_match"] == "partial": - filters.append(Q(name__contains=query_params["name"])) - elif query_params["name_match"] == "exact": - filters.append(Q(name__exact=query_params["name"])) - elif not query_params["name_match"]: - filters.append(Q(name__contains=query_params["name"])) - else: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "errors": [ - { - "detail": "Invalid name match value provided", - "source": "name_match query parameter", - "status": status.HTTP_400_BAD_REQUEST, - } - ] - }, - ) - # Permission filter - if query_params["permission"]: - permissions = query_params["permission"].split(",") - filters.append(Q(access__permission__permission__in=permissions)) - - # Apply the filters - filtered_queryset = filtered_queryset.filter(*filters) - - # Serialize the queryset for response - serializer = RoleSerializer(filtered_queryset, many=True, context={"request": request}) - - # Metadata - meta = {} - if query_params.get("username"): - meta["count"] = sum(len(group.roles()) for group in princ.group.all()) - else: - meta["count"] = filtered_queryset.count() - - # Pagination - links = { - "first": f"{path}?limit={limit}&offset=0", - "next": f"{path}?limit={limit}&offset={offset + limit}", - "previous": f"{path}?limit={limit}&offset={previous_offset}", - "last": f"{path}?limit={limit}&offset={meta['count'] - limit if (meta['count'] - limit) >= 0 else 0}", - } - - return Response({"meta": meta, "links": links, "data": serializer.data}) + return super().list(request=request, args=args, kwargs=kwargs) def retrieve(self, request, *args, **kwargs): """Get a role. diff --git a/rbac/rbac/settings.py b/rbac/rbac/settings.py index 6e832678b..d59c0c45f 100644 --- a/rbac/rbac/settings.py +++ b/rbac/rbac/settings.py @@ -84,7 +84,6 @@ ALLOWED_HOSTS = ["*"] - # Application definition INSTALLED_APPS = [ diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index 3d7f6ce25..c0c10c1ea 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -151,10 +151,8 @@ def setUp(self): "display_name", "system", "created", - "access", "policyCount", "accessCount", - "groups_in", "modified", "platform_default", "admin_default", @@ -665,7 +663,6 @@ def test_read_role_list_success(self): self.assertIsNotNone(iterRole.get("name")) # fields displayed are same as defined self.assertEqual(self.display_fields, set(iterRole.keys())) - if iterRole.get("name") == role_name: self.assertEqual(iterRole.get("accessCount"), 2) role = iterRole @@ -677,7 +674,6 @@ def test_get_role_by_application_single(self): url = "{}?application={}".format(URL, "app") client = APIClient() response = client.get(url, **self.headers) - self.assertEqual(response.data.get("meta").get("count"), 1) self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name) @@ -686,7 +682,6 @@ def test_get_role_by_application_using_ext_tenant(self): url = "{}?application={}".format(URL, "foo") client = APIClient() response = client.get(url, **self.headers) - self.assertEqual(response.data.get("meta").get("count"), 1) self.assertEqual(response.data.get("data")[0].get("name"), self.defRole.name) @@ -907,7 +902,7 @@ def test_list_role_with_groups_in_fields_with_username_param_for_non_org_admin(s # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == custom_role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role[groups_in_count], 2) + self.assertEqual(created_role[groups_in_count], 1) self.assertEqual(created_role[groups_in][0]["name"], custom_group_name) # make sure all roles are from: @@ -1003,7 +998,7 @@ def test_list_role_with_groups_in_fields_with_username_param_for_org_admin(self, # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == custom_role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role[groups_in_count], 2) + self.assertEqual(created_role[groups_in_count], 1) self.assertEqual(created_role[groups_in][0]["name"], custom_group_name) # make sure all roles are from: @@ -1085,13 +1080,13 @@ def test_list_role_with_groups_in_fields_for_principal_scope_success(self, mock_ # make sure created role exists in result set and has correct values created_role = next((iterRole for iterRole in response_data if iterRole["name"] == role_name), None) self.assertIsNotNone(created_role) - self.assertEqual(created_role["groups_in_count"], 2) + self.assertEqual(created_role["groups_in_count"], 1) self.assertEqual(created_role["groups_in"][0]["name"], group_name) # make sure a default role exists in result set and has correct values default_role = next((iterRole for iterRole in response_data if iterRole["name"] == self.defRole.name), None) self.assertIsNotNone(default_role) - self.assertEqual(default_role["groups_in_count"], 2) + self.assertEqual(default_role["groups_in_count"], 1) self.assertEqual(default_role["groups_in"][0]["name"], self.group.name) def test_list_role_with_groups_in_fields_for_admin_scope_success(self): @@ -1126,7 +1121,7 @@ def test_list_role_with_groups_in_fields_for_admin_scope_success(self): # make sure a default role exists in result set and has correct values default_role = next((iterRole for iterRole in response_data if iterRole["name"] == self.defRole.name), None) self.assertIsNotNone(default_role) - self.assertEqual(default_role["groups_in_count"], 2) + self.assertEqual(default_role["groups_in_count"], 1) self.assertEqual(default_role["groups_in"][0]["name"], self.group.name) # make sure an admin role exists in result set and has correct values @@ -1188,6 +1183,7 @@ def test_list_role_with_additional_fields_username_success(self, mock_request): url = "{}?add_fields={},{}&username={}".format(URL, field_1, field_2, self.principal.username) client = APIClient() response = client.get(url, **self.headers) + self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] @@ -1205,6 +1201,7 @@ def test_list_role_with_additional_fields_principal_success(self): url = "{}?add_fields={},{}&scope=principal".format(URL, field_1, field_2) client = APIClient() response = client.get(url, **self.headers) + self.assertEqual(len(response.data.get("data")), 5) role = response.data.get("data")[0] @@ -1735,6 +1732,7 @@ def test_external_tenant_filter(self): response = client.get(URL, **self.headers) self.assertEqual(len(response.data.get("data")), 5) + url = f"{URL}?external_tenant=foo" client = APIClient() response = client.get(url, **self.headers) @@ -1753,6 +1751,7 @@ def test_list_role_admin_platform_default_groups(self): url = f"{URL}?display_name=platform_admin_default_display&add_fields=groups_in_count%2Cgroups_in" client = APIClient() response = client.get(url, **self.headers) + self.assertEqual(len(response.data.get("data")), 1) role = response.data.get("data")[0] self.assertEqual(role.get("groups_in_count"), 2) @@ -1955,7 +1954,6 @@ def test_list_roles_with_User_Access_Admin_success(self): response = client.get(url, **self.headers_user_based_principal) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_count = self.system_roles_count + self.non_system_roles_count + 1 - self.assertEqual(len(response.data.get("data")), expected_count) response = client.get(url, **self.headers_service_account_principal) From eeb98de149444968cabd5d7e091481fb0a267d22 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 14 Jan 2025 10:49:15 +0000 Subject: [PATCH 15/16] reduction from to 6 queries for a request for roles endpoint from 33 queries --- rbac/management/querysets.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rbac/management/querysets.py b/rbac/management/querysets.py index aa6a7b371..5ad6bb85e 100644 --- a/rbac/management/querysets.py +++ b/rbac/management/querysets.py @@ -155,9 +155,9 @@ def get_role_queryset(request) -> QuerySet: """Obtain the queryset for roles.""" scope = validate_and_get_key(request.query_params, SCOPE_KEY, VALID_SCOPES, ORG_ID_SCOPE) public_tenant = Tenant.objects.get(tenant_name="public") - base_query = annotate_roles_with_counts(Role.objects.prefetch_related("access", "ext_relation")).filter( - tenant__in=[request.tenant, public_tenant] - ) + base_query = annotate_roles_with_counts( + Role.objects.prefetch_related("access", "ext_relation", "access__permission") + ).filter(tenant__in=[request.tenant, public_tenant]) if scope == PRINCIPAL_SCOPE: queryset = get_object_principal_queryset( From b8ef8fe9ac51f230e9632d00f13480fd7d2ae71f Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Tue, 14 Jan 2025 11:47:16 +0000 Subject: [PATCH 16/16] remove redundant default limit for common pagination# --- rbac/management/role/view.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rbac/management/role/view.py b/rbac/management/role/view.py index 165c9773e..0fcc42d5d 100644 --- a/rbac/management/role/view.py +++ b/rbac/management/role/view.py @@ -46,7 +46,6 @@ from rest_framework.filters import OrderingFilter from rest_framework.response import Response -from api.common.pagination import StandardResultsSetPagination from api.models import Tenant from rbac.env import ENVIRONMENT from .model import Role @@ -142,7 +141,6 @@ class RoleViewSet( filterset_class = RoleFilter ordering_fields = ("name", "display_name", "modified", "policyCount") ordering = ("name",) - default_limit = StandardResultsSetPagination.default_limit def get_queryset(self): """Obtain queryset for requesting user based on access and action."""