From be8a1f48599760118e6d7f399ec2082452c082c6 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Tue, 3 May 2016 20:48:06 -0400 Subject: [PATCH 1/7] Another orgfunc migration fix --- awx/main/migrations/_rbac.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 6f1ef0d4ad..05fff2adbb 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -150,7 +150,10 @@ def _discover_credentials(instances, cred, orgfunc): pass if len(orgs) == 1: - _update_credential_parents(orgfunc(instances[0]), cred) + try: + _update_credential_parents(orgfunc(instances[0]), cred) + except AttributeError: + pass else: for pos, org in enumerate(orgs): if pos == 0: From fb97438573a891bb4304c3fbad1accce9266566d Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 11:31:04 -0400 Subject: [PATCH 2/7] Enforce jt admin_role requirement for changing/deleting JobTemplates --- awx/main/access.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 5e36bc7068..d9055e925b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -813,6 +813,8 @@ class JobTemplateAccess(BaseAccess): def can_change(self, obj, data): data_for_change = data + if self.user not in obj.admin_role: + return False if data is not None: data_for_change = dict(data) for required_field in ('credential', 'cloud_credential', 'inventory', 'project'): @@ -822,12 +824,7 @@ class JobTemplateAccess(BaseAccess): return self.can_read(obj) and self.can_add(data_for_change) def can_delete(self, obj): - add_obj = dict(credential=obj.credential.id if obj.credential is not None else None, - cloud_credential=obj.cloud_credential.id if obj.cloud_credential is not None else None, - inventory=obj.inventory.id if obj.inventory is not None else None, - project=obj.project.id if obj.project is not None else None, - job_type=obj.job_type) - return self.can_add(add_obj) + return self.user in obj.admin_role class JobAccess(BaseAccess): From 6c0c7896127a58962777fc27e3eda1e81d668238 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 11:33:10 -0400 Subject: [PATCH 3/7] Make job queryset only require JT read access to see As opposed to credential access, since users can see and potentially run JT's without credential access now. --- awx/main/access.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index d9055e925b..e952ba730e 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -838,9 +838,7 @@ class JobAccess(BaseAccess): if self.user.is_superuser: return qs.all() - credential_ids = self.user.get_queryset(Credential) return qs.filter( - credential_id__in=credential_ids, job_template__in=JobTemplate.accessible_objects(self.user, 'read_role') ) From 4378430c4fb1bdc1b49c4658a0d6f13edbff7efb Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 11:35:11 -0400 Subject: [PATCH 4/7] Minor migration speed improvement for old access queries --- awx/main/migrations/_old_access.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/awx/main/migrations/_old_access.py b/awx/main/migrations/_old_access.py index 88021bc937..72bda1ee8d 100644 --- a/awx/main/migrations/_old_access.py +++ b/awx/main/migrations/_old_access.py @@ -686,7 +686,7 @@ class ProjectAccess(BaseAccess): qs = qs.select_related('modified_by', 'credential', 'current_job', 'last_job') if self.user.is_superuser: return qs - team_ids = set(Team.objects.filter(deprecated_users__in=[self.user]).values_list('id', flat=True)) + team_ids = Team.objects.filter(deprecated_users__in=[self.user]) qs = qs.filter(Q(created_by=self.user, deprecated_organizations__isnull=True) | Q(deprecated_organizations__deprecated_admins__in=[self.user]) | Q(deprecated_organizations__deprecated_users__in=[self.user]) | @@ -694,17 +694,17 @@ class ProjectAccess(BaseAccess): allowed_deploy = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY] allowed_check = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY, PERM_INVENTORY_CHECK] - deploy_permissions_ids = set(Permission.objects.filter( + deploy_permissions = Permission.objects.filter( Q(user=self.user) | Q(team_id__in=team_ids), permission_type__in=allowed_deploy, - ).values_list('id', flat=True)) - check_permissions_ids = set(Permission.objects.filter( + ) + check_permissions = Permission.objects.filter( Q(user=self.user) | Q(team_id__in=team_ids), permission_type__in=allowed_check, - ).values_list('id', flat=True)) + ) - perm_deploy_qs = qs.filter(permissions__in=deploy_permissions_ids) - perm_check_qs = qs.filter(permissions__in=check_permissions_ids) + perm_deploy_qs = qs.filter(permissions__in=deploy_permissions) + perm_check_qs = qs.filter(permissions__in=check_permissions) return qs | perm_deploy_qs | perm_check_qs def can_add(self, data): @@ -1047,8 +1047,10 @@ class JobTemplateAccess(BaseAccess): obj.job_type == PERM_INVENTORY_CHECK: has_perm = True - dep_access = check_user_access(self.user, Inventory, 'read', obj.inventory) and check_user_access(self.user, Project, 'read', obj.project) - return dep_access and has_perm + return \ + has_perm and \ + check_user_access(self.user, Inventory, 'read', obj.inventory) and \ + check_user_access(self.user, Project, 'read', obj.project) def can_change(self, obj, data): data_for_change = data From 27457239bc3bb758a42180917f171e3c89bb1532 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 11:41:16 -0400 Subject: [PATCH 5/7] Fixed up job template migrations Massive speed improvement as well as made it not wrong (or at least less wrong). --- awx/main/migrations/_rbac.py | 83 ++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 05fff2adbb..36257fd5d3 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -8,7 +8,6 @@ from collections import defaultdict from awx.main.utils import getattrd from awx.main.models.rbac import Role, batch_role_ancestor_rebuilding -import _old_access as old_access logger = logging.getLogger(__name__) def log_migration(wrapped): @@ -385,33 +384,81 @@ def migrate_job_templates(apps, schema_editor): JobTemplate = apps.get_model('main', 'JobTemplate') Team = apps.get_model('main', 'Team') Permission = apps.get_model('main', 'Permission') + Credential = apps.get_model('main', 'Credential') - for jt in JobTemplate.objects.iterator(): - permission = Permission.objects.filter( + jt_queryset = JobTemplate.objects.select_related('inventory', 'project', 'inventory__organization', 'execute_role') + + for jt in jt_queryset.iterator(): + jt_permission_qs = Permission.objects.filter( inventory=jt.inventory, project=jt.project, - permission_type__in=['create', 'check', 'run'] if jt.job_type == 'check' else ['create', 'run'], ) - for team in Team.objects.iterator(): - if permission.filter(team=team).exists(): + inventory_permission_qs = Permission.objects.filter( + inventory=jt.inventory, + project__isnull=True, + ) + + team_create_permissions = set( + jt_permission_qs + .filter(permission_type__in=['create'] if jt.job_type == 'check' else ['create']) + .values_list('team__id', flat=True) + ) + team_run_permissions = set( + jt_permission_qs + .filter(permission_type__in=['check', 'run'] if jt.job_type == 'check' else ['run']) + .values_list('team__id', flat=True) + ) + user_create_permissions = set( + jt_permission_qs + .filter(permission_type__in=['create'] if jt.job_type == 'check' else ['run']) + .values_list('user__id', flat=True) + ) + user_run_permissions = set( + jt_permission_qs + .filter(permission_type__in=['check', 'run'] if jt.job_type == 'check' else ['create']) + .values_list('user__id', flat=True) + ) + + team_inv_permissions = defaultdict(set) + user_inv_permissions = defaultdict(set) + + for user_id, team_id, inventory_id in inventory_permission_qs.values_list('user_id', 'team_id', 'inventory_id'): + if user_id: + user_inv_permissions[user_id].add(inventory_id) + if team_id: + team_inv_permissions[team_id].add(inventory_id) + + + for team in Team.objects.filter(id__in=team_create_permissions).iterator(): + if jt.inventory.id in team_inv_permissions[team.id] and \ + ((not jt.credential and not jt.cloud_credential) or + Credential.objects.filter(deprecated_team=team, jobtemplates=jt).exists()): + team.member_role.children.add(jt.admin_role) + logger.info(smart_text(u'transfering admin access on JobTemplate({}) to Team({})'.format(jt.name, team.name))) + for team in Team.objects.filter(id__in=team_run_permissions).iterator(): + if jt.inventory.id in team_inv_permissions[team.id] and \ + ((not jt.credential and not jt.cloud_credential) or + Credential.objects.filter(deprecated_team=team, jobtemplates=jt).exists()): team.member_role.children.add(jt.execute_role) - logger.info(smart_text(u'adding Team({}) access to JobTemplate({})'.format(team.name, jt.name))) + logger.info(smart_text(u'transfering execute access on JobTemplate({}) to Team({})'.format(jt.name, team.name))) - for user in User.objects.iterator(): - if permission.filter(user=user).exists(): + for user in User.objects.filter(id__in=user_create_permissions).iterator(): + if (jt.inventory.id in user_inv_permissions[user.id] or + any([jt.inventory.id in team_inv_permissions[team.id] for team in user.deprecated_teams.all()])) and \ + ((not jt.credential and not jt.cloud_credential) or + Credential.objects.filter(Q(deprecated_user=user) | Q(deprecated_team__deprecated_users=user), jobtemplates=jt).exists()): + jt.admin_role.members.add(user) + logger.info(smart_text(u'transfering admin access on JobTemplate({}) to User({})'.format(jt.name, user.username))) + for user in User.objects.filter(id__in=user_run_permissions).iterator(): + if (jt.inventory.id in user_inv_permissions[user.id] or + any([jt.inventory.id in team_inv_permissions[team.id] for team in user.deprecated_teams.all()])) and \ + ((not jt.credential and not jt.cloud_credential) or + Credential.objects.filter(Q(deprecated_user=user) | Q(deprecated_team__deprecated_users=user), jobtemplates=jt).exists()): jt.execute_role.members.add(user) - logger.info(smart_text(u'adding User({}) access to JobTemplate({})'.format(user.username, jt.name))) + logger.info(smart_text(u'transfering execute access on JobTemplate({}) to User({})'.format(jt.name, user.username))) - if jt.execute_role.ancestors.filter(members=user).exists(): # aka "user in jt.execute_role" - # If the job template is already accessible by the user, because they - # are a sytem, organization, or project admin, then don't add an explicit - # role entry for them - continue - if old_access.check_user_access(user, jt.__class__, 'start', jt, False): - jt.execute_role.members.add(user) - logger.info(smart_text(u'adding User({}) access to JobTemplate({})'.format(user.username, jt.name))) @log_migration def rebuild_role_hierarchy(apps, schema_editor): From acc49bbbefd5d74fa60f0390ed57eed35c774383 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 13:46:13 -0400 Subject: [PATCH 6/7] Fixed up JT migration tests --- .../functional/test_rbac_job_templates.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/awx/main/tests/functional/test_rbac_job_templates.py b/awx/main/tests/functional/test_rbac_job_templates.py index e439533f07..f775053ccc 100644 --- a/awx/main/tests/functional/test_rbac_job_templates.py +++ b/awx/main/tests/functional/test_rbac_job_templates.py @@ -11,10 +11,12 @@ from django.apps import apps @pytest.mark.django_db -def test_job_template_migration_check(deploy_jobtemplate, check_jobtemplate, user): +def test_job_template_migration_check(credential, deploy_jobtemplate, check_jobtemplate, user): admin = user('admin', is_superuser=True) joe = user('joe') + credential.deprecated_user = joe + credential.save() check_jobtemplate.project.organization.deprecated_users.add(joe) @@ -22,6 +24,7 @@ def test_job_template_migration_check(deploy_jobtemplate, check_jobtemplate, use Permission(user=joe, inventory=check_jobtemplate.inventory, project=check_jobtemplate.project, permission_type='check').save() + rbac.migrate_users(apps, None) rbac.migrate_organization(apps, None) rbac.migrate_projects(apps, None) @@ -39,10 +42,12 @@ def test_job_template_migration_check(deploy_jobtemplate, check_jobtemplate, use assert joe not in deploy_jobtemplate.execute_role @pytest.mark.django_db -def test_job_template_migration_deploy(deploy_jobtemplate, check_jobtemplate, user): +def test_job_template_migration_deploy(credential, deploy_jobtemplate, check_jobtemplate, user): admin = user('admin', is_superuser=True) joe = user('joe') + credential.deprecated_user = joe + credential.save() deploy_jobtemplate.project.organization.deprecated_users.add(joe) @@ -68,13 +73,16 @@ def test_job_template_migration_deploy(deploy_jobtemplate, check_jobtemplate, us @pytest.mark.django_db -def test_job_template_team_migration_check(deploy_jobtemplate, check_jobtemplate, organization, team, user): +def test_job_template_team_migration_check(credential, deploy_jobtemplate, check_jobtemplate, organization, team, user): admin = user('admin', is_superuser=True) joe = user('joe') team.deprecated_users.add(joe) team.organization = organization team.save() + credential.deprecated_team = team + credential.save() + check_jobtemplate.project.organization.deprecated_users.add(joe) Permission(team=team, inventory=check_jobtemplate.inventory, permission_type='read').save() @@ -101,13 +109,16 @@ def test_job_template_team_migration_check(deploy_jobtemplate, check_jobtemplate @pytest.mark.django_db -def test_job_template_team_deploy_migration(deploy_jobtemplate, check_jobtemplate, organization, team, user): +def test_job_template_team_deploy_migration(credential, deploy_jobtemplate, check_jobtemplate, organization, team, user): admin = user('admin', is_superuser=True) joe = user('joe') team.deprecated_users.add(joe) team.organization = organization team.save() + credential.deprecated_team = team + credential.save() + deploy_jobtemplate.project.organization.deprecated_users.add(joe) Permission(team=team, inventory=deploy_jobtemplate.inventory, permission_type='read').save() From 9d46a39e428ce77ba3ccdccd285e7061000698d1 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 12 May 2016 16:31:32 -0400 Subject: [PATCH 7/7] Added comment context to caught exception --- awx/main/migrations/_rbac.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 36257fd5d3..94465084b4 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -152,6 +152,8 @@ def _discover_credentials(instances, cred, orgfunc): try: _update_credential_parents(orgfunc(instances[0]), cred) except AttributeError: + # JobTemplate.inventory can be NULL sometimes, eg when an inventory + # has been deleted. This protects against that. pass else: for pos, org in enumerate(orgs):