From 4738c8333a917ef09fa85ff2b50b9c9c03adaf4f Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 20 Jun 2024 16:34:34 -0400 Subject: [PATCH] Fix object-level permission bugs with DAB RBAC system (#15284) * Fix object-level permission bugs with DAB RBAC system * Fix NT organization change regression * Mark tests to AAP number --- awx/main/access.py | 15 ++++-------- .../functional/api/test_instance_group.py | 7 ------ awx/main/tests/functional/conftest.py | 7 +++++- .../dab_rbac/test_access_regressions.py | 23 +++++++++++++++++++ .../functional/test_rbac_notifications.py | 2 ++ 5 files changed, 36 insertions(+), 18 deletions(-) create mode 100644 awx/main/tests/functional/dab_rbac/test_access_regressions.py diff --git a/awx/main/access.py b/awx/main/access.py index f89d05cd2b33..9819f9d9aa07 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -598,7 +598,7 @@ class InstanceGroupAccess(BaseAccess): - a superuser - admin role on the Instance group I can add/delete Instance Groups: - - a superuser(system administrator) + - a superuser(system administrator), because these are not org-scoped I can use Instance Groups when I have: - use_role on the instance group """ @@ -627,7 +627,7 @@ def can_admin(self, obj): def can_delete(self, obj): if obj.name in [settings.DEFAULT_EXECUTION_QUEUE_NAME, settings.DEFAULT_CONTROL_PLANE_QUEUE_NAME]: return False - return self.user.is_superuser + return self.user.has_obj_perm(obj, 'delete') class UserAccess(BaseAccess): @@ -2628,7 +2628,7 @@ def can_delete(self, obj): class NotificationTemplateAccess(BaseAccess): """ - I can see/use a notification_template if I have permission to + Run standard logic from DAB RBAC """ model = NotificationTemplate @@ -2649,10 +2649,7 @@ def can_add(self, data): @check_superuser def can_change(self, obj, data): - if obj.organization is None: - # only superusers are allowed to edit orphan notification templates - return False - return self.check_related('organization', Organization, data, obj=obj, role_field='notification_admin_role', mandatory=True) + return self.user.has_obj_perm(obj, 'change') and self.check_related('organization', Organization, data, obj=obj, role_field='notification_admin_role') def can_admin(self, obj, data): return self.can_change(obj, data) @@ -2662,9 +2659,7 @@ def can_delete(self, obj): @check_superuser def can_start(self, obj, validate_license=True): - if obj.organization is None: - return False - return self.user in obj.organization.notification_admin_role + return self.can_change(obj, None) class NotificationAccess(BaseAccess): diff --git a/awx/main/tests/functional/api/test_instance_group.py b/awx/main/tests/functional/api/test_instance_group.py index aa8204c6dae9..5bc56940c980 100644 --- a/awx/main/tests/functional/api/test_instance_group.py +++ b/awx/main/tests/functional/api/test_instance_group.py @@ -32,13 +32,6 @@ def fn(hostname, node_type): return fn -@pytest.fixture -def instance_group(job_factory): - ig = InstanceGroup(name="east") - ig.save() - return ig - - @pytest.fixture def containerized_instance_group(instance_group, kube_credential): ig = InstanceGroup(name="container") diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index abecda397e2c..b23a9a7d2f64 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -20,7 +20,7 @@ # AWX from awx.main.models.projects import Project -from awx.main.models.ha import Instance +from awx.main.models.ha import Instance, InstanceGroup from rest_framework.test import ( APIRequestFactory, @@ -730,6 +730,11 @@ def jt_linked(organization, project, inventory, machine_credential, credential, return jt +@pytest.fixture +def instance_group(): + return InstanceGroup.objects.create(name="east") + + @pytest.fixture def workflow_job_template(organization): wjt = WorkflowJobTemplate.objects.create(name='test-workflow_job_template', organization=organization) diff --git a/awx/main/tests/functional/dab_rbac/test_access_regressions.py b/awx/main/tests/functional/dab_rbac/test_access_regressions.py new file mode 100644 index 000000000000..21b5560dceed --- /dev/null +++ b/awx/main/tests/functional/dab_rbac/test_access_regressions.py @@ -0,0 +1,23 @@ +import pytest + +from awx.main.access import InstanceGroupAccess, NotificationTemplateAccess + +from ansible_base.rbac.models import RoleDefinition + + +@pytest.mark.django_db +def test_instance_group_object_role_delete(rando, instance_group, setup_managed_roles): + """Basic functionality of IG object-level admin role function AAP-25506""" + rd = RoleDefinition.objects.get(name='InstanceGroup Admin') + rd.give_permission(rando, instance_group) + access = InstanceGroupAccess(rando) + assert access.can_delete(instance_group) + + +@pytest.mark.django_db +def test_notification_template_object_role_change(rando, notification_template, setup_managed_roles): + """Basic functionality of NT object-level admin role function AAP-25493""" + rd = RoleDefinition.objects.get(name='NotificationTemplate Admin') + rd.give_permission(rando, notification_template) + access = NotificationTemplateAccess(rando) + assert access.can_change(notification_template, {'name': 'new name'}) diff --git a/awx/main/tests/functional/test_rbac_notifications.py b/awx/main/tests/functional/test_rbac_notifications.py index d05efa244c9b..72d5d016a954 100644 --- a/awx/main/tests/functional/test_rbac_notifications.py +++ b/awx/main/tests/functional/test_rbac_notifications.py @@ -99,7 +99,9 @@ def test_notification_template_access_org_user(notification_template, user): @pytest.mark.django_db def test_notificaiton_template_orphan_access_org_admin(notification_template, organization, org_admin): notification_template.organization = None + notification_template.save(update_fields=['organization']) access = NotificationTemplateAccess(org_admin) + assert not org_admin.has_obj_perm(notification_template, 'change') assert not access.can_change(notification_template, {'organization': organization.id})