ORMified RBAC classes; Added GenericForeignKey backref for convenience

The RoleHierarchy table has been eliminated in favor of just using
a ManyToMany map, which is what we should have been using all along.

ORMifications still need improvement, in particular filtering on
ResourceMixin.accessible_by should reduce permission calculation
overhead, but with the current implemenation this is not true.
ResourceMixin.get_permission performs adequately but not as good
as it can yet.

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,6 +32,48 @@ class ResourceMixin(models.Model):
         `myresource.get_permissions(user)`.
         '''
 
+        # TODO: Clean this up once we have settled on an optimal implementation
+
+        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:
+                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
+
+        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 = ''
@@ -55,12 +100,13 @@ class ResourceMixin(models.Model):
                               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)
+                                     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.role_id)
+                                     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
                         )
@@ -75,7 +121,7 @@ class ResourceMixin(models.Model):
                         'where_clause'           : where_clause,
                         'rbac_role'              : Role._meta.db_table,
                         'rbac_permission'        : RolePermission._meta.db_table,
-                    'rbac_role_hierachy'     : RoleHierarchy._meta.db_table
+                        'rbac_role_hierachy'     : 'main_rbac_roles_ancestors'
                     }
                 ]
             )
@@ -96,6 +142,39 @@ class ResourceMixin(models.Model):
         access.
         '''
 
+        # TODO: Clean this up once we have settled on an optimal implementation
+
+        if True:
+            # This works well enough at scale, but is about 5x slower than the
+            # raw sql variant, further optimization is desired.
+
+            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(
@@ -111,11 +190,11 @@ class ResourceMixin(models.Model):
                               MAX("use")        as "use"
 
                           FROM %(rbac_permission)s
-                      LEFT JOIN %(rbac_role_hierachy)s 
+                          LEFT JOIN main_rbac_roles_ancestors
-                           ON (%(rbac_permission)s.role_id = %(rbac_role_hierachy)s.role_id)
+                               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 = %(rbac_role_hierachy)s.ancestor_id
+                                     %(rbac_role)s_members.role_id = main_rbac_roles_ancestors.to_role_id
                                      AND %(rbac_role)s_members.user_id = %(user_id)d
                                   )
 
@@ -128,7 +207,7 @@ class ResourceMixin(models.Model):
                         'resource_id': self.resource.id,
                         'rbac_role': Role._meta.db_table,
                         'rbac_permission': RolePermission._meta.db_table,
-                    'rbac_role_hierachy': RoleHierarchy._meta.db_table
+                        #'rbac_role_hierachy': RoleHierarchy._meta.db_table
                     }
                 )
                 row = cursor.fetchone()
@@ -136,6 +215,7 @@ class ResourceMixin(models.Model):
                     return dict(zip([x[0] for x in cursor.description], row))
             return None
 
+
     def accessible_by(self, user, permissions):
         '''
         Returns true if the user has all of the specified permissions

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):
+    def rebuild_role_ancestor_list(self):
-        'Rebuilds the associated entries in the RoleHierarchy model'
+        '''
+        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
+        You should never need to call this. Signal handlers should be calling
-        # parent's cached hierarchy being correct)
+        this method when the role hierachy changes automatically.
-        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)])
+        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)
-
+        if None in actual_ancestors:
-        # Compute what we have stored
+            actual_ancestors.remove(None)
-        stored_ancestors = set([r.ancestor.id for r in RoleHierarchy.objects.filter(role__id=self.id)])
+        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 - stored_ancestors:
-            for id in actual_ancestors:
+                self.ancestors.add(Role.objects.get(id=id))
-                rh = RoleHierarchy(role=self, ancestor=Role.objects.get(id=id))
+            for id in stored_ancestors - actual_ancestors:
-                rh.save()
+                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
+        return role.ancestors.filter(id=self.id).exists()
-
-
-
-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)
 
 
 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):
     '''