diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 8ce444bbb4..3b7b5eb8d4 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -1,11 +1,14 @@ # Django from django.db import models from django.db import connection +from django.db.models.aggregates import Max +from django.contrib.contenttypes.models import ContentType # AWX -from awx.main.models.rbac import RolePermission, Role, RoleHierarchy +from awx.main.models.rbac import RolePermission, Role, Resource from awx.main.fields import ImplicitResourceField + __all__ = 'ResourceMixin' class ResourceMixin(models.Model): @@ -29,62 +32,105 @@ class ResourceMixin(models.Model): `myresource.get_permissions(user)`. ''' - aggregate_where_clause = '' - aggregates = '' - group_clause = '' - where_clause = '' + # TODO: Clean this up once we have settled on an optimal implementation - if len(permissions) > 1: - group_clause = 'GROUP BY %s.resource_id' % RolePermission._meta.db_table + if False: + # This query does not work, but it is not apparent to me at this + # time why it does not. The intent is to be able to return a query + # set that does not involve a subselect, with the hope that this + # will perform better than our subselect method. I'm leaving it + # here for now so that I can revisit it with fresh eyes and + # hopefully find the issue. + # + # If someone else comes along and fixes or eliminates this, please + # tag me or let me know! - anoek 2016-02-11 + + qs = cls.objects for perm in permissions: - if not aggregate_where_clause: - aggregate_where_clause = 'WHERE ' - else: - aggregate_where_clause += ' AND ' - aggregate_where_clause += '"%s" = %d' % (perm, int(permissions[perm])) - aggregates += ', MAX("%s") as "%s"' % (perm, perm) - if len(permissions) == 1: - perm = list(permissions.keys())[0] - where_clause = 'AND "%s" = %d' % (perm, int(permissions[perm])) + kw = {'max_' + perm: Max('resource__permissions__' + perm)} + qs = qs.annotate(**kw) + kw = {'max_' + perm: int(permissions[perm])} + qs = qs.filter(**kw) + qs = qs.filter(resource__permissions__role__ancestors__members=user) + return qs - return cls.objects.extra( - where=[ - ''' - %(table_name)s.resource_id in ( - SELECT resource_id FROM ( - SELECT %(rbac_permission)s.resource_id %(aggregates)s - FROM %(rbac_role)s_members - LEFT JOIN %(rbac_role_hierachy)s - ON (%(rbac_role_hierachy)s.ancestor_id = %(rbac_role)s_members.role_id) - LEFT JOIN %(rbac_permission)s - ON (%(rbac_permission)s.role_id = %(rbac_role_hierachy)s.role_id) - WHERE %(rbac_role)s_members.user_id=%(user_id)d - %(where_clause)s - %(group_clause)s - ) summarized_permissions - %(aggregate_where_clause)s - ) - ''' - % - { - 'table_name' : cls._meta.db_table, - 'aggregates' : aggregates, - 'user_id' : user.id, - 'aggregate_where_clause' : aggregate_where_clause, - 'group_clause' : group_clause, - 'where_clause' : where_clause, - 'rbac_role' : Role._meta.db_table, - 'rbac_permission' : RolePermission._meta.db_table, - 'rbac_role_hierachy' : RoleHierarchy._meta.db_table - } - ] - ) + if True: + # Begin working code.. this results in a subselect which I'm not too + # thrilled about, but performs reasonably ok + + qs = Resource.objects.filter( + content_type=ContentType.objects.get_for_model(cls), + permissions__role__ancestors__members=user + ) + for perm in permissions: + kw = {'max_' + perm: Max('permissions__' + perm)} + qs = qs.annotate(**kw) + kw = {'max_' + perm: int(permissions[perm])} + qs = qs.filter(**kw) + + return cls.objects.filter(resource__in=qs) + + if False: + # This works and remains the most performant implementation. Keeping it here + # until we can dethrone it with a proper ORM implementation + + aggregate_where_clause = '' + aggregates = '' + group_clause = '' + where_clause = '' + + if len(permissions) > 1: + group_clause = 'GROUP BY %s.resource_id' % RolePermission._meta.db_table + for perm in permissions: + if not aggregate_where_clause: + aggregate_where_clause = 'WHERE ' + else: + aggregate_where_clause += ' AND ' + aggregate_where_clause += '"%s" = %d' % (perm, int(permissions[perm])) + aggregates += ', MAX("%s") as "%s"' % (perm, perm) + if len(permissions) == 1: + perm = list(permissions.keys())[0] + where_clause = 'AND "%s" = %d' % (perm, int(permissions[perm])) + + return cls.objects.extra( + where=[ + ''' + %(table_name)s.resource_id in ( + SELECT resource_id FROM ( + SELECT %(rbac_permission)s.resource_id %(aggregates)s + FROM %(rbac_role)s_members + LEFT JOIN %(rbac_role_hierachy)s + ON (%(rbac_role_hierachy)s.to_role_id = %(rbac_role)s_members.role_id) + LEFT JOIN %(rbac_permission)s + ON (%(rbac_permission)s.role_id = %(rbac_role_hierachy)s.from_role_id) + WHERE %(rbac_role)s_members.user_id=%(user_id)d + %(where_clause)s + %(group_clause)s + order by resource_id + ) summarized_permissions + %(aggregate_where_clause)s + ) + ''' + % + { + 'table_name' : cls._meta.db_table, + 'aggregates' : aggregates, + 'user_id' : user.id, + 'aggregate_where_clause' : aggregate_where_clause, + 'group_clause' : group_clause, + 'where_clause' : where_clause, + 'rbac_role' : Role._meta.db_table, + 'rbac_permission' : RolePermission._meta.db_table, + 'rbac_role_hierachy' : 'main_rbac_roles_ancestors' + } + ] + ) def get_permissions(self, user): ''' Returns a dict (or None) of the permissions a user has for a given - resource. + resource. Note: Each field in the dict is the `or` of all respective permissions that have been granted to the roles that are applicable for the given @@ -96,45 +142,79 @@ class ResourceMixin(models.Model): access. ''' + # TODO: Clean this up once we have settled on an optimal implementation - with connection.cursor() as cursor: - cursor.execute( - ''' - SELECT - MAX("create") as "create", - MAX("read") as "read", - MAX("write") as "write", - MAX("update") as "update", - MAX("delete") as "delete", - MAX("scm_update") as "scm_update", - MAX("execute") as "execute", - MAX("use") as "use" + if True: + # This works well enough at scale, but is about 5x slower than the + # raw sql variant, further optimization is desired. - FROM %(rbac_permission)s - LEFT JOIN %(rbac_role_hierachy)s - ON (%(rbac_permission)s.role_id = %(rbac_role_hierachy)s.role_id) - INNER JOIN %(rbac_role)s_members - ON ( - %(rbac_role)s_members.role_id = %(rbac_role_hierachy)s.ancestor_id - AND %(rbac_role)s_members.user_id = %(user_id)d - ) + qs = user.__class__.objects.filter(id=user.id, roles__descendents__permissions__resource=self.resource) + + qs = qs.annotate(max_create = Max('roles__descendents__permissions__create')) + qs = qs.annotate(max_read = Max('roles__descendents__permissions__read')) + qs = qs.annotate(max_write = Max('roles__descendents__permissions__write')) + qs = qs.annotate(max_update = Max('roles__descendents__permissions__update')) + qs = qs.annotate(max_delete = Max('roles__descendents__permissions__delete')) + qs = qs.annotate(max_scm_update = Max('roles__descendents__permissions__scm_update')) + qs = qs.annotate(max_execute = Max('roles__descendents__permissions__execute')) + qs = qs.annotate(max_use = Max('roles__descendents__permissions__use')) + + qs = qs.values('max_create', 'max_read', 'max_write', 'max_update', + 'max_delete', 'max_scm_update', 'max_execute', 'max_use') + + #print('###############') + #print(qs.query) + #print('###############') + + res = qs.all() + if len(res): + return {k[4:]:v for k,v in res[0].items()} + return None + + + if False: + # This works and remains the most performant implementation. Keeping it here + # until we can dethrone it with a proper ORM implementation + + with connection.cursor() as cursor: + cursor.execute( + ''' + SELECT + MAX("create") as "create", + MAX("read") as "read", + MAX("write") as "write", + MAX("update") as "update", + MAX("delete") as "delete", + MAX("scm_update") as "scm_update", + MAX("execute") as "execute", + MAX("use") as "use" + + FROM %(rbac_permission)s + LEFT JOIN main_rbac_roles_ancestors + ON (%(rbac_permission)s.role_id = main_rbac_roles_ancestors.from_role_id) + INNER JOIN %(rbac_role)s_members + ON ( + %(rbac_role)s_members.role_id = main_rbac_roles_ancestors.to_role_id + AND %(rbac_role)s_members.user_id = %(user_id)d + ) + + WHERE %(rbac_permission)s.resource_id=%(resource_id)s + GROUP BY %(rbac_role)s_members.user_id + ''' + % + { + 'user_id': user.id, + 'resource_id': self.resource.id, + 'rbac_role': Role._meta.db_table, + 'rbac_permission': RolePermission._meta.db_table, + #'rbac_role_hierachy': RoleHierarchy._meta.db_table + } + ) + row = cursor.fetchone() + if row: + return dict(zip([x[0] for x in cursor.description], row)) + return None - WHERE %(rbac_permission)s.resource_id=%(resource_id)s - GROUP BY %(rbac_role)s_members.user_id - ''' - % - { - 'user_id': user.id, - 'resource_id': self.resource.id, - 'rbac_role': Role._meta.db_table, - 'rbac_permission': RolePermission._meta.db_table, - 'rbac_role_hierachy': RoleHierarchy._meta.db_table - } - ) - row = cursor.fetchone() - if row: - return dict(zip([x[0] for x in cursor.description], row)) - return None def accessible_by(self, user, permissions): ''' diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 1268f68443..1a5c189892 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -7,11 +7,13 @@ import logging # Django from django.db import models from django.utils.translation import ugettext_lazy as _ +from django.contrib.contenttypes.models import ContentType +from django.contrib.contenttypes.fields import GenericForeignKey # AWX from awx.main.models.base import * # noqa -__all__ = ['Role', 'RolePermission', 'Resource', 'RoleHierarchy'] +__all__ = ['Role', 'RolePermission', 'Resource'] logger = logging.getLogger('awx.main.models.rbac') @@ -28,32 +30,41 @@ class Role(CommonModelNameNotUnique): singleton_name = models.TextField(null=True, default=None, db_index=True, unique=True) parents = models.ManyToManyField('Role', related_name='children') + ancestors = models.ManyToManyField('Role', related_name='descendents') # auto-generated by `rebuild_role_ancestor_list` members = models.ManyToManyField('auth.User', related_name='roles') + content_type = models.ForeignKey(ContentType, null=True, default=None) + object_id = models.PositiveIntegerField(null=True, default=None) + content_object = GenericForeignKey('content_type', 'object_id') def save(self, *args, **kwargs): super(Role, self).save(*args, **kwargs) - self.rebuild_role_hierarchy_cache() + self.rebuild_role_ancestor_list() - def rebuild_role_hierarchy_cache(self): - 'Rebuilds the associated entries in the RoleHierarchy model' + def rebuild_role_ancestor_list(self): + ''' + Updates our `ancestors` map to accurately reflect all of the ancestors for a role - # Compute what our hierarchy should be. (Note: this depends on our - # parent's cached hierarchy being correct) - parent_ids = set([parent.id for parent in self.parents.all()]) - actual_ancestors = set([r.ancestor.id for r in RoleHierarchy.objects.filter(role__id__in=parent_ids)]) + You should never need to call this. Signal handlers should be calling + this method when the role hierachy changes automatically. + + Note that this method relies on any parents' ancestor list being correct. + ''' + + actual_ancestors = set(Role.objects.filter(id=self.id).values_list('parents__ancestors__id', flat=True)) actual_ancestors.add(self.id) - - # Compute what we have stored - stored_ancestors = set([r.ancestor.id for r in RoleHierarchy.objects.filter(role__id=self.id)]) + if None in actual_ancestors: + actual_ancestors.remove(None) + stored_ancestors = set(self.ancestors.all().values_list('id', flat=True)) # If it differs, update, and then update all of our children if actual_ancestors != stored_ancestors: - RoleHierarchy.objects.filter(role__id=self.id).delete() - for id in actual_ancestors: - rh = RoleHierarchy(role=self, ancestor=Role.objects.get(id=id)) - rh.save() + for id in actual_ancestors - stored_ancestors: + self.ancestors.add(Role.objects.get(id=id)) + for id in stored_ancestors - actual_ancestors: + self.ancestors.remove(Role.objects.get(id=id)) + for child in self.children.all(): - child.rebuild_role_hierarchy_cache() + child.rebuild_role_ancestor_list() def grant(self, resource, permissions): # take either the raw Resource or something that includes the ResourceMixin @@ -85,22 +96,7 @@ class Role(CommonModelNameNotUnique): return ret def is_ancestor_of(self, role): - return RoleHierarchy.objects.filter(role_id=role.id, ancestor_id=self.id).count() > 0 - - - -class RoleHierarchy(CreatedModifiedModel): - ''' - Stores a flattened relation map of all roles in the system for easy joining - ''' - - class Meta: - app_label = 'main' - verbose_name_plural = _('role_ancestors') - db_table = 'main_rbac_role_hierarchy' - - role = models.ForeignKey('Role', related_name='+', on_delete=models.CASCADE) - ancestor = models.ForeignKey('Role', related_name='+', on_delete=models.CASCADE) + return role.ancestors.filter(id=self.id).exists() class Resource(CommonModelNameNotUnique): @@ -113,6 +109,10 @@ class Resource(CommonModelNameNotUnique): verbose_name_plural = _('resources') db_table = 'main_rbac_resources' + content_type = models.ForeignKey(ContentType, null=True, default=None) + object_id = models.PositiveIntegerField(null=True, default=None) + content_object = GenericForeignKey('content_type', 'object_id') + class RolePermission(CreatedModifiedModel): '''