From 280993a15dc89e3d160e18b16c92057e1ca88b3d Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Wed, 20 Apr 2016 22:38:32 -0400 Subject: [PATCH] Dropped stored role name/description and other superflous fields For name and description, we'll derive these from the role_field and content type, which is much better for lots of reasons (eg changing text the future). Also ditched the rest of the fields comming from the standard common base model, we didn't use them and they cost several indexes on the table. --- awx/main/fields.py | 17 +---- awx/main/migrations/_rbac.py | 14 ++-- awx/main/models/credential.py | 8 --- awx/main/models/inventory.py | 31 --------- awx/main/models/jobs.py | 8 --- awx/main/models/organization.py | 19 +----- awx/main/models/projects.py | 10 --- awx/main/models/rbac.py | 64 ++++++++++++++++--- awx/main/signals.py | 3 +- .../api/test_resource_access_lists.py | 3 +- awx/main/tests/functional/conftest.py | 6 -- awx/main/tests/functional/test_rbac_api.py | 6 +- awx/main/tests/functional/test_rbac_core.py | 28 ++++---- .../tests/functional/test_rbac_project.py | 2 +- awx/main/tests/functional/test_rbac_user.py | 2 +- 15 files changed, 88 insertions(+), 133 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index e116299bcb..568126c536 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -92,9 +92,7 @@ class ImplicitRoleDescriptor(ReverseSingleRelatedObjectDescriptor): class ImplicitRoleField(models.ForeignKey): """Implicitly creates a role entry for a resource""" - 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 "" + def __init__(self, parent_role=None, *args, **kwargs): self.parent_role = parent_role kwargs.setdefault('to', 'Role') @@ -104,8 +102,6 @@ class ImplicitRoleField(models.ForeignKey): def deconstruct(self): name, path, args, kwargs = super(ImplicitRoleField, self).deconstruct() - kwargs['role_name'] = self.role_name - kwargs['role_description'] = self.role_description kwargs['parent_role'] = self.parent_role return name, path, args, kwargs @@ -190,11 +186,7 @@ class ImplicitRoleField(models.ForeignKey): if cur_role is None: missing_roles.append( Role_( - created=now(), - modified=now(), role_field=implicit_role_field.name, - name=implicit_role_field.role_name, - description=implicit_role_field.role_description, content_type_id=ct_id, object_id=instance.id ) @@ -247,12 +239,7 @@ class ImplicitRoleField(models.ForeignKey): if qs.count() >= 1: role = qs[0] else: - role = Role_.objects.create(created=now(), - modified=now(), - role_field=path, - singleton_name=singleton_name, - name=singleton_name, - description=singleton_name) + role = Role_.objects.create(singleton_name=singleton_name, role_field=singleton_name) parents = [role.id] else: parents = resolve_role_field(instance, path) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 0d7aa3ecb7..8710f8c772 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -44,9 +44,7 @@ def migrate_users(apps, schema_editor): logger.info(smart_text(u"found existing role for user: {}".format(user.username))) except Role.DoesNotExist: role = Role.objects.create( - created=now(), - modified=now(), - singleton_name = smart_text(u'{}-admin_role'.format(user.username)), + role_field='admin_role', content_type = user_content_type, object_id = user.id ) @@ -54,14 +52,12 @@ def migrate_users(apps, schema_editor): logger.info(smart_text(u"migrating to new role for user: {}".format(user.username))) if user.is_superuser: - if Role.objects.filter(singleton_name='System Administrator').exists(): - sa_role = Role.objects.get(singleton_name='System Administrator') + if Role.objects.filter(singleton_name='system_administrator').exists(): + sa_role = Role.objects.get(singleton_name='system_administrator') else: sa_role = Role.objects.create( - created=now(), - modified=now(), - singleton_name='System Administrator', - name='System Administrator' + singleton_name='system_administrator', + role_field='system_administrator' ) sa_role.members.add(user) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index d47153285d..11b2a31e87 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -204,27 +204,19 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): help_text=_('Tenant identifier for this credential'), ) owner_role = ImplicitRoleField( - role_name='Credential Owner', - role_description='Owner of the credential', parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ], ) auditor_role = ImplicitRoleField( - role_name='Credential Auditor', - role_description='Auditor of the credential', parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ], ) use_role = ImplicitRoleField( - role_name='Credential User', - role_description='May use this credential, but not read sensitive portions or modify it', parent_role=['owner_role'] ) read_role = ImplicitRoleField( - role_name='Credential REad', - role_description='May read this credential', parent_role=[ 'use_role', 'auditor_role', 'owner_role' ], diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index d192825ec7..ac9a8840cf 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -97,39 +97,25 @@ class Inventory(CommonModel, ResourceMixin): help_text=_('Number of external inventory sources in this inventory with failures.'), ) admin_role = ImplicitRoleField( - role_name='Inventory Administrator', - role_description='May manage this inventory', parent_role='organization.admin_role', ) auditor_role = ImplicitRoleField( - role_name='Inventory Auditor', - role_description='May view but not modify this inventory', parent_role='organization.auditor_role', ) update_role = ImplicitRoleField( - role_name='Inventory Updater', - role_description='May update the inventory', parent_role=['admin_role'], ) use_role = ImplicitRoleField( - role_name='Inventory User', - role_description='May use this inventory, but not read sensitive portions or modify it', parent_role=['admin_role'], ) adhoc_role = ImplicitRoleField( - role_name='Inventory Ad Hoc', - role_description='May execute ad hoc commands against this inventory', parent_role=['admin_role'], ) execute_role = ImplicitRoleField( - role_name='Inventory Executor', - role_description='May execute jobs against this inventory', parent_role='adhoc_role', ) read_role = ImplicitRoleField( - role_name='Read', parent_role=['auditor_role', 'execute_role', 'update_role', 'use_role', 'admin_role'], - role_description='May view this inventory', ) def get_absolute_url(self): @@ -531,28 +517,21 @@ class Group(CommonModelNameNotUnique, ResourceMixin): help_text=_('Inventory source(s) that created or modified this group.'), ) admin_role = ImplicitRoleField( - role_name='Inventory Group Administrator', parent_role=['inventory.admin_role', 'parents.admin_role'], ) auditor_role = ImplicitRoleField( - role_name='Inventory Group Auditor', parent_role=['inventory.auditor_role', 'parents.auditor_role'], ) update_role = ImplicitRoleField( - role_name='Inventory Group Updater', parent_role=['inventory.update_role', 'parents.update_role', 'admin_role'], ) adhoc_role = ImplicitRoleField( - role_name='Inventory Ad Hoc', parent_role=['inventory.adhoc_role', 'parents.adhoc_role', 'admin_role'], - role_description='May execute ad hoc commands against this inventory', ) execute_role = ImplicitRoleField( - role_name='Inventory Group Executor', parent_role=['inventory.execute_role', 'parents.execute_role', 'adhoc_role'], ) read_role = ImplicitRoleField( - role_name='Inventory Group Executor', parent_role=['execute_role', 'update_role', 'auditor_role', 'admin_role'], ) @@ -1321,25 +1300,15 @@ class CustomInventoryScript(CommonModelNameNotUnique, ResourceMixin): ) admin_role = ImplicitRoleField( - role_name='CustomInventory Administrator', - role_description='May manage this inventory', parent_role='organization.admin_role', ) - member_role = ImplicitRoleField( - role_name='CustomInventory Member', - role_description='May view but not modify this inventory', parent_role='organization.member_role', ) - auditor_role = ImplicitRoleField( - role_name='CustomInventory Auditor', - role_description='May view but not modify this inventory', parent_role='organization.auditor_role', ) read_role = ImplicitRoleField( - role_name='CustomInventory Read', - role_description='May view but not modify this inventory', parent_role=['auditor_role', 'member_role', 'admin_role'], ) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index c48007e24a..c64b1e1f8b 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -226,23 +226,15 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): default={}, ) admin_role = ImplicitRoleField( - role_name='Job Template Administrator', - role_description='Full access to all settings', parent_role=[('project.admin_role', 'inventory.admin_role')] ) auditor_role = ImplicitRoleField( - role_name='Job Template Auditor', - role_description='Read-only access to all settings', parent_role=[('project.auditor_role', 'inventory.auditor_role')] ) execute_role = ImplicitRoleField( - role_name='Job Template Runner', - role_description='May run the job template', parent_role=['admin_role'], ) read_role = ImplicitRoleField( - role_name='Job Template Runner', - role_description='May run the job template', parent_role=['execute_role', 'auditor_role', 'admin_role'], ) diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 571f9117ab..3d8a06f446 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -53,23 +53,15 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin): related_name='deprecated_organizations', ) admin_role = ImplicitRoleField( - role_name='Organization Administrator', - role_description='May manage all aspects of this organization', parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ) auditor_role = ImplicitRoleField( - role_name='Organization Auditor', - role_description='May read all settings associated with this organization', parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ) member_role = ImplicitRoleField( - role_name='Organization Member', - role_description='A member of this organization', parent_role='admin_role', ) read_role = ImplicitRoleField( - role_name='Organization Read Access', - role_description='Read an organization', parent_role=['member_role', 'auditor_role'], ) @@ -110,22 +102,13 @@ class Team(CommonModelNameNotUnique, ResourceMixin): related_name='deprecated_teams', ) admin_role = ImplicitRoleField( - role_name='Team Administrator', - role_description='May manage this team', parent_role='organization.admin_role', ) auditor_role = ImplicitRoleField( - role_name='Team Auditor', - role_description='May read all settings associated with this team', parent_role='organization.auditor_role', ) - member_role = ImplicitRoleField( - role_name='Team Member', - role_description='A member of this team', - ) + member_role = ImplicitRoleField() read_role = ImplicitRoleField( - role_name='Read', - role_description='Can view this team', parent_role=['admin_role', 'auditor_role', 'member_role'], ) diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index 12357fca2e..41145821f4 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -221,34 +221,24 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin): blank=True, ) admin_role = ImplicitRoleField( - role_name='Project Administrator', - role_description='May manage this project', parent_role=[ 'organization.admin_role', 'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ], ) auditor_role = ImplicitRoleField( - role_name='Project Auditor', - role_description='May read all settings associated with this project', parent_role=[ 'organization.auditor_role', 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, ], ) member_role = ImplicitRoleField( - role_name='Project Member', - role_description='Implies membership within this project', parent_role='admin_role', ) scm_update_role = ImplicitRoleField( - role_name='Project Updater', - role_description='May update this project from the source control management system', parent_role='admin_role', ) read_role = ImplicitRoleField( - role_name='Project Read Access', - role_description='Read access to this project', parent_role=['member_role', 'auditor_role', 'scm_update_role'], ) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index f757dc580e..c2cdca2f92 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -29,8 +29,39 @@ __all__ = [ logger = logging.getLogger('awx.main.models.rbac') -ROLE_SINGLETON_SYSTEM_ADMINISTRATOR='System Administrator' -ROLE_SINGLETON_SYSTEM_AUDITOR='System Auditor' +ROLE_SINGLETON_SYSTEM_ADMINISTRATOR='system_administrator' +ROLE_SINGLETON_SYSTEM_AUDITOR='system_auditor' + +role_names = { + 'system_administrator' : 'System Administrator', + 'system_auditor' : 'System Auditor', + 'adhoc_role' : 'Ad Hoc', + 'admin_role' : 'Admin', + 'auditor_role' : 'Auditor', + 'execute_role' : 'Execute', + 'member_role' : 'Member', + 'owner_role' : 'Owner', + 'read_role' : 'Read', + 'scm_update_role' : 'SCM Update', + 'update_role' : 'Update', + 'use_role' : 'Use', +} + +role_descriptions = { + 'system_administrator' : '[TODO] System Administrator', + 'system_auditor' : '[TODO] System Auditor', + 'adhoc_role' : '[TODO] Ad Hoc', + 'admin_role' : '[TODO] Admin', + 'auditor_role' : '[TODO] Auditor', + 'execute_role' : '[TODO] Execute', + 'member_role' : '[TODO] Member', + 'owner_role' : '[TODO] Owner', + 'read_role' : '[TODO] Read', + 'scm_update_role' : '[TODO] SCM Update', + 'update_role' : '[TODO] Update', + 'use_role' : '[TODO] Use', +} + tls = threading.local() # thread local storage @@ -67,7 +98,7 @@ def batch_role_ancestor_rebuilding(allow_nesting=False): delattr(tls, 'roles_needing_rebuilding') -class Role(CommonModelNameNotUnique): +class Role(models.Model): ''' Role model ''' @@ -77,8 +108,8 @@ class Role(CommonModelNameNotUnique): verbose_name_plural = _('roles') db_table = 'main_rbac_roles' + role_field = models.TextField(null=False) singleton_name = models.TextField(null=True, default=None, db_index=True, unique=True) - role_field = models.TextField(null=False, default='') parents = models.ManyToManyField('Role', related_name='children') implicit_parents = models.TextField(null=False, default='[]') ancestors = models.ManyToManyField( @@ -123,6 +154,17 @@ class Role(CommonModelNameNotUnique): ''' Role._simultaneous_ancestry_rebuild([self.id]) + @property + def name(self): + global role_names + return role_names[self.role_field] + + @property + def description(self): + global role_descriptions + return role_descriptions[self.role_field] + + @staticmethod def _simultaneous_ancestry_rebuild(role_ids_to_rebuild): @@ -226,12 +268,18 @@ class Role(CommonModelNameNotUnique): 'roles_table': Role._meta.db_table, } + # SQLlite has a 1M sql statement limit.. since the django sqllite + # driver isn't letting us pass in the ids through the preferred + # parameter binding system, this function exists to obey this. + # est max 12 bytes per number, used up to 3 times in a query, + # minus 4k of padding for the other parts of the query, leads us + # to the magic number of 20748, or 20500 for a nice round number def split_ids_for_sqlite(role_ids): - for i in xrange(0, len(role_ids), 999): - yield role_ids[i:i + 999] + for i in xrange(0, len(role_ids), 20500): + yield role_ids[i:i + 20500] while role_ids_to_rebuild: - if loop_ct > 1000: + if loop_ct > 100: raise Exception('Ancestry role rebuilding error: infinite loop detected') loop_ct += 1 @@ -313,7 +361,7 @@ class Role(CommonModelNameNotUnique): @staticmethod def singleton(name): - role, _ = Role.objects.get_or_create(singleton_name=name, name=name) + role, _ = Role.objects.get_or_create(singleton_name=name, role_field=name) return role def is_ancestor_of(self, role): diff --git a/awx/main/signals.py b/awx/main/signals.py index e4893d34c2..ec1e80869b 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -127,11 +127,10 @@ def create_user_role(instance, **kwargs): Role.objects.get( content_type=ContentType.objects.get_for_model(instance), object_id=instance.id, - name = 'User Admin' + role_field='admin_role' ) except Role.DoesNotExist: role = Role.objects.create( - name = 'User Admin', role_field='admin_role', content_object = instance, ) diff --git a/awx/main/tests/functional/api/test_resource_access_lists.py b/awx/main/tests/functional/api/test_resource_access_lists.py index 75e55fd8ca..9d8d95c98a 100644 --- a/awx/main/tests/functional/api/test_resource_access_lists.py +++ b/awx/main/tests/functional/api/test_resource_access_lists.py @@ -1,6 +1,7 @@ import pytest from django.core.urlresolvers import reverse +from awx.main.models import Role @pytest.mark.django_db def test_indirect_access_list(get, organization, project, team_factory, user, admin): @@ -53,5 +54,5 @@ def test_indirect_access_list(get, organization, project, team_factory, user, ad assert org_admin_team_member_entry['team_name'] == org_admin_team.name admin_entry = admin_res['summary_fields']['indirect_access'][0]['role'] - assert admin_entry['name'] == 'System Administrator' + assert admin_entry['name'] == Role.singleton('system_administrator').name diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 4b481895d6..33c24f3cc6 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -36,7 +36,6 @@ from awx.main.models.organization import ( Team, ) -from awx.main.models.rbac import Role from awx.main.models.notifications import Notifier ''' @@ -193,11 +192,6 @@ def notifier(organization): notification_type="webhook", notification_configuration=dict(url="http://localhost", headers={"Test": "Header"})) - -@pytest.fixture -def role(): - return Role.objects.create(name='role') - @pytest.fixture def admin(user): return user('admin', True) diff --git a/awx/main/tests/functional/test_rbac_api.py b/awx/main/tests/functional/test_rbac_api.py index 3e080c8453..9c1fd34356 100644 --- a/awx/main/tests/functional/test_rbac_api.py +++ b/awx/main/tests/functional/test_rbac_api.py @@ -10,6 +10,10 @@ def mock_feature_enabled(feature, bypass_database=None): #@mock.patch('awx.api.views.feature_enabled', new=mock_feature_enabled) +@pytest.fixture +def role(): + return Role.objects.create() + # # /roles @@ -85,7 +89,7 @@ def test_get_user_roles_list(get, admin): response = get(url, admin) assert response.status_code == 200 roles = response.data - assert roles['count'] > 0 # 'System Administrator' role if nothing else + assert roles['count'] > 0 # 'system_administrator' role if nothing else @pytest.mark.django_db def test_user_view_other_user_roles(organization, inventory, team, get, alice, bob): diff --git a/awx/main/tests/functional/test_rbac_core.py b/awx/main/tests/functional/test_rbac_core.py index 537052afd2..8001cb6d71 100644 --- a/awx/main/tests/functional/test_rbac_core.py +++ b/awx/main/tests/functional/test_rbac_core.py @@ -11,8 +11,8 @@ from awx.main.models import ( @pytest.mark.django_db def test_auto_inheritance_by_children(organization, alice): - A = Role.objects.create(name='A', role_field='') - B = Role.objects.create(name='B', role_field='') + A = Role.objects.create() + B = Role.objects.create() A.members.add(alice) assert alice not in organization.admin_role @@ -38,8 +38,8 @@ def test_auto_inheritance_by_children(organization, alice): @pytest.mark.django_db def test_auto_inheritance_by_parents(organization, alice): - A = Role.objects.create(name='A') - B = Role.objects.create(name='B') + A = Role.objects.create() + B = Role.objects.create() A.members.add(alice) assert alice not in organization.admin_role @@ -58,9 +58,9 @@ def test_auto_inheritance_by_parents(organization, alice): @pytest.mark.django_db def test_accessible_objects(organization, alice, bob): - A = Role.objects.create(name='A') + A = Role.objects.create() A.members.add(alice) - B = Role.objects.create(name='B') + B = Role.objects.create() B.members.add(alice) B.members.add(bob) @@ -118,7 +118,7 @@ def test_auto_field_adjustments(organization, inventory, team, alice): def test_implicit_deletes(alice): 'Ensures implicit resources and roles delete themselves' delorg = Organization.objects.create(name='test-org') - child = Role.objects.create(name='child-role') + child = Role.objects.create() child.parents.add(delorg.admin_role) delorg.admin_role.members.add(alice) @@ -129,14 +129,14 @@ def test_implicit_deletes(alice): assert Role.objects.filter(id=admin_role_id).count() == 1 assert Role.objects.filter(id=auditor_role_id).count() == 1 n_alice_roles = alice.roles.count() - n_system_admin_children = Role.singleton('System Administrator').children.count() + n_system_admin_children = Role.singleton('system_administrator').children.count() delorg.delete() assert Role.objects.filter(id=admin_role_id).count() == 0 assert Role.objects.filter(id=auditor_role_id).count() == 0 assert alice.roles.count() == (n_alice_roles - 1) - assert Role.singleton('System Administrator').children.count() == (n_system_admin_children - 1) + assert Role.singleton('system_administrator').children.count() == (n_system_admin_children - 1) assert child.ancestors.count() == 1 assert child.ancestors.all()[0] == child @@ -152,11 +152,11 @@ def test_content_object(user): def test_hierarchy_rebuilding_multi_path(): 'Tests a subdtle cases around role hierarchy rebuilding when you have multiple paths to the same role of different length' - X = Role.objects.create(name='X') - A = Role.objects.create(name='A') - B = Role.objects.create(name='B') - C = Role.objects.create(name='C') - D = Role.objects.create(name='D') + X = Role.objects.create() + A = Role.objects.create() + B = Role.objects.create() + C = Role.objects.create() + D = Role.objects.create() A.children.add(B) A.children.add(D) diff --git a/awx/main/tests/functional/test_rbac_project.py b/awx/main/tests/functional/test_rbac_project.py index d2e504645a..a225154d21 100644 --- a/awx/main/tests/functional/test_rbac_project.py +++ b/awx/main/tests/functional/test_rbac_project.py @@ -148,7 +148,7 @@ def test_project_user_project(user_project, project, user): def test_project_accessible_by_sa(user, project): u = user('systemadmin', is_superuser=True) # This gets setup by a signal, but we want to test the migration which will set this up too, so remove it - Role.singleton('System Administrator').members.remove(u) + Role.singleton('system_administrator').members.remove(u) assert u not in project.read_role rbac.migrate_organization(apps, None) diff --git a/awx/main/tests/functional/test_rbac_user.py b/awx/main/tests/functional/test_rbac_user.py index 8e620771f5..c5959a2c32 100644 --- a/awx/main/tests/functional/test_rbac_user.py +++ b/awx/main/tests/functional/test_rbac_user.py @@ -13,7 +13,7 @@ def test_user_admin(user_project, project, user): joe = user(username, is_superuser = False) admin = user('admin', is_superuser = True) - sa = Role.singleton('System Administrator') + sa = Role.singleton('system_administrator') # this should happen automatically with our signal assert sa.members.filter(id=admin.id).exists() is True