diff --git a/awx/api/pagination.py b/awx/api/pagination.py index 95f7b585..95f6a9a7 100644 --- a/awx/api/pagination.py +++ b/awx/api/pagination.py @@ -6,12 +6,15 @@ # Django REST Framework from django.conf import settings from django.core.paginator import Paginator as DjangoPaginator +from django.utils.functional import cached_property from rest_framework import pagination from rest_framework.response import Response from rest_framework.utils.urls import replace_query_param from rest_framework.settings import api_settings from django.utils.translation import gettext_lazy as _ +from awx.main.models import UnifiedJob + class DisabledPaginator(DjangoPaginator): @property @@ -23,6 +26,18 @@ def count(self): return 200 +class UnifiedJobPaginator(DjangoPaginator): + """Use unfiltered table count for unified job pagination. + + The RBAC-filtered COUNT query is prohibitively slow on large unified job + tables; an approximate over-count is harmless for pagination UI. + """ + + @cached_property + def count(self): + return UnifiedJob.objects.count() + + class Pagination(pagination.PageNumberPagination): page_size_query_param = 'page_size' max_page_size = settings.MAX_PAGE_SIZE @@ -57,12 +72,13 @@ def get_html_context(self): def paginate_queryset(self, queryset, request, **kwargs): self.count_disabled = 'count_disabled' in request.query_params + original_paginator = self.django_paginator_class try: if self.count_disabled: self.django_paginator_class = DisabledPaginator return super(Pagination, self).paginate_queryset(queryset, request, **kwargs) finally: - self.django_paginator_class = DjangoPaginator + self.django_paginator_class = original_paginator def get_paginated_response(self, data): if self.count_disabled: @@ -70,6 +86,10 @@ def get_paginated_response(self, data): return super(Pagination, self).get_paginated_response(data) +class UnifiedJobPagination(Pagination): + django_paginator_class = UnifiedJobPaginator + + 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 a7459cbf..e13a172a 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -120,7 +120,7 @@ NoTruncateMixin, UnifiedJobIncludeMixin, ) -from awx.api.pagination import UnifiedJobEventPagination +from awx.api.pagination import UnifiedJobEventPagination, UnifiedJobPagination from awx.main.utils import set_environ logger = logging.getLogger('awx.api.views') @@ -4176,6 +4176,7 @@ class UnifiedJobList(UnifiedJobIncludeMixin, ListAPIView): model = models.UnifiedJob serializer_class = serializers.UnifiedJobListSerializer search_fields = ('description', 'name', 'job__playbook') + pagination_class = UnifiedJobPagination # Pre-compile ANSI patterns for performance diff --git a/awx/main/access.py b/awx/main/access.py index 8cdb2847..f2335d54 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -11,6 +11,7 @@ from django.conf import settings from django.db.models import Q, Prefetch from django.contrib.auth.models import User +from django.contrib.contenttypes.models import ContentType from django.utils.translation import gettext_lazy as _ from django.core.exceptions import ObjectDoesNotExist, FieldDoesNotExist @@ -77,6 +78,7 @@ ROLE_SINGLETON_SYSTEM_AUDITOR, ) from awx.main.models.mixins import ResourceMixin +from awx.main.models.rbac import RoleAncestorEntry __all__ = [ 'get_user_queryset', @@ -2587,15 +2589,50 @@ class UnifiedJobAccess(BaseAccess): # ) def filtered_queryset(self): - inv_pk_qs = Inventory._accessible_pk_qs(Inventory, self.user, 'read_role') - org_auditor_qs = Organization.objects.filter(Q(admin_role__members=self.user) | Q(auditor_role__members=self.user)) - qs = self.model.objects.filter( - Q(unified_job_template_id__in=UnifiedJobTemplate.accessible_pk_qs(self.user, 'read_role')) - | Q(pk__in=InventoryUpdate.objects.filter(inventory_source__inventory_id__in=inv_pk_qs).values('pk')) - | Q(pk__in=AdHocCommand.objects.filter(inventory_id__in=inv_pk_qs).values('pk')) - | Q(organization__in=org_auditor_qs) + # Pre-compute the user's direct role memberships once and pass them + # as literal parameters to each RBAC branch, and use OR (not UNION) + # so the database can single-pass filter with an early LIMIT exit. + # UNION forces materialization of every accessible job id before + # ordering/LIMIT can apply, and repeats the role-membership subquery + # in every branch. + user_role_ids = list(self.user.roles.values_list('id', flat=True)) + if not user_role_ids: + return self.model.objects.none() + + ujt_accessible = ( + RoleAncestorEntry.objects.filter( + ancestor_id__in=user_role_ids, + role_field='read_role', + content_type_id__in=UnifiedJobTemplate._submodels_with_roles(), + ) + .values_list('object_id') + .distinct() + ) + + inv_accessible = ( + RoleAncestorEntry.objects.filter( + ancestor_id__in=user_role_ids, + role_field='read_role', + content_type_id=ContentType.objects.get_for_model(Inventory).id, + ) + .values_list('object_id') + .distinct() + ) + + return self.model.objects.filter( + Q(unified_job_template_id__in=ujt_accessible) + | Q( + pk__in=InventoryUpdate.objects.filter( + inventory_source__inventory_id__in=inv_accessible, + ).values('pk') + ) + | Q( + pk__in=AdHocCommand.objects.filter( + inventory_id__in=inv_accessible, + ).values('pk') + ) + | Q(organization__in=Organization.objects.filter(Q(admin_role_id__in=user_role_ids) | Q(auditor_role_id__in=user_role_ids))) ) - return qs def get_queryset(self): return super(UnifiedJobAccess, self).get_queryset().filter(workflowapproval__isnull=True) diff --git a/awx/main/tests/conftest.py b/awx/main/tests/conftest.py index 3b8d3e55..83773d51 100644 --- a/awx/main/tests/conftest.py +++ b/awx/main/tests/conftest.py @@ -22,6 +22,21 @@ def pytest_addoption(parser): parser.addoption("--genschema", action="store_true", default=False, help="execute schema validator") +@pytest.fixture(scope='session') +def django_db_setup(django_db_setup, django_db_blocker): + # django-ansible-base's resource registry populates ResourceType rows in a + # post_migrate handler, but DAB devel skips it when the migration plan is + # empty — which is always the case under pytest --nomigrations. Without + # these rows, saving any registered model (Organization, User, Team, ...) + # raises ContentType.resource_type.RelatedObjectDoesNotExist. Run the + # initialization explicitly after the test database is created. + from ansible_base.resource_registry.apps import initialize_resources + + with django_db_blocker.unblock(): + initialize_resources(None) + yield + + def pytest_configure(config): import sys diff --git a/awx/main/tests/functional/test_rbac_unified_jobs.py b/awx/main/tests/functional/test_rbac_unified_jobs.py new file mode 100644 index 00000000..a915917d --- /dev/null +++ b/awx/main/tests/functional/test_rbac_unified_jobs.py @@ -0,0 +1,205 @@ +import pytest + +from django.test.utils import CaptureQueriesContext +from django.db import connection + +from awx.api.versioning import reverse +from awx.main.models import ( + AdHocCommand, + InventorySource, + InventoryUpdate, + JobTemplate, + Organization, + Project, + UnifiedJob, +) + + +@pytest.mark.django_db +def test_unified_job_list_uses_or_not_union(user, organization, inventory, get): + """The unified job list RBAC query uses OR-based filtering, not UNION.""" + org_admin = user('uj-org-admin') + organization.admin_role.members.add(org_admin) + + project = Project.objects.create(name='uj-test-project', organization=organization) + jt = JobTemplate.objects.create(name='uj-test-jt', project=project, inventory=inventory, organization=organization) + jt.create_unified_job() + + inv_src = InventorySource.objects.create(name='uj-test-invsrc', inventory=inventory, source='ec2') + InventoryUpdate.objects.create(inventory_source=inv_src, source=inv_src.source) + + AdHocCommand.objects.create(name='uj-test-adhoc', inventory=inventory) + + with CaptureQueriesContext(connection) as ctx: + response = get(reverse('api:unified_job_list'), org_admin) + + assert response.status_code == 200 + assert response.data['count'] >= 3 + + uj_rbac_queries = [q['sql'] for q in ctx.captured_queries if 'main_unifiedjob' in q['sql'] and 'main_rbac_role_ancestors' in q['sql']] + assert uj_rbac_queries, "Expected a unified-job RBAC query" + for sql in uj_rbac_queries: + assert 'UNION' not in sql, "RBAC query should use OR, not UNION" + + +@pytest.mark.django_db +def test_unified_job_list_org_auditor_sees_jobs(user, get): + """Org auditors see unified jobs in their org via the org auditor RBAC branch.""" + org = Organization.objects.create(name='uj-audit-org') + auditor = user('uj-auditor') + org.auditor_role.members.add(auditor) + + inventory = org.inventories.create(name='uj-audit-inv') + project = Project.objects.create(name='uj-audit-project', organization=org) + jt = JobTemplate.objects.create(name='uj-audit-jt', project=project, inventory=inventory, organization=org) + job = jt.create_unified_job() + + response = get(reverse('api:unified_job_list'), auditor) + assert response.status_code == 200 + result_ids = [r['id'] for r in response.data['results']] + assert job.pk in result_ids + + +@pytest.mark.django_db +def test_unified_job_list_inventory_viewer_sees_inventory_updates(user, get): + """Users with inventory read permission see inventory updates via the inventory RBAC branch.""" + org = Organization.objects.create(name='uj-inv-org') + inventory = org.inventories.create(name='uj-inv-test') + inv_viewer = user('uj-inv-viewer') + inventory.read_role.members.add(inv_viewer) + + inv_src = InventorySource.objects.create(name='uj-inv-src', inventory=inventory, source='ec2') + inv_update = InventoryUpdate.objects.create(inventory_source=inv_src, source=inv_src.source) + + response = get(reverse('api:unified_job_list'), inv_viewer) + assert response.status_code == 200 + result_ids = [r['id'] for r in response.data['results']] + assert inv_update.pk in result_ids + + +@pytest.mark.django_db +def test_unified_job_list_team_grant_sees_jobs(user, get): + """Access granted through a team (not a direct user->role grant) still + surfaces jobs; the pre-computed role set must include team-mediated + ancestry, not just directly-granted object roles.""" + from awx.main.models import Team + + org = Organization.objects.create(name='uj-team-org') + inventory = org.inventories.create(name='uj-team-inv') + project = Project.objects.create(name='uj-team-project', organization=org) + jt = JobTemplate.objects.create(name='uj-team-jt', project=project, inventory=inventory, organization=org) + job = jt.create_unified_job() + + team = Team.objects.create(name='uj-team', organization=org) + team_member = user('uj-team-member') + team.member_role.members.add(team_member) + jt.read_role.parents.add(team.member_role) + + response = get(reverse('api:unified_job_list'), team_member) + assert response.status_code == 200 + result_ids = [r['id'] for r in response.data['results']] + assert job.pk in result_ids + + +@pytest.mark.django_db +def test_unified_job_list_org_member_sees_nothing(user, get): + """A user with roles (org member) but no job-related access sees no jobs. + Unlike rando, this user has a non-empty role set, so it exercises the + full OR query rather than the empty-role-set early exit.""" + org = Organization.objects.create(name='uj-member-org') + member = user('uj-member') + org.member_role.members.add(member) + + inventory = org.inventories.create(name='uj-member-inv') + project = Project.objects.create(name='uj-member-project', organization=org) + jt = JobTemplate.objects.create(name='uj-member-jt', project=project, inventory=inventory, organization=org) + jt.create_unified_job() + + response = get(reverse('api:unified_job_list'), member) + assert response.status_code == 200 + assert len(response.data['results']) == 0 + + +@pytest.mark.django_db +def test_unified_job_list_superuser_no_roles_sees_all(user, get): + """A superuser with zero role memberships sees all jobs. Superusers are + handled by the BaseAccess.get_queryset short-circuit, so the empty-role-set + early exit in filtered_queryset must never be reachable for them.""" + superuser = user('uj-superuser', True) + # Old-RBAC signals auto-enroll superusers in the system_administrator + # singleton role; strip role memberships (restoring the flag without + # signals) to prove the bypass does not depend on any role rows. + superuser.roles.clear() + type(superuser).objects.filter(pk=superuser.pk).update(is_superuser=True) + superuser.refresh_from_db() + assert superuser.is_superuser + assert superuser.roles.count() == 0 + + org = Organization.objects.create(name='uj-super-org') + inventory = org.inventories.create(name='uj-super-inv') + project = Project.objects.create(name='uj-super-project', organization=org) + jt = JobTemplate.objects.create(name='uj-super-jt', project=project, inventory=inventory, organization=org) + job = jt.create_unified_job() + + response = get(reverse('api:unified_job_list'), superuser) + assert response.status_code == 200 + result_ids = [r['id'] for r in response.data['results']] + assert job.pk in result_ids + + +@pytest.mark.django_db +def test_unified_job_list_system_auditor_sees_all(system_auditor, get): + """System auditors bypass filtered_queryset via BaseAccess.get_queryset — + the closest analog of upstream's singleton-permission shortcut paths.""" + org = Organization.objects.create(name='uj-sysaud-org') + inventory = org.inventories.create(name='uj-sysaud-inv') + project = Project.objects.create(name='uj-sysaud-project', organization=org) + jt = JobTemplate.objects.create(name='uj-sysaud-jt', project=project, inventory=inventory, organization=org) + job = jt.create_unified_job() + + inv_src = InventorySource.objects.create(name='uj-sysaud-invsrc', inventory=inventory, source='ec2') + inv_update = InventoryUpdate.objects.create(inventory_source=inv_src, source=inv_src.source) + + adhoc = AdHocCommand.objects.create(name='uj-sysaud-adhoc', inventory=inventory) + + response = get(reverse('api:unified_job_list'), system_auditor) + assert response.status_code == 200 + result_ids = [r['id'] for r in response.data['results']] + assert job.pk in result_ids + assert inv_update.pk in result_ids + assert adhoc.pk in result_ids + + +@pytest.mark.django_db +def test_unified_job_list_rando_sees_nothing(rando, get): + """Unprivileged user sees no unified jobs.""" + org = Organization.objects.create(name='uj-rando-org') + inventory = org.inventories.create(name='uj-rando-inv') + project = Project.objects.create(name='uj-rando-project', organization=org) + jt = JobTemplate.objects.create(name='uj-rando-jt', project=project, inventory=inventory, organization=org) + jt.create_unified_job() + AdHocCommand.objects.create(name='uj-rando-adhoc', inventory=inventory) + + response = get(reverse('api:unified_job_list'), rando) + assert response.status_code == 200 + assert len(response.data['results']) == 0 + + +@pytest.mark.django_db +def test_unified_job_list_pagination_uses_unfiltered_count(rando, get): + """The pagination count should reflect total unified job rows, not + the RBAC-filtered subset. The RBAC-filtered COUNT is catastrophically + slow on large tables with pk__in UNION subqueries.""" + org = Organization.objects.create(name='uj-count-org') + inventory = org.inventories.create(name='uj-count-inv') + project = Project.objects.create(name='uj-count-project', organization=org) + jt = JobTemplate.objects.create(name='uj-count-jt', project=project, inventory=inventory, organization=org) + jt.create_unified_job() + + total_jobs = UnifiedJob.objects.count() + assert total_jobs > 0 + + response = get(reverse('api:unified_job_list'), rando) + assert response.status_code == 200 + assert len(response.data['results']) == 0 + assert response.data['count'] == total_jobs diff --git a/awx/main/tests/unit/api/test_pagination.py b/awx/main/tests/unit/api/test_pagination.py new file mode 100644 index 00000000..d4fe588f --- /dev/null +++ b/awx/main/tests/unit/api/test_pagination.py @@ -0,0 +1,63 @@ +from unittest.mock import patch, MagicMock + +from awx.api.pagination import UnifiedJobPaginator, UnifiedJobPagination, DisabledPaginator + + +class TestUnifiedJobPaginator: + def test_count_uses_unfiltered_table_count(self): + with patch('awx.api.pagination.UnifiedJob') as mock_uj: + mock_uj.objects.count.return_value = 42000 + paginator = UnifiedJobPaginator(object_list=[], per_page=25) + assert paginator.count == 42000 + mock_uj.objects.count.assert_called_once() + + def test_count_is_cached(self): + with patch('awx.api.pagination.UnifiedJob') as mock_uj: + mock_uj.objects.count.return_value = 500 + paginator = UnifiedJobPaginator(object_list=[], per_page=25) + _ = paginator.count + _ = paginator.count + mock_uj.objects.count.assert_called_once() + + +class TestUnifiedJobPagination: + def test_default_paginator_class(self): + pagination = UnifiedJobPagination() + assert pagination.django_paginator_class is UnifiedJobPaginator + + def test_normal_request_preserves_unified_job_paginator(self): + pagination = UnifiedJobPagination() + request = MagicMock() + request.query_params = {} + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]): + pagination.paginate_queryset(MagicMock(), request) + + assert pagination.count_disabled is False + assert pagination.django_paginator_class is UnifiedJobPaginator + + def test_count_disabled_restores_unified_job_paginator(self): + pagination = UnifiedJobPagination() + request = MagicMock() + request.query_params = {'count_disabled': 'true'} + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]): + pagination.paginate_queryset(MagicMock(), request) + + assert pagination.count_disabled is True + assert pagination.django_paginator_class is UnifiedJobPaginator + + def test_count_disabled_temporarily_uses_disabled_paginator(self): + pagination = UnifiedJobPagination() + request = MagicMock() + request.query_params = {'count_disabled': 'true'} + captured_class = {} + + def capture_paginator_class(self_inner, queryset, request, **kwargs): + captured_class['during'] = pagination.django_paginator_class + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', capture_paginator_class): + pagination.paginate_queryset(MagicMock(), request) + + assert captured_class['during'] is DisabledPaginator + assert pagination.django_paginator_class is UnifiedJobPaginator