Skip to content

Commit

Permalink
Narrow the actor types accepted for RBAC evaluations (ansible#14709)
Browse files Browse the repository at this point in the history
* Narrow the scope of RBAC evaluations

* Update tests for RBAC method changes

* Simplify querset for credentials in org

* Fix call pattern to pass in team role obj
  • Loading branch information
AlanCoding authored and djyasin committed Nov 11, 2024
1 parent 669eaaf commit 9d16807
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 18 deletions.
6 changes: 3 additions & 3 deletions awx/api/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,8 @@ def get_queryset(self):
qs = self.request.user.get_queryset(self.model)
return qs.filter(
Q(team=parent)
| Q(project__in=models.Project.accessible_objects(parent, 'read_role'))
| Q(credential__in=models.Credential.accessible_objects(parent, 'read_role'))
| Q(project__in=models.Project.accessible_objects(parent.member_role, 'read_role'))
| Q(credential__in=models.Credential.accessible_objects(parent.member_role, 'read_role'))
)


Expand Down Expand Up @@ -1397,7 +1397,7 @@ def get_queryset(self):
self.check_parent_access(organization)

user_visible = models.Credential.accessible_objects(self.request.user, 'read_role').all()
org_set = models.Credential.accessible_objects(organization.admin_role, 'read_role').all()
org_set = models.Credential.objects.filter(organization=organization)

if self.request.user.is_superuser or self.request.user.is_system_auditor:
return org_set
Expand Down
4 changes: 1 addition & 3 deletions awx/main/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
# Django
from django.apps import apps
from django.conf import settings
from django.contrib.auth.models import User # noqa
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError
from django.db import models
Expand Down Expand Up @@ -69,8 +68,7 @@ def _accessible_pk_qs(cls, accessor, role_field, content_types=None):
elif type(accessor) == Role:
ancestor_roles = [accessor]
else:
accessor_type = ContentType.objects.get_for_model(accessor)
ancestor_roles = Role.objects.filter(content_type__pk=accessor_type.id, object_id=accessor.id)
raise RuntimeError(f'Role filters only valid for users and ancestor role, received {accessor}')

if content_types is None:
ct_kwarg = dict(content_type_id=ContentType.objects.get_for_model(cls).id)
Expand Down
9 changes: 1 addition & 8 deletions awx/main/models/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

# AWX
from awx.api.versioning import reverse
from django.contrib.auth.models import User # noqa

__all__ = [
'Role',
Expand Down Expand Up @@ -171,14 +170,8 @@ def get_absolute_url(self, request=None):
def __contains__(self, accessor):
if accessor._meta.model_name == 'user':
return self.ancestors.filter(members=accessor).exists()
elif accessor.__class__.__name__ == 'Team':
return self.ancestors.filter(pk=accessor.member_role.id).exists()
elif type(accessor) == Role:
return self.ancestors.filter(pk=accessor.pk).exists()
else:
accessor_type = ContentType.objects.get_for_model(accessor)
roles = Role.objects.filter(content_type__pk=accessor_type.id, object_id=accessor.id)
return self.ancestors.filter(pk__in=roles).exists()
raise RuntimeError(f'Role evaluations only valid for users, received {accessor}')

@property
def name(self):
Expand Down
4 changes: 2 additions & 2 deletions awx/main/tests/functional/test_rbac_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,6 @@ def test_auto_parenting():
@pytest.mark.django_db
def test_update_parents_keeps_teams(team, project):
project.update_role.parents.add(team.member_role)
assert team.member_role in project.update_role # test prep sanity check
assert list(Project.accessible_objects(team.member_role, 'update_role')) == [project] # test prep sanity check
update_role_parentage_for_instance(project)
assert team.member_role in project.update_role # actual assertion
assert list(Project.accessible_objects(team.member_role, 'update_role')) == [project] # actual assertion
4 changes: 2 additions & 2 deletions awx/main/tests/functional/test_rbac_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_team_accessible_by(team, user, project):
u = user('team_member', False)

team.member_role.children.add(project.use_role)
assert team in project.read_role
assert list(Project.accessible_objects(team.member_role, 'read_role')) == [project]
assert u not in project.read_role

team.member_role.members.add(u)
Expand All @@ -104,7 +104,7 @@ def test_team_accessible_objects(team, user, project):
u = user('team_member', False)

team.member_role.children.add(project.use_role)
assert len(Project.accessible_objects(team, 'read_role')) == 1
assert len(Project.accessible_objects(team.member_role, 'read_role')) == 1
assert not Project.accessible_objects(u, 'read_role')

team.member_role.members.add(u)
Expand Down

0 comments on commit 9d16807

Please sign in to comment.