Skip to content

Commit

Permalink
Fix object-level permission bugs with DAB RBAC system (#15284)
Browse files Browse the repository at this point in the history
* Fix object-level permission bugs with DAB RBAC system

* Fix NT organization change regression

* Mark tests to AAP number
  • Loading branch information
AlanCoding authored Jun 20, 2024
1 parent 13dcea0 commit 4738c83
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 18 deletions.
15 changes: 5 additions & 10 deletions awx/main/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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):
Expand Down
7 changes: 0 additions & 7 deletions awx/main/tests/functional/api/test_instance_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
7 changes: 6 additions & 1 deletion awx/main/tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions awx/main/tests/functional/dab_rbac/test_access_regressions.py
Original file line number Diff line number Diff line change
@@ -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'})
2 changes: 2 additions & 0 deletions awx/main/tests/functional/test_rbac_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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})


Expand Down

0 comments on commit 4738c83

Please sign in to comment.