diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 5dae65e338..ec497d97f6 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1443,8 +1443,13 @@ class RoleSerializer(BaseSerializer): class Meta: model = Role - fields = ('*', 'description', 'name') - read_only_fields = ('description', 'name') + read_only_fields = ('id', 'role_field', 'description', 'name') + + def to_representation(self, obj): + ret = super(RoleSerializer, self).to_representation(obj) + ret.pop('created') + ret.pop('modified') + return ret def get_related(self, obj): ret = super(RoleSerializer, self).get_related(obj) diff --git a/awx/api/views.py b/awx/api/views.py index 3e70cb8099..81351a9066 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -814,8 +814,10 @@ class TeamRolesList(SubListCreateAttachDetachAPIView): relationship='member_role.children' def get_queryset(self): - team = Team.objects.get(pk=self.kwargs['pk']) - return team.member_role.children.filter(id__in=Role.visible_roles(self.request.user)) + team = get_object_or_404(Team, pk=self.kwargs['pk']) + if not self.request.user.can_access(Team, 'read', team): + raise PermissionDenied() + return Role.filter_visible_roles(self.request.user, team.member_role.children.all()) # XXX: Need to enforce permissions def post(self, request, *args, **kwargs): @@ -1081,8 +1083,10 @@ class UserRolesList(SubListCreateAttachDetachAPIView): permission_classes = (IsAuthenticated,) def get_queryset(self): - #u = User.objects.get(pk=self.kwargs['pk']) - return Role.visible_roles(self.request.user).filter(members__in=[int(self.kwargs['pk']), ]) + u = get_object_or_404(User, pk=self.kwargs['pk']) + if not self.request.user.can_access(User, 'read', u): + raise PermissionDenied() + return Role.filter_visible_roles(self.request.user, u.roles.all()) def post(self, request, *args, **kwargs): # Forbid implicit role creation here diff --git a/awx/main/access.py b/awx/main/access.py index ae99592afe..ec1ad16317 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -379,12 +379,16 @@ class HostAccess(BaseAccess): def get_queryset(self): inv_qs = Inventory.accessible_objects(self.user, 'read_role') - group_qs = Group.accessible_objects(self.user, 'read_role') - qs = (self.model.objects.filter(inventory=inv_qs) | self.model.objects.filter(groups=group_qs)).distinct() - #qs = qs.select_related('created_by', 'modified_by', 'inventory', - # 'last_job__job_template', - # 'last_job_host_summary__job') - #return qs.prefetch_related('groups').all() + group_qs = Group.accessible_objects(self.user, 'read_role').exclude(inventory__in=inv_qs) + if group_qs.count(): + qs = self.model.objects.filter(Q(inventory__in=inv_qs) | Q(groups__in=group_qs)) + else: + qs = self.model.objects.filter(inventory__in=inv_qs) + + qs = qs.select_related('created_by', 'modified_by', 'inventory', + 'last_job__job_template', + 'last_job_host_summary__job') + qs =qs.prefetch_related('groups').all() return qs def can_read(self, obj): @@ -491,7 +495,7 @@ class InventorySourceAccess(BaseAccess): def get_queryset(self): qs = self.model.objects.all() qs = qs.select_related('created_by', 'modified_by', 'group', 'inventory') - inventory_ids = set(self.user.get_queryset(Inventory).values_list('id', flat=True)) + inventory_ids = self.user.get_queryset(Inventory) return qs.filter(Q(inventory_id__in=inventory_ids) | Q(group__inventory_id__in=inventory_ids)) diff --git a/awx/main/migrations/0008_v300_rbac_changes.py b/awx/main/migrations/0008_v300_rbac_changes.py index 896e81558a..dd112700cf 100644 --- a/awx/main/migrations/0008_v300_rbac_changes.py +++ b/awx/main/migrations/0008_v300_rbac_changes.py @@ -129,6 +129,10 @@ class Migration(migrations.Migration): name='ancestors', field=models.ManyToManyField(related_name='descendents', through='main.RoleAncestorEntry', to='main.Role'), ), + migrations.AlterIndexTogether( + name='role', + index_together=set([('content_type', 'object_id')]), + ), migrations.AlterIndexTogether( name='roleancestorentry', index_together=set([('ancestor', 'content_type_id', 'object_id'), ('ancestor', 'content_type_id', 'role_field'), ('ancestor', 'descendent')]), diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 64e748a20a..d2869fca38 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -8,7 +8,6 @@ import contextlib # Django from django.db import models, transaction, connection -from django.db.models import Q from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ from django.contrib.contenttypes.models import ContentType @@ -106,6 +105,9 @@ class Role(models.Model): app_label = 'main' verbose_name_plural = _('roles') db_table = 'main_rbac_roles' + index_together = [ + ("content_type", "object_id") + ] role_field = models.TextField(null=False) singleton_name = models.TextField(null=True, default=None, db_index=True, unique=True) @@ -348,7 +350,43 @@ class Role(models.Model): @staticmethod def visible_roles(user): - return Role.objects.filter(Q(descendents__in=user.roles.filter()) | Q(ancestors__in=user.roles.filter())) + sql_params = { + 'ancestors_table': Role.ancestors.through._meta.db_table, + 'parents_table': Role.parents.through._meta.db_table, + 'roles_table': Role._meta.db_table, + 'ids': ','.join(str(x) for x in user.roles.values_list('id', flat=True)) + } + + qs = Role.objects.extra( + where = [''' + %(roles_table)s.id IN ( + SELECT descendent_id FROM %(ancestors_table)s WHERE ancestor_id IN (%(ids)s) + UNION + SELECT ancestor_id FROM %(ancestors_table)s WHERE descendent_id IN (%(ids)s) + ) + ''' % sql_params] + ) + return qs + + @staticmethod + def filter_visible_roles(user, roles_qs): + sql_params = { + 'ancestors_table': Role.ancestors.through._meta.db_table, + 'parents_table': Role.parents.through._meta.db_table, + 'roles_table': Role._meta.db_table, + 'ids': ','.join(str(x) for x in user.roles.all().values_list('id', flat=True)) + } + + qs = roles_qs.extra( + where = [''' + EXISTS ( + SELECT 1 FROM + %(ancestors_table)s + WHERE (descendent_id = %(roles_table)s.id AND ancestor_id IN (%(ids)s)) + OR (ancestor_id = %(roles_table)s.id AND descendent_id IN (%(ids)s)) + ) ''' % sql_params] + ) + return qs @staticmethod def singleton(name): diff --git a/awx/main/tests/functional/test_rbac_api.py b/awx/main/tests/functional/test_rbac_api.py index 9c1fd34356..2297868aa6 100644 --- a/awx/main/tests/functional/test_rbac_api.py +++ b/awx/main/tests/functional/test_rbac_api.py @@ -96,6 +96,7 @@ def test_user_view_other_user_roles(organization, inventory, team, get, alice, b 'Users can see roles for other users, but only the roles that that user has access to see as well' organization.member_role.members.add(alice) organization.admin_role.members.add(bob) + organization.member_role.members.add(bob) custom_role = Role.objects.create(name='custom_role-test_user_view_admin_roles_list') organization.member_role.children.add(custom_role) team.member_role.members.add(bob)