-
Notifications
You must be signed in to change notification settings - Fork 24
Upstream 16555 - Replace 4-way OR with UNION in unified job list RBAC query #645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
cigamit
wants to merge
3
commits into
main
Choose a base branch
from
upstream16555
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.