From 8887db231b1930e030bc79b296b95f21fa086bb1 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 14 Apr 2016 09:44:20 -0400 Subject: [PATCH] Progress on ripping out RolePermissions --- awx/main/access.py | 1 - awx/main/fields.py | 37 +------ awx/main/models/__init__.py | 1 - awx/main/models/credential.py | 3 - awx/main/models/inventory.py | 12 -- awx/main/models/jobs.py | 3 - awx/main/models/mixins.py | 6 +- awx/main/models/organization.py | 7 -- awx/main/models/projects.py | 4 - awx/main/models/rbac.py | 115 +------------------- awx/main/tests/functional/test_rbac_core.py | 36 ++---- 11 files changed, 16 insertions(+), 209 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 6ea07c318b..3305dc8983 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -18,7 +18,6 @@ from rest_framework.exceptions import ParseError, PermissionDenied from awx.main.utils import * # noqa from awx.main.models import * # noqa from awx.main.models.mixins import ResourceMixin -from awx.main.models.rbac import ALL_PERMISSIONS from awx.api.license import LicenseForbids from awx.main.task_engine import TaskSerializer from awx.main.conf import tower_settings diff --git a/awx/main/fields.py b/awx/main/fields.py index 7e57c29c7d..30bd8e03f3 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -93,10 +93,9 @@ class ImplicitRoleDescriptor(ReverseSingleRelatedObjectDescriptor): class ImplicitRoleField(models.ForeignKey): """Implicitly creates a role entry for a resource""" - def __init__(self, role_name=None, role_description=None, permissions=None, parent_role=None, *args, **kwargs): + def __init__(self, role_name=None, role_description=None, parent_role=None, *args, **kwargs): self.role_name = role_name self.role_description = role_description if role_description else "" - self.permissions = permissions self.parent_role = parent_role kwargs.setdefault('to', 'Role') @@ -108,7 +107,6 @@ class ImplicitRoleField(models.ForeignKey): name, path, args, kwargs = super(ImplicitRoleField, self).deconstruct() kwargs['role_name'] = self.role_name kwargs['role_description'] = self.role_description - kwargs['permissions'] = self.permissions kwargs['parent_role'] = self.parent_role return name, path, args, kwargs @@ -190,40 +188,11 @@ class ImplicitRoleField(models.ForeignKey): ) setattr(instance, self.name, role) - def _patch_role_content_object_and_grant_permissions(self, instance): + def _patch_role_content_object(self, instance): role = getattr(instance, self.name) role.content_object = instance role.save() - if self.permissions is not None: - RolePermission_ = get_current_apps().get_model('main', 'RolePermission') - ContentType = get_current_apps().get_model('contenttypes', "ContentType") - instance_content_type = ContentType.objects.get_for_model(instance) - - permissions = RolePermission_( - created=now(), - modified=now(), - role=role, - content_type=instance_content_type, - object_id=instance.id, - auto_generated=True - ) - - if 'all' in self.permissions and self.permissions['all']: - del self.permissions['all'] - self.permissions['create'] = True - self.permissions['read'] = True - self.permissions['write'] = True - self.permissions['update'] = True - self.permissions['delete'] = True - self.permissions['scm_update'] = True - self.permissions['use'] = True - self.permissions['execute'] = True - - for k,v in self.permissions.items(): - setattr(permissions, k, v) - permissions.save() - def _pre_save(self, instance, *args, **kwargs): for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): implicit_role_field._create_role_instance_if_not_exists(instance) @@ -231,7 +200,7 @@ class ImplicitRoleField(models.ForeignKey): def _post_save(self, instance, created, *args, **kwargs): if created: for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): - implicit_role_field._patch_role_content_object_and_grant_permissions(instance) + implicit_role_field._patch_role_content_object(instance) with batch_role_ancestor_rebuilding(): for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 847f52bb35..fef22b5e2d 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -46,7 +46,6 @@ User.add_to_class('can_access', check_user_access) User.add_to_class('accessible_by', user_accessible_by) User.add_to_class('accessible_objects', user_accessible_objects) User.add_to_class('admin_role', user_admin_role) -User.add_to_class('role_permissions', GenericRelation('main.RolePermission')) @property def user_get_organizations(user): diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index d247154fd7..1b20476c58 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -174,7 +174,6 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ], - permissions = {'all': True} ) auditor_role = ImplicitRoleField( role_name='Credential Auditor', @@ -182,12 +181,10 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ], - permissions = {'read': True} ) usage_role = ImplicitRoleField( role_name='Credential User', role_description='May use this credential, but not read sensitive portions or modify it', - permissions = {'use': True} ) @property diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 2af823885d..90515c2895 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -100,28 +100,23 @@ class Inventory(CommonModel, ResourceMixin): role_name='Inventory Administrator', role_description='May manage this inventory', parent_role='organization.admin_role', - permissions = {'all': True} ) auditor_role = ImplicitRoleField( role_name='Inventory Auditor', role_description='May view but not modify this inventory', parent_role='organization.auditor_role', - permissions = {'read': True} ) updater_role = ImplicitRoleField( role_name='Inventory Updater', role_description='May update the inventory', - permissions = {'read': True, 'update': True} ) usage_role = ImplicitRoleField( role_name='Inventory User', role_description='May use this inventory, but not read sensitive portions or modify it', - permissions = {'use': True} ) executor_role = ImplicitRoleField( role_name='Inventory Executor', role_description='May execute jobs against this inventory', - permissions = {'read': True, 'execute': True} ) def get_absolute_url(self): @@ -525,22 +520,18 @@ class Group(CommonModelNameNotUnique, ResourceMixin): admin_role = ImplicitRoleField( role_name='Inventory Group Administrator', parent_role=['inventory.admin_role', 'parents.admin_role'], - permissions = {'all': True} ) auditor_role = ImplicitRoleField( role_name='Inventory Group Auditor', parent_role=['inventory.auditor_role', 'parents.auditor_role'], - permissions = {'read': True} ) updater_role = ImplicitRoleField( role_name='Inventory Group Updater', parent_role=['inventory.updater_role', 'parents.updater_role'], - permissions = {'read': True, 'write': True, 'create': True, 'use': True}, ) executor_role = ImplicitRoleField( role_name='Inventory Group Executor', parent_role=['inventory.executor_role', 'parents.executor_role'], - permissions = {'read':True, 'execute':True}, ) def __unicode__(self): @@ -1296,21 +1287,18 @@ class CustomInventoryScript(CommonModelNameNotUnique, ResourceMixin): role_name='CustomInventory Administrator', role_description='May manage this inventory', parent_role='organization.admin_role', - permissions = {'all': True} ) member_role = ImplicitRoleField( role_name='CustomInventory Member', role_description='May view but not modify this inventory', parent_role='organization.member_role', - permissions = {'read': True} ) auditor_role = ImplicitRoleField( role_name='CustomInventory Auditor', role_description='May view but not modify this inventory', parent_role='organization.auditor_role', - permissions = {'read': True} ) def get_absolute_url(self): diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 0314a57fb7..d9747f71cf 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -207,18 +207,15 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): role_name='Job Template Administrator', role_description='Full access to all settings', parent_role='project.admin_role', - permissions = {'all': True} ) auditor_role = ImplicitRoleField( role_name='Job Template Auditor', role_description='Read-only access to all settings', parent_role='project.auditor_role', - permissions = {'read': True} ) executor_role = ImplicitRoleField( role_name='Job Template Runner', role_description='May run the job template', - permissions = {'read': True, 'execute': True} ) @classmethod diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index bcf95fa4ac..1396698c28 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -7,8 +7,6 @@ from django.contrib.auth.models import User # noqa # AWX from awx.main.models.rbac import ( - get_user_permissions_on_resource, - get_role_permissions_on_resource, Role, ) @@ -20,10 +18,8 @@ class ResourceMixin(models.Model): class Meta: abstract = True - role_permissions = GenericRelation('main.RolePermission') - @classmethod - def accessible_objects(cls, accessor, permissions): + def accessible_objects(cls, accessor, role_name): ''' Use instead of `MyModel.objects` when you want to only consider resources that a user has specific permissions for. For example: diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 615a9104fe..f497d2a120 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -19,7 +19,6 @@ from django.utils.translation import ugettext_lazy as _ from awx.main.fields import AutoOneToOneField, ImplicitRoleField from awx.main.models.base import * # noqa from awx.main.models.rbac import ( - ALL_PERMISSIONS, ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ROLE_SINGLETON_SYSTEM_AUDITOR, ) @@ -57,19 +56,16 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin): role_name='Organization Administrator', role_description='May manage all aspects of this organization', parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, - permissions = ALL_PERMISSIONS, ) auditor_role = ImplicitRoleField( role_name='Organization Auditor', role_description='May read all settings associated with this organization', parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, - permissions = {'read': True} ) member_role = ImplicitRoleField( role_name='Organization Member', role_description='A member of this organization', parent_role='admin_role', - permissions = {'read': True} ) @@ -112,19 +108,16 @@ class Team(CommonModelNameNotUnique, ResourceMixin): role_name='Team Administrator', role_description='May manage this team', parent_role='organization.admin_role', - permissions = ALL_PERMISSIONS, ) auditor_role = ImplicitRoleField( role_name='Team Auditor', role_description='May read all settings associated with this team', parent_role='organization.auditor_role', - permissions = {'read': True} ) member_role = ImplicitRoleField( role_name='Team Member', role_description='A member of this team', parent_role='admin_role', - permissions = {'read':True}, ) def get_absolute_url(self): diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index e5d1d58d19..0f023ab56b 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -227,7 +227,6 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin): 'organization.admin_role', 'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ], - permissions = {'all': True} ) auditor_role = ImplicitRoleField( role_name='Project Auditor', @@ -236,18 +235,15 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin): 'organization.auditor_role', 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ], - permissions = {'read': True} ) member_role = ImplicitRoleField( role_name='Project Member', role_description='Implies membership within this project', - permissions = {'read': True} ) scm_update_role = ImplicitRoleField( role_name='Project Updater', role_description='May update this project from the source control management system', parent_role='admin_role', - permissions = {'scm_update': True} ) @classmethod diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 91e055f6eb..2d7b5d279e 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -21,10 +21,7 @@ from awx.main.models.base import * # noqa __all__ = [ 'Role', - 'RolePermission', 'batch_role_ancestor_rebuilding', - 'get_user_permissions_on_resource', - 'get_role_permissions_on_resource', 'ROLE_SINGLETON_SYSTEM_ADMINISTRATOR', 'ROLE_SINGLETON_SYSTEM_AUDITOR', ] @@ -34,10 +31,6 @@ logger = logging.getLogger('awx.main.models.rbac') ROLE_SINGLETON_SYSTEM_ADMINISTRATOR='System Administrator' ROLE_SINGLETON_SYSTEM_AUDITOR='System Auditor' -ALL_PERMISSIONS = {'create': True, 'read': True, 'update': True, 'delete': True, - 'write': True, 'scm_update': True, 'use': True, 'execute': True} - - tls = threading.local() # thread local storage @contextlib.contextmanager @@ -97,6 +90,8 @@ class Role(CommonModelNameNotUnique): def get_absolute_url(self): return reverse('api:role_detail', args=(self.pk,)) + def __contains__(self, user): + return self.ancestors.filter(members=user).exists() def rebuild_role_ancestor_list(self): ''' @@ -143,109 +138,3 @@ class Role(CommonModelNameNotUnique): def is_ancestor_of(self, role): return role.ancestors.filter(id=self.id).exists() - -class RolePermission(CreatedModifiedModel): - ''' - Defines the permissions a role has - ''' - - class Meta: - app_label = 'main' - verbose_name_plural = _('permissions') - db_table = 'main_rbac_permissions' - index_together = [ - ('content_type', 'object_id') - ] - - role = models.ForeignKey( - Role, - null=False, - on_delete=models.CASCADE, - related_name='permissions', - ) - content_type = models.ForeignKey(ContentType, null=False, default=None) - object_id = models.PositiveIntegerField(null=False, default=None) - resource = GenericForeignKey('content_type', 'object_id') - auto_generated = models.BooleanField(default=False) - - create = models.IntegerField(default = 0) - read = models.IntegerField(default = 0) - write = models.IntegerField(default = 0) - delete = models.IntegerField(default = 0) - update = models.IntegerField(default = 0) - execute = models.IntegerField(default = 0) - scm_update = models.IntegerField(default = 0) - use = models.IntegerField(default = 0) - - - -def get_user_permissions_on_resource(resource, user): - ''' - Returns a dict (or None) of the permissions a user has for a given - 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 - user. - - In example, if a user has been granted read access through a permission - on one role and write access through a permission on a separate role, - the returned dict will denote that the user has both read and write - access. - ''' - - if type(user) == User: - roles = user.roles.all() - else: - accessor_type = ContentType.objects.get_for_model(user) - roles = Role.objects.filter(content_type__pk=accessor_type.id, - object_id=user.id) - - qs = RolePermission.objects.filter( - content_type=ContentType.objects.get_for_model(resource), - object_id=resource.id, - role__ancestors__in=roles, - ) - - res = qs = qs.aggregate( - create = Max('create'), - read = Max('read'), - write = Max('write'), - update = Max('update'), - delete = Max('delete'), - scm_update = Max('scm_update'), - execute = Max('execute'), - use = Max('use') - ) - if res['read'] is None: - return None - return res - -def get_role_permissions_on_resource(resource, role): - ''' - Returns a dict (or None) of the permissions a role has for a given - resource. - - Note: Each field in the dict is the `or` of all respective permissions - that have been granted to either the role or any descendents of that role. - ''' - - qs = RolePermission.objects.filter( - content_type=ContentType.objects.get_for_model(resource), - object_id=resource.id, - role__ancestors=role - ) - - res = qs = qs.aggregate( - create = Max('create'), - read = Max('read'), - write = Max('write'), - update = Max('update'), - delete = Max('delete'), - scm_update = Max('scm_update'), - execute = Max('execute'), - use = Max('use') - ) - if res['read'] is None: - return None - return res diff --git a/awx/main/tests/functional/test_rbac_core.py b/awx/main/tests/functional/test_rbac_core.py index 2ad1250f81..f381ddd030 100644 --- a/awx/main/tests/functional/test_rbac_core.py +++ b/awx/main/tests/functional/test_rbac_core.py @@ -2,7 +2,6 @@ import pytest from awx.main.models import ( Role, - RolePermission, Organization, Project, ) @@ -14,23 +13,21 @@ def test_auto_inheritance_by_children(organization, alice): B = Role.objects.create(name='B') A.members.add(alice) - - - assert organization.accessible_by(alice, {'read': True}) is False - assert Organization.accessible_objects(alice, {'read': True}).count() == 0 + assert alice not in organization.admin_role + assert Organization.accessible_objects(alice, 'admin_role').count() == 0 A.children.add(B) - assert organization.accessible_by(alice, {'read': True}) is False - assert Organization.accessible_objects(alice, {'read': True}).count() == 0 + assert alice not in organization.admin_role + assert Organization.accessible_objects(alice, 'admin_role').count() == 0 A.children.add(organization.admin_role) - assert organization.accessible_by(alice, {'read': True}) is True - assert Organization.accessible_objects(alice, {'read': True}).count() == 1 + assert alice in organization.admin_role + assert Organization.accessible_objects(alice, 'admin_role').count() == 1 A.children.remove(organization.admin_role) - assert organization.accessible_by(alice, {'read': True}) is False + assert alice not in organization.admin_role B.children.add(organization.admin_role) - assert organization.accessible_by(alice, {'read': True}) is True + assert alice in organization.admin_role B.children.remove(organization.admin_role) - assert organization.accessible_by(alice, {'read': True}) is False - assert Organization.accessible_objects(alice, {'read': True}).count() == 0 + assert alice not in organization.admin_role + assert Organization.accessible_objects(alice, 'admin_role').count() == 0 # We've had the case where our pre/post save init handlers in our field descriptors # end up creating a ton of role objects because of various not-so-obvious issues @@ -56,19 +53,6 @@ def test_auto_inheritance_by_parents(organization, alice): assert organization.accessible_by(alice, {'read': True}) is False -@pytest.mark.django_db -def test_permission_union(organization, alice): - A = Role.objects.create(name='A') - A.members.add(alice) - B = Role.objects.create(name='B') - B.members.add(alice) - - assert organization.accessible_by(alice, {'read': True, 'write': True}) is False - RolePermission.objects.create(role=A, resource=organization, read=True) - assert organization.accessible_by(alice, {'read': True, 'write': True}) is False - RolePermission.objects.create(role=A, resource=organization, write=True) - assert organization.accessible_by(alice, {'read': True, 'write': True}) is True - @pytest.mark.django_db def test_accessible_objects(organization, alice, bob):