diff --git a/awx/api/pagination.py b/awx/api/pagination.py index de5cb2b56c..517fa40dfc 100644 --- a/awx/api/pagination.py +++ b/awx/api/pagination.py @@ -45,6 +45,15 @@ class UnifiedJobPaginator(DjangoPaginator): return UnifiedJob.objects.count() +class JobPaginator(DjangoPaginator): + """Count visible jobs through a primary-key subquery instead of full rows.""" + + @cached_property + def count(self): + visible_pks = self.object_list.order_by().values("pk") + return self.object_list.model.objects.filter(pk__in=visible_pks).count() + + class Pagination(pagination.PageNumberPagination): page_size_query_param = "page_size" max_page_size = settings.MAX_PAGE_SIZE @@ -135,6 +144,10 @@ class UnifiedJobPagination(Pagination): return DjangoPaginator +class JobPagination(Pagination): + django_paginator_class = JobPaginator + + class LimitPagination(pagination.BasePagination): default_limit = api_settings.PAGE_SIZE limit_query_param = "limit" diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index b2c202ddda..380f633580 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -130,7 +130,7 @@ from awx.api.views.mixin import ( NoTruncateMixin, UnifiedJobExcludeMixin, ) -from awx.api.pagination import ActivityStreamPagination, UnifiedJobEventPagination, UnifiedJobPagination +from awx.api.pagination import ActivityStreamPagination, JobPagination, UnifiedJobEventPagination, UnifiedJobPagination from awx.main.utils import set_environ logger = logging.getLogger('awx.api.views') @@ -3885,6 +3885,7 @@ class SystemJobTemplateNotificationTemplatesSuccessList(SystemJobTemplateNotific class JobList(UnifiedJobExcludeMixin, ListAPIView): model = models.Job serializer_class = serializers.JobListSerializer + pagination_class = JobPagination resource_purpose = 'jobs' @extend_schema_if_available(extensions={"x-ai-description": "A list of jobs."}) diff --git a/awx/main/access.py b/awx/main/access.py index c4c41bc164..d8f0dac031 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1680,7 +1680,7 @@ class JobAccess(BaseAccess): if not org_access_qs.exists(): return qs.filter(job_template__in=JobTemplate.access_qs(self.user, 'view')) - return qs.filter(Q(job_template__in=JobTemplate.access_qs(self.user, 'view')) | Q(organization__in=org_access_qs)).distinct() + return qs.filter(Q(job_template__in=JobTemplate.access_qs(self.user, 'view')) | Q(organization__in=org_access_qs)) def can_add(self, data, validate_license=True): raise NotImplementedError('Direct job creation not possible in v2 API') diff --git a/awx/main/tests/functional/dab_rbac/test_unified_job_access.py b/awx/main/tests/functional/dab_rbac/test_unified_job_access.py index 2051f53352..69d604e28f 100644 --- a/awx/main/tests/functional/dab_rbac/test_unified_job_access.py +++ b/awx/main/tests/functional/dab_rbac/test_unified_job_access.py @@ -138,6 +138,41 @@ def test_unified_job_list_team_member_sees_team_granted_jobs(user, setup_managed assert job.pk in result_ids, f"Team member should see job {job.pk} via team-granted JT execute permission, but got result IDs: {result_ids}" +@pytest.mark.django_db +def test_job_list_non_superuser_count_matches_visible_jobs(user, setup_managed_roles, get): + """The job list count must include only jobs visible to the requesting user.""" + visible_org = Organization.objects.create(name='job-list-visible-org') + visible_inventory = visible_org.inventories.create(name='job-list-visible-inv') + visible_project = Project.objects.create(name='job-list-visible-project', organization=visible_org) + visible_jt = JobTemplate.objects.create( + name='job-list-visible-jt', + project=visible_project, + inventory=visible_inventory, + organization=visible_org, + ) + visible_job = visible_jt.create_unified_job() + + hidden_org = Organization.objects.create(name='job-list-hidden-org') + hidden_inventory = hidden_org.inventories.create(name='job-list-hidden-inv') + hidden_project = Project.objects.create(name='job-list-hidden-project', organization=hidden_org) + hidden_jt = JobTemplate.objects.create( + name='job-list-hidden-jt', + project=hidden_project, + inventory=hidden_inventory, + organization=hidden_org, + ) + hidden_jt.create_unified_job() + + org_admin = user('job-list-org-admin') + RoleDefinition.objects.get(name='Organization Admin').give_permission(org_admin, visible_org) + + response = get(reverse('api:job_list'), org_admin) + + assert response.status_code == 200 + assert response.data['count'] == 1 + assert [result['id'] for result in response.data['results']] == [visible_job.pk] + + @pytest.mark.django_db def test_unified_job_list_rando_sees_nothing(rando, setup_managed_roles, get): """Unprivileged user sees no unified jobs.""" diff --git a/awx/main/tests/functional/rbac/test_rbac_job.py b/awx/main/tests/functional/rbac/test_rbac_job.py index c4bcee00d6..01cdece614 100644 --- a/awx/main/tests/functional/rbac/test_rbac_job.py +++ b/awx/main/tests/functional/rbac/test_rbac_job.py @@ -102,6 +102,14 @@ def test_org_auditor_sees_orphans(normal_job, org_auditor): assert access.can_read(normal_job) +@pytest.mark.django_db +def test_job_access_queryset_does_not_add_distinct(normal_job, org_admin): + queryset = JobAccess(org_admin).filtered_queryset() + + assert not queryset.query.distinct + assert queryset.count() == queryset.values('pk').distinct().count() + + # Delete permissions testing @pytest.mark.django_db def test_JT_admin_delete_denied(normal_job, rando): diff --git a/awx/main/tests/unit/api/test_pagination.py b/awx/main/tests/unit/api/test_pagination.py index 32eca80af1..ebd8192d38 100644 --- a/awx/main/tests/unit/api/test_pagination.py +++ b/awx/main/tests/unit/api/test_pagination.py @@ -1,6 +1,14 @@ from unittest.mock import patch, MagicMock -from awx.api.pagination import ActivityStreamPaginator, ActivityStreamPagination, UnifiedJobPaginator, UnifiedJobPagination, DisabledPaginator +from awx.api.pagination import ( + ActivityStreamPaginator, + ActivityStreamPagination, + DisabledPaginator, + JobPaginator, + JobPagination, + UnifiedJobPaginator, + UnifiedJobPagination, +) class TestActivityStreamPaginator: @@ -80,6 +88,37 @@ class TestUnifiedJobPaginator: mock_uj.objects.count.assert_called_once() +class TestJobPaginator: + def test_count_uses_primary_key_subquery(self): + object_list = MagicMock() + object_list.order_by.return_value.values.return_value = "visible_pks" + object_list.model.objects.filter.return_value.count.return_value = 713 + + paginator = JobPaginator(object_list=object_list, per_page=25) + + assert paginator.count == 713 + object_list.order_by.assert_called_once_with() + object_list.order_by.return_value.values.assert_called_once_with("pk") + object_list.model.objects.filter.assert_called_once_with(pk__in="visible_pks") + + def test_count_is_cached(self): + object_list = MagicMock() + object_list.order_by.return_value.values.return_value = "visible_pks" + object_list.model.objects.filter.return_value.count.return_value = 500 + paginator = JobPaginator(object_list=object_list, per_page=25) + + _ = paginator.count + _ = paginator.count + + object_list.model.objects.filter.assert_called_once_with(pk__in="visible_pks") + + +class TestJobPagination: + def test_default_paginator_class(self): + pagination = JobPagination() + assert pagination.django_paginator_class is JobPaginator + + class TestUnifiedJobPagination: def test_default_paginator_class(self): pagination = UnifiedJobPagination()