From daa9282790ca9c3f4fedfdd9fdf0251bba663824 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 16 Jan 2020 14:59:43 -0500 Subject: [PATCH 1/6] Initial (editable) pass of adding JT.organization This is the old version of this feature from 2019 this allows setting the organization in the data sent to the API when creating a JT, and exposes the field in the UI as well Subsequent commit changes the field from editable to read-only, but as of this commit, the machinery is not hooked up to infer it from project --- awx/api/generics.py | 9 + awx/api/serializers.py | 32 ++-- awx/api/urls/organization.py | 2 + awx/api/views/__init__.py | 1 + awx/api/views/mixin.py | 37 +--- awx/api/views/organization.py | 22 ++- awx/main/access.py | 132 ++++++------- awx/main/fields.py | 38 ++-- awx/main/middleware.py | 36 +++- ...85_v360_job_template_organization_field.py | 72 +++++++ awx/main/migrations/_rbac.py | 179 ++++++++++++++++-- awx/main/models/inventory.py | 16 +- awx/main/models/jobs.py | 30 +-- awx/main/models/organization.py | 8 +- awx/main/models/projects.py | 13 +- awx/main/models/unified_jobs.py | 16 ++ awx/main/models/workflow.py | 9 +- awx/main/signals.py | 1 - awx/main/tests/factories/fixtures.py | 3 +- awx/main/tests/factories/tower.py | 2 +- .../test_deprecated_credential_assignment.py | 1 + awx/main/tests/functional/api/test_job.py | 29 ++- .../tests/functional/api/test_job_template.py | 47 ++++- .../api/test_organization_counts.py | 60 ++---- .../functional/api/test_rbac_displays.py | 5 +- .../api/test_unified_job_template.py | 110 +++++++++++ awx/main/tests/functional/conftest.py | 37 ++-- .../functional/models/test_activity_stream.py | 5 +- awx/main/tests/functional/models/test_job.py | 9 +- .../tests/functional/models/test_project.py | 6 + awx/main/tests/functional/test_copy.py | 4 +- awx/main/tests/functional/test_instances.py | 11 +- awx/main/tests/functional/test_named_url.py | 22 ++- awx/main/tests/functional/test_projects.py | 26 +-- awx/main/tests/functional/test_rbac_job.py | 3 +- .../tests/functional/test_rbac_job_start.py | 11 +- .../functional/test_rbac_job_templates.py | 115 +++++++---- .../tests/functional/test_rbac_migration.py | 64 +++++++ .../tests/functional/test_rbac_workflow.py | 5 +- .../test_job_template_serializers.py | 1 + .../unit/models/test_unified_job_unit.py | 8 + awx/main/tests/unit/test_access.py | 29 +-- awx/main/tests/unit/test_fields.py | 63 +++++- .../job_templates/job-template.form.js | 13 ++ awxkit/awxkit/api/pages/job_templates.py | 14 +- .../api/pages/workflow_job_templates.py | 6 +- 46 files changed, 985 insertions(+), 377 deletions(-) create mode 100644 awx/main/migrations/0085_v360_job_template_organization_field.py create mode 100644 awx/main/tests/functional/test_rbac_migration.py diff --git a/awx/api/generics.py b/awx/api/generics.py index af763d875e..f352019c65 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -548,6 +548,15 @@ class SubListCreateAPIView(SubListAPIView, ListCreateAPIView): }) return d + def get_queryset(self): + if hasattr(self, 'parent_key'): + # Prefer this filtering because ForeignKey allows us more assumptions + parent = self.get_parent_object() + self.check_parent_access(parent) + qs = self.request.user.get_queryset(self.model) + return qs.filter(**{self.parent_key: parent}) + return super(SubListCreateAPIView, self).get_queryset() + def create(self, request, *args, **kwargs): # If the object ID was not specified, it probably doesn't exist in the # DB yet. We want to see if we can create it. The URL may choose to diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 858aeb4ebf..1240d00291 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -642,7 +642,7 @@ class UnifiedJobTemplateSerializer(BaseSerializer): _capabilities_prefetch = [ 'admin', 'execute', {'copy': ['jobtemplate.project.use', 'jobtemplate.inventory.use', - 'workflowjobtemplate.organization.workflow_admin']} + 'organization.workflow_admin']} ] class Meta: @@ -700,6 +700,18 @@ class UnifiedJobTemplateSerializer(BaseSerializer): else: return super(UnifiedJobTemplateSerializer, self).to_representation(obj) + def validate(self, attrs): + if 'organization' in self.fields: + # Do not allow setting template organization to null + # otherwise be as non-restrictive as possible for PATCH or PUT, even with orphans + # does not correspond with any REST framework field construct + if self.instance is None and attrs.get('organization', None) is None: + raise serializers.ValidationError({'organization': _('Organization required for new object.')}) + if self.instance and self.instance.organization_id and attrs.get('organization', 'blank') is None: + raise serializers.ValidationError({'organization': _('Organization can not be set to null.')}) + + return super(UnifiedJobTemplateSerializer, self).validate(attrs) + class UnifiedJobSerializer(BaseSerializer): show_capabilities = ['start', 'delete'] @@ -1387,12 +1399,6 @@ class ProjectSerializer(UnifiedJobTemplateSerializer, ProjectOptionsSerializer): def get_field_from_model_or_attrs(fd): return attrs.get(fd, self.instance and getattr(self.instance, fd) or None) - organization = None - if 'organization' in attrs: - organization = attrs['organization'] - elif self.instance: - organization = self.instance.organization - if 'allow_override' in attrs and self.instance: # case where user is turning off this project setting if self.instance.allow_override and not attrs['allow_override']: @@ -1408,11 +1414,7 @@ class ProjectSerializer(UnifiedJobTemplateSerializer, ProjectOptionsSerializer): ' '.join([str(pk) for pk in used_by]) )}) - view = self.context.get('view', None) - if not organization and not view.request.user.is_superuser: - # Only allow super users to create orgless projects - raise serializers.ValidationError(_('Organization is missing')) - elif get_field_from_model_or_attrs('scm_type') == '': + if get_field_from_model_or_attrs('scm_type') == '': for fd in ('scm_update_on_launch', 'scm_delete_on_update', 'scm_clean'): if get_field_from_model_or_attrs(fd): raise serializers.ValidationError({fd: _('Update options must be set to false for manual projects.')}) @@ -2738,7 +2740,7 @@ class JobOptionsSerializer(LabelsListMixin, BaseSerializer): fields = ('*', 'job_type', 'inventory', 'project', 'playbook', 'scm_branch', 'forks', 'limit', 'verbosity', 'extra_vars', 'job_tags', 'force_handlers', 'skip_tags', 'start_at_task', 'timeout', - 'use_fact_cache',) + 'use_fact_cache', 'organization',) def get_related(self, obj): res = super(JobOptionsSerializer, self).get_related(obj) @@ -2753,6 +2755,8 @@ class JobOptionsSerializer(LabelsListMixin, BaseSerializer): res['project'] = self.reverse('api:project_detail', kwargs={'pk': obj.project.pk}) except ObjectDoesNotExist: setattr(obj, 'project', None) + if obj.organization_id: + res['organization'] = self.reverse('api:organization_detail', kwargs={'pk': obj.organization_id}) if isinstance(obj, UnifiedJobTemplate): res['extra_credentials'] = self.reverse( 'api:job_template_extra_credentials_list', @@ -2899,6 +2903,8 @@ class JobTemplateSerializer(JobTemplateMixin, UnifiedJobTemplateSerializer, JobO ) if obj.host_config_key: res['callback'] = self.reverse('api:job_template_callback', kwargs={'pk': obj.pk}) + if obj.organization_id: + res['organization'] = self.reverse('api:organization_detail', kwargs={'pk': obj.organization_id}) return res def validate(self, attrs): diff --git a/awx/api/urls/organization.py b/awx/api/urls/organization.py index 1b0997b05c..3d172f1360 100644 --- a/awx/api/urls/organization.py +++ b/awx/api/urls/organization.py @@ -10,6 +10,7 @@ from awx.api.views import ( OrganizationAdminsList, OrganizationInventoriesList, OrganizationProjectsList, + OrganizationJobTemplatesList, OrganizationWorkflowJobTemplatesList, OrganizationTeamsList, OrganizationCredentialList, @@ -33,6 +34,7 @@ urls = [ url(r'^(?P[0-9]+)/admins/$', OrganizationAdminsList.as_view(), name='organization_admins_list'), url(r'^(?P[0-9]+)/inventories/$', OrganizationInventoriesList.as_view(), name='organization_inventories_list'), url(r'^(?P[0-9]+)/projects/$', OrganizationProjectsList.as_view(), name='organization_projects_list'), + url(r'^(?P[0-9]+)/job_templates/$', OrganizationJobTemplatesList.as_view(), name='organization_job_templates_list'), url(r'^(?P[0-9]+)/workflow_job_templates/$', OrganizationWorkflowJobTemplatesList.as_view(), name='organization_workflow_job_templates_list'), url(r'^(?P[0-9]+)/teams/$', OrganizationTeamsList.as_view(), name='organization_teams_list'), url(r'^(?P[0-9]+)/credentials/$', OrganizationCredentialList.as_view(), name='organization_credential_list'), diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 0e937da67c..1339c50c68 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -111,6 +111,7 @@ from awx.api.views.organization import ( # noqa OrganizationUsersList, OrganizationAdminsList, OrganizationProjectsList, + OrganizationJobTemplatesList, OrganizationWorkflowJobTemplatesList, OrganizationTeamsList, OrganizationActivityStreamList, diff --git a/awx/api/views/mixin.py b/awx/api/views/mixin.py index e7d4959dfc..9b57278e2e 100644 --- a/awx/api/views/mixin.py +++ b/awx/api/views/mixin.py @@ -4,10 +4,7 @@ import dateutil import logging -from django.db.models import ( - Count, - F, -) +from django.db.models import Count from django.db import transaction from django.shortcuts import get_object_or_404 from django.utils.timezone import now @@ -175,28 +172,18 @@ class OrganizationCountsMixin(object): inv_qs = Inventory.accessible_objects(self.request.user, 'read_role') project_qs = Project.accessible_objects(self.request.user, 'read_role') + jt_qs = JobTemplate.accessible_objects(self.request.user, 'read_role') # Produce counts of Foreign Key relationships - db_results['inventories'] = inv_qs\ - .values('organization').annotate(Count('organization')).order_by('organization') + db_results['inventories'] = inv_qs.values('organization').annotate(Count('organization')).order_by('organization') db_results['teams'] = Team.accessible_objects( self.request.user, 'read_role').values('organization').annotate( Count('organization')).order_by('organization') - JT_project_reference = 'project__organization' - JT_inventory_reference = 'inventory__organization' - db_results['job_templates_project'] = JobTemplate.accessible_objects( - self.request.user, 'read_role').exclude( - project__organization=F(JT_inventory_reference)).values(JT_project_reference).annotate( - Count(JT_project_reference)).order_by(JT_project_reference) + db_results['job_templates'] = jt_qs.values('organization').annotate(Count('organization')).order_by('organization') - db_results['job_templates_inventory'] = JobTemplate.accessible_objects( - self.request.user, 'read_role').values(JT_inventory_reference).annotate( - Count(JT_inventory_reference)).order_by(JT_inventory_reference) - - db_results['projects'] = project_qs\ - .values('organization').annotate(Count('organization')).order_by('organization') + db_results['projects'] = project_qs.values('organization').annotate(Count('organization')).order_by('organization') # Other members and admins of organization are always viewable db_results['users'] = org_qs.annotate( @@ -212,11 +199,7 @@ class OrganizationCountsMixin(object): 'admins': 0, 'projects': 0} for res, count_qs in db_results.items(): - if res == 'job_templates_project': - org_reference = JT_project_reference - elif res == 'job_templates_inventory': - org_reference = JT_inventory_reference - elif res == 'users': + if res == 'users': org_reference = 'id' else: org_reference = 'organization' @@ -229,14 +212,6 @@ class OrganizationCountsMixin(object): continue count_context[org_id][res] = entry['%s__count' % org_reference] - # Combine the counts for job templates by project and inventory - for org in org_id_list: - org_id = org['id'] - count_context[org_id]['job_templates'] = 0 - for related_path in ['job_templates_project', 'job_templates_inventory']: - if related_path in count_context[org_id]: - count_context[org_id]['job_templates'] += count_context[org_id].pop(related_path) - full_context['related_field_counts'] = count_context return full_context diff --git a/awx/api/views/organization.py b/awx/api/views/organization.py index e1af4c67b1..cb929ec5b5 100644 --- a/awx/api/views/organization.py +++ b/awx/api/views/organization.py @@ -20,7 +20,7 @@ from awx.main.models import ( Role, User, Team, - InstanceGroup, + InstanceGroup ) from awx.api.generics import ( ListCreateAPIView, @@ -28,6 +28,7 @@ from awx.api.generics import ( SubListAPIView, SubListCreateAttachDetachAPIView, SubListAttachDetachAPIView, + SubListCreateAPIView, ResourceAccessList, BaseUsersList, ) @@ -35,14 +36,13 @@ from awx.api.generics import ( from awx.api.serializers import ( OrganizationSerializer, InventorySerializer, - ProjectSerializer, UserSerializer, TeamSerializer, ActivityStreamSerializer, RoleSerializer, NotificationTemplateSerializer, - WorkflowJobTemplateSerializer, InstanceGroupSerializer, + ProjectSerializer, JobTemplateSerializer, WorkflowJobTemplateSerializer ) from awx.api.views.mixin import ( RelatedJobsPreventDeleteMixin, @@ -94,7 +94,7 @@ class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPI org_counts['projects'] = Project.accessible_objects(**access_kwargs).filter( organization__id=org_id).count() org_counts['job_templates'] = JobTemplate.accessible_objects(**access_kwargs).filter( - project__organization__id=org_id).count() + organization__id=org_id).count() full_context['related_field_counts'] = {} full_context['related_field_counts'][org_id] = org_counts @@ -128,21 +128,27 @@ class OrganizationAdminsList(BaseUsersList): ordering = ('username',) -class OrganizationProjectsList(SubListCreateAttachDetachAPIView): +class OrganizationProjectsList(SubListCreateAPIView): model = Project serializer_class = ProjectSerializer parent_model = Organization - relationship = 'projects' parent_key = 'organization' -class OrganizationWorkflowJobTemplatesList(SubListCreateAttachDetachAPIView): +class OrganizationJobTemplatesList(SubListCreateAPIView): + + model = JobTemplate + serializer_class = JobTemplateSerializer + parent_model = Organization + parent_key = 'organization' + + +class OrganizationWorkflowJobTemplatesList(SubListCreateAPIView): model = WorkflowJobTemplate serializer_class = WorkflowJobTemplateSerializer parent_model = Organization - relationship = 'workflows' parent_key = 'organization' diff --git a/awx/main/access.py b/awx/main/access.py index 95ec0f20a8..d6ae9c0082 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1411,7 +1411,7 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): ''' model = JobTemplate - select_related = ('created_by', 'modified_by', 'inventory', 'project', + select_related = ('created_by', 'modified_by', 'inventory', 'project', 'organization', 'next_schedule',) prefetch_related = ( 'instance_groups', @@ -1435,9 +1435,7 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): Users who are able to create deploy jobs can also run normal and check (dry run) jobs. ''' if not data: # So the browseable API will work - return ( - Project.accessible_objects(self.user, 'use_role').exists() or - Inventory.accessible_objects(self.user, 'use_role').exists()) + return Organization.accessible_objects(self.user, 'job_template_admin_role').exists() # if reference_obj is provided, determine if it can be copied reference_obj = data.get('reference_obj', None) @@ -1467,6 +1465,10 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): if self.user not in inventory.use_role: return False + organization = get_value(Organization, 'organization') + if (not organization) or (self.user not in organization.job_template_admin_role): + return False + project = get_value(Project, 'project') # If the user has admin access to the project (as an org admin), should # be able to proceed without additional checks. @@ -1504,22 +1506,31 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): return self.user in obj.execute_role def can_change(self, obj, data): - data_for_change = data if self.user not in obj.admin_role and not self.user.is_superuser: return False - if data is not None: - data = dict(data) + if data is None: + return True - if self.changes_are_non_sensitive(obj, data): - if 'survey_enabled' in data and obj.survey_enabled != data['survey_enabled'] and data['survey_enabled']: - self.check_license(feature='surveys') - return True + # standard type of check for organization - cannot change the value + # unless posessing the respective job_template_admin_role, otherwise non-blocking + if not self.check_related('organization', Organization, data, obj=obj, role_field='job_template_admin_role'): + return False - for required_field in ('inventory', 'project'): - required_obj = getattr(obj, required_field, None) - if required_field not in data_for_change and required_obj is not None: - data_for_change[required_field] = required_obj.pk - return self.can_read(obj) and (self.can_add(data_for_change) if data is not None else True) + data = dict(data) + + if 'survey_enabled' in data and obj.survey_enabled != data['survey_enabled'] and data['survey_enabled']: + self.check_license(feature='surveys') + + if self.changes_are_non_sensitive(obj, data): + return True + + for required_field, cls in (('inventory', Inventory), ('project', Project)): + is_mandatory = True + if not getattr(obj, '{}_id'.format(required_field)): + is_mandatory = False + if not self.check_related(required_field, cls, data, obj=obj, role_field='use_role', mandatory=is_mandatory): + return False + return True def changes_are_non_sensitive(self, obj, data): ''' @@ -1554,9 +1565,9 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): @check_superuser def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): if relationship == "instance_groups": - if not obj.project.organization: + if not obj.organization: return False - return self.user.can_access(type(sub_obj), "read", sub_obj) and self.user in obj.project.organization.admin_role + return self.user.can_access(type(sub_obj), "read", sub_obj) and self.user in obj.organization.admin_role if relationship == 'credentials' and isinstance(sub_obj, Credential): return self.user in obj.admin_role and self.user in sub_obj.use_role return super(JobTemplateAccess, self).can_attach( @@ -1587,6 +1598,7 @@ class JobAccess(BaseAccess): select_related = ('created_by', 'modified_by', 'job_template', 'inventory', 'project', 'project_update',) prefetch_related = ( + 'organization', 'unified_job_template', 'instance_group', 'credentials__credential_type', @@ -1607,42 +1619,19 @@ class JobAccess(BaseAccess): return qs.filter( Q(job_template__in=JobTemplate.accessible_objects(self.user, 'read_role')) | - Q(inventory__organization__in=org_access_qs) | - Q(project__organization__in=org_access_qs)).distinct() - - def related_orgs(self, obj): - orgs = [] - if obj.inventory and obj.inventory.organization: - orgs.append(obj.inventory.organization) - if obj.project and obj.project.organization and obj.project.organization not in orgs: - orgs.append(obj.project.organization) - return orgs - - def org_access(self, obj, role_types=['admin_role']): - orgs = self.related_orgs(obj) - for org in orgs: - for role_type in role_types: - role = getattr(org, role_type) - if self.user in role: - return True - return False + Q(organization__in=org_access_qs)).distinct() def can_add(self, data, validate_license=True): - if validate_license: - self.check_license() - - if not data: # So the browseable API will work - return True - return self.user.is_superuser + raise NotImplementedError('Direct job creation not possible in v2 API') def can_change(self, obj, data): - return (obj.status == 'new' and - self.can_read(obj) and - self.can_add(data, validate_license=False)) + raise NotImplementedError('Direct job editing not supported in v2 API') @check_superuser def can_delete(self, obj): - return self.org_access(obj) + if not obj.organization: + return False + return self.user in obj.organization.admin_role def can_start(self, obj, validate_license=True): if validate_license: @@ -1662,6 +1651,7 @@ class JobAccess(BaseAccess): except JobLaunchConfig.DoesNotExist: config = None + # Standard permissions model (1) if obj.job_template and (self.user not in obj.job_template.execute_role): return False @@ -1676,24 +1666,15 @@ class JobAccess(BaseAccess): if JobLaunchConfigAccess(self.user).can_add({'reference_obj': config}): return True - org_access = bool(obj.inventory) and self.user in obj.inventory.organization.inventory_admin_role - project_access = obj.project is None or self.user in obj.project.admin_role - credential_access = all([self.user in cred.use_role for cred in obj.credentials.all()]) + # Standard permissions model (2) + if obj.organization and self.user in obj.organization.execute_role: + # Respect organization ownership of orphaned jobs + return True + elif not (obj.job_template or obj.organization): + if self.save_messages: + self.messages['detail'] = _('Job has been orphaned from its job template and organization.') - # job can be relaunched if user could make an equivalent JT - ret = org_access and credential_access and project_access - if not ret and self.save_messages and not self.messages: - if not obj.job_template: - pretext = _('Job has been orphaned from its job template.') - elif config is None: - pretext = _('Job was launched with unknown prompted fields.') - else: - pretext = _('Job was launched with prompted fields.') - if credential_access: - self.messages['detail'] = '{} {}'.format(pretext, _(' Organization level permissions required.')) - else: - self.messages['detail'] = '{} {}'.format(pretext, _(' You do not have permission to related resources.')) - return ret + return False def get_method_capability(self, method, obj, parent_obj): if method == 'start': @@ -1706,10 +1687,16 @@ class JobAccess(BaseAccess): def can_cancel(self, obj): if not obj.can_cancel: return False - # Delete access allows org admins to stop running jobs - if self.user == obj.created_by or self.can_delete(obj): + # Users may always cancel their own jobs + if self.user == obj.created_by: return True - return obj.job_template is not None and self.user in obj.job_template.admin_role + # Users with direct admin to JT may cancel jobs started by anyone + if obj.job_template and self.user in obj.job_template.admin_role: + return True + # If orphaned, allow org JT admins to stop running jobs + if not obj.job_template and obj.organization and self.user in obj.organization.job_template_admin_role: + return True + return False class SystemJobTemplateAccess(BaseAccess): @@ -1944,11 +1931,11 @@ class WorkflowJobNodeAccess(BaseAccess): # TODO: notification attachments? class WorkflowJobTemplateAccess(NotificationAttachMixin, BaseAccess): ''' - I can only see/manage Workflow Job Templates if I'm a super user + I can see/manage Workflow Job Templates based on object roles ''' model = WorkflowJobTemplate - select_related = ('created_by', 'modified_by', 'next_schedule', + select_related = ('created_by', 'modified_by', 'organization', 'next_schedule', 'admin_role', 'execute_role', 'read_role',) def filtered_queryset(self): @@ -2038,7 +2025,7 @@ class WorkflowJobAccess(BaseAccess): I can also cancel it if I started it ''' model = WorkflowJob - select_related = ('created_by', 'modified_by',) + select_related = ('created_by', 'modified_by', 'organization',) def filtered_queryset(self): return WorkflowJob.objects.filter( @@ -2332,6 +2319,7 @@ class UnifiedJobTemplateAccess(BaseAccess): prefetch_related = ( 'last_job', 'current_job', + 'organization', 'credentials__credential_type', Prefetch('labels', queryset=Label.objects.all().order_by('name')), ) @@ -2371,6 +2359,7 @@ class UnifiedJobAccess(BaseAccess): prefetch_related = ( 'created_by', 'modified_by', + 'organization', 'unified_job_node__workflow_job', 'unified_job_template', 'instance_group', @@ -2401,8 +2390,7 @@ class UnifiedJobAccess(BaseAccess): Q(unified_job_template_id__in=UnifiedJobTemplate.accessible_pk_qs(self.user, 'read_role')) | Q(inventoryupdate__inventory_source__inventory__id__in=inv_pk_qs) | Q(adhoccommand__inventory__id__in=inv_pk_qs) | - Q(job__inventory__organization__in=org_auditor_qs) | - Q(job__project__organization__in=org_auditor_qs) + Q(organization__in=org_auditor_qs) ) return qs diff --git a/awx/main/fields.py b/awx/main/fields.py index d395803c7c..1038673eb2 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -56,7 +56,8 @@ from awx.main import utils __all__ = ['AutoOneToOneField', 'ImplicitRoleField', 'JSONField', 'SmartFilterField', 'OrderedManyToManyField', - 'update_role_parentage_for_instance', 'is_implicit_parent'] + 'update_role_parentage_for_instance', + 'is_implicit_parent'] # Provide a (better) custom error message for enum jsonschema validation @@ -140,8 +141,9 @@ def resolve_role_field(obj, field): return [] if len(field_components) == 1: - role_cls = str(utils.get_current_apps().get_model('main', 'Role')) - if not str(type(obj)) == role_cls: + # use extremely generous duck typing to accomidate all possible forms + # of the model that may be used during various migrations + if obj._meta.model_name != 'role' or obj._meta.app_label != 'main': raise Exception(smart_text('{} refers to a {}, not a Role'.format(field, type(obj)))) ret.append(obj.id) else: @@ -197,18 +199,30 @@ def update_role_parentage_for_instance(instance): updates the parents listing for all the roles of a given instance if they have changed ''' + changed_ct = 0 for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): + changed = False cur_role = getattr(instance, implicit_role_field.name) original_parents = set(json.loads(cur_role.implicit_parents)) new_parents = implicit_role_field._resolve_parent_roles(instance) - cur_role.parents.remove(*list(original_parents - new_parents)) - cur_role.parents.add(*list(new_parents - original_parents)) + removals = original_parents - new_parents + if removals: + changed = True + cur_role.parents.remove(*list(removals)) + additions = new_parents - original_parents + if additions: + changed = True + cur_role.parents.add(*list(additions)) new_parents_list = list(new_parents) new_parents_list.sort() new_parents_json = json.dumps(new_parents_list) if cur_role.implicit_parents != new_parents_json: + changed = True cur_role.implicit_parents = new_parents_json cur_role.save() + if changed: + changed_ct += 1 + return changed_ct class ImplicitRoleDescriptor(ForwardManyToOneDescriptor): @@ -256,20 +270,18 @@ class ImplicitRoleField(models.ForeignKey): field_names = [field_names] for field_name in field_names: - # Handle the OR syntax for role parents - if type(field_name) == tuple: - continue - - if type(field_name) == bytes: - field_name = field_name.decode('utf-8') if field_name.startswith('singleton:'): continue field_name, sep, field_attr = field_name.partition('.') - field = getattr(cls, field_name) + # Non existent fields will occur if ever a parent model is + # moved inside a migration, needed for job_template_organization_field + # migration in particular + # consistency is assured by unit test awx.main.tests.functional + field = getattr(cls, field_name, None) - if type(field) is ReverseManyToOneDescriptor or \ + if field and type(field) is ReverseManyToOneDescriptor or \ type(field) is ManyToManyDescriptor: if '.' in field_attr: diff --git a/awx/main/middleware.py b/awx/main/middleware.py index d79eb06af9..8382b81408 100644 --- a/awx/main/middleware.py +++ b/awx/main/middleware.py @@ -192,21 +192,41 @@ class URLModificationMiddleware(MiddlewareMixin): ) super().__init__(get_response) - def _named_url_to_pk(self, node, named_url): - kwargs = {} - if not node.populate_named_url_query_kwargs(kwargs, named_url): - return named_url - return str(get_object_or_404(node.model, **kwargs).pk) + @staticmethod + def _hijack_for_old_jt_name(node, kwargs, named_url): + try: + int(named_url) + return False + except ValueError: + pass + JobTemplate = node.model + name = urllib.parse.unquote(named_url) + return JobTemplate.objects.filter(name=name).order_by('organization__created').first() - def _convert_named_url(self, url_path): + @classmethod + def _named_url_to_pk(cls, node, resource, named_url): + kwargs = {} + if node.populate_named_url_query_kwargs(kwargs, named_url): + return str(get_object_or_404(node.model, **kwargs).pk) + if resource == 'job_templates' and '++' not in named_url: + # special case for deprecated job template case + # will not raise a 404 on its own + jt = cls._hijack_for_old_jt_name(node, kwargs, named_url) + if jt: + return str(jt.pk) + return named_url + + @classmethod + def _convert_named_url(cls, url_path): url_units = url_path.split('/') # If the identifier is an empty string, it is always invalid. if len(url_units) < 6 or url_units[1] != 'api' or url_units[2] not in ['v2'] or not url_units[4]: return url_path resource = url_units[3] if resource in settings.NAMED_URL_MAPPINGS: - url_units[4] = self._named_url_to_pk(settings.NAMED_URL_GRAPH[settings.NAMED_URL_MAPPINGS[resource]], - url_units[4]) + url_units[4] = cls._named_url_to_pk( + settings.NAMED_URL_GRAPH[settings.NAMED_URL_MAPPINGS[resource]], + resource, url_units[4]) return '/'.join(url_units) def process_request(self, request): diff --git a/awx/main/migrations/0085_v360_job_template_organization_field.py b/awx/main/migrations/0085_v360_job_template_organization_field.py new file mode 100644 index 0000000000..a454083e98 --- /dev/null +++ b/awx/main/migrations/0085_v360_job_template_organization_field.py @@ -0,0 +1,72 @@ +# Generated by Django 2.2.4 on 2019-08-07 19:56 + +import awx.main.utils.polymorphic +import awx.main.fields +from django.db import migrations, models +import django.db.models.deletion + +from awx.main.migrations._rbac import rebuild_role_parentage, migrate_ujt_organization, migrate_ujt_organization_backward + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0084_v360_token_description'), + ] + + operations = [ + # backwards parents and ancestors caching + migrations.RunPython(migrations.RunPython.noop, rebuild_role_parentage), + # add new organization field for JT and all other unified jobs + migrations.AddField( + model_name='unifiedjob', + name='tmp_organization', + field=models.ForeignKey(blank=True, help_text='The organization used to determine access to this unified job.', null=True, on_delete=awx.main.utils.polymorphic.SET_NULL, related_name='unifiedjobs', to='main.Organization'), + ), + migrations.AddField( + model_name='unifiedjobtemplate', + name='tmp_organization', + field=models.ForeignKey(blank=True, help_text='The organization used to determine access to this template.', null=True, on_delete=awx.main.utils.polymorphic.SET_NULL, related_name='unifiedjobtemplates', to='main.Organization'), + ), + # while new and old fields exist, copy the organization fields + migrations.RunPython(migrate_ujt_organization, migrate_ujt_organization_backward), + # with data saved, remove old fields + migrations.RemoveField( + model_name='project', + name='organization', + ), + migrations.RemoveField( + model_name='workflowjobtemplate', + name='organization', + ), + # now, without safely rename the new field without conflicts from old field + migrations.RenameField( + model_name='unifiedjobtemplate', + old_name='tmp_organization', + new_name='organization', + ), + migrations.RenameField( + model_name='unifiedjob', + old_name='tmp_organization', + new_name='organization', + ), + # parentage of job template roles has genuinely changed at this point + migrations.AlterField( + model_name='jobtemplate', + name='admin_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['organization.job_template_admin_role'], related_name='+', to='main.Role'), + ), + migrations.AlterField( + model_name='jobtemplate', + name='execute_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['admin_role', 'organization.execute_role'], related_name='+', to='main.Role'), + ), + migrations.AlterField( + model_name='jobtemplate', + name='read_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['organization.auditor_role', 'inventory.organization.auditor_role', 'execute_role', 'admin_role'], related_name='+', to='main.Role'), + ), + # Re-compute the role parents and ancestors caching + # this may be a no-op because field post_save hooks from migrate_jt_organization + migrations.RunPython(rebuild_role_parentage, migrations.RunPython.noop), + ] diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 9b85c71086..0b052c2350 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -1,6 +1,9 @@ import logging from time import time +from django.db.models import Subquery, OuterRef + +from awx.main.fields import update_role_parentage_for_instance from awx.main.models.rbac import Role, batch_role_ancestor_rebuilding logger = logging.getLogger('rbac_migrations') @@ -10,11 +13,11 @@ def create_roles(apps, schema_editor): ''' Implicit role creation happens in our post_save hook for all of our resources. Here we iterate through all of our resource types and call - .save() to ensure all that happens for every object in the system before we - get busy with the actual migration work. + .save() to ensure all that happens for every object in the system. - This gets run after migrate_users, which does role creation for users a - little differently. + This can be used whenever new roles are introduced in a migration to + create those roles for pre-existing objects that did not previously + have them created via signals. ''' models = [ @@ -35,7 +38,118 @@ def create_roles(apps, schema_editor): obj.save() +def delete_all_user_roles(apps, schema_editor): + ContentType = apps.get_model('contenttypes', "ContentType") + Role = apps.get_model('main', "Role") + User = apps.get_model('auth', "User") + user_content_type = ContentType.objects.get_for_model(User) + for role in Role.objects.filter(content_type=user_content_type).iterator(): + role.delete() + + +UNIFIED_ORG_LOOKUPS = { + # Job Templates had an implicit organization via their project + 'jobtemplate': 'project', + # Inventory Sources had an implicit organization via their inventory + 'inventorysource': 'inventory', + # Projects had an explicit organization in their subclass table + 'project': None, + # Workflow JTs also had an explicit organization in their subclass table + 'workflowjobtemplate': None, + # Jobs inherited project from job templates as a convenience field + 'job': 'project', + # Inventory Sources had an convenience field of inventory + 'inventoryupdate': 'inventory', + # Project Updates did not have a direct organization field, obtained it from project + 'projectupdate': 'project', + # Workflow Jobs are handled same as project updates + # Sliced jobs are a special case, but old data is not given special treatment for simplicity + 'workflowjob': 'workflow_job_template', + # AdHocCommands do not have a template, but still migrate them + 'adhoccommand': 'inventory' +} + + +def implicit_org_subquery(UnifiedClass, cls, backward=False): + """Returns a subquery that returns the so-called organization for objects + in the class in question, before migration to the explicit unified org field. + In some cases, this can still be applied post-migration. + """ + if cls._meta.model_name not in UNIFIED_ORG_LOOKUPS: + return None + cls_name = cls._meta.model_name + source_field = UNIFIED_ORG_LOOKUPS[cls_name] + + unified_field = UnifiedClass._meta.get_field(cls_name) + unified_ptr = unified_field.remote_field.name + if backward: + qs = UnifiedClass.objects.filter(**{cls_name: OuterRef('id')}).order_by().values_list('tmp_organization')[:1] + elif source_field is None: + qs = cls.objects.filter(**{unified_ptr: OuterRef('id')}).order_by().values_list('organization')[:1] + else: + intermediary_field = cls._meta.get_field(source_field) + intermediary_model = intermediary_field.related_model + intermediary_reverse_rel = intermediary_field.remote_field.name + qs = intermediary_model.objects.filter(**{ + # this filter leverages the fact that the Unified models have same pk as subclasses. + # For instance... filters projects used in job template, where that job template + # has same id same as UJT from the outer reference (which it does) + intermediary_reverse_rel: OuterRef('id')} + ).order_by().values_list('organization')[:1] + return Subquery(qs) + + +def _migrate_unified_organization(apps, unified_cls_name, backward=False): + """Given a unified base model (either UJT or UJ) + and a dict org_field_mapping which gives related model to get org from + saves organization for those objects to the temporary migration + variable tmp_organization on the unified model + (optimized method) + """ + start = time() + UnifiedClass = apps.get_model('main', unified_cls_name) + ContentType = apps.get_model('contenttypes', 'ContentType') + + for cls in UnifiedClass.__subclasses__(): + cls_name = cls._meta.model_name + if backward and UNIFIED_ORG_LOOKUPS.get(cls_name, 'not-found') is not None: + logger.debug('Not reverse migrating {}, existing data should remain valid'.format(cls_name)) + continue + logger.debug('Migrating {} to new organization field'.format(cls_name)) + + sub_qs = implicit_org_subquery(UnifiedClass, cls, backward=backward) + if sub_qs is None: + logger.debug('Class {} has no organization migration'.format(cls_name)) + continue + + this_ct = ContentType.objects.get_for_model(cls) + if backward: + r = cls.objects.order_by().update(organization=sub_qs) + else: + r = UnifiedClass.objects.order_by().filter(polymorphic_ctype=this_ct).update(tmp_organization=sub_qs) + if r: + logger.info('Organization migration on {} affected {} rows.'.format(cls_name, r)) + logger.info('Unified organization migration completed in %f seconds' % (time() - start)) + + +def migrate_ujt_organization(apps, schema_editor): + '''Move organization field to UJT and UJ models''' + _migrate_unified_organization(apps, 'UnifiedJobTemplate') + _migrate_unified_organization(apps, 'UnifiedJob') + + +def migrate_ujt_organization_backward(apps, schema_editor): + '''Move organization field from UJT and UJ models back to their original places''' + _migrate_unified_organization(apps, 'UnifiedJobTemplate', backward=True) + _migrate_unified_organization(apps, 'UnifiedJob', backward=True) + + def rebuild_role_hierarchy(apps, schema_editor): + ''' + This should be called in any migration when ownerships are changed. + Ex. I remove a user from the admin_role of a credential. + Ancestors are cached from parents for performance, this re-computes ancestors. + ''' logger.info('Computing role roots..') start = time() roots = Role.objects \ @@ -46,14 +160,57 @@ def rebuild_role_hierarchy(apps, schema_editor): start = time() Role.rebuild_role_ancestor_list(roots, []) stop = time() - logger.info('Rebuild completed in %f seconds' % (stop - start)) + logger.info('Rebuild ancestors completed in %f seconds' % (stop - start)) logger.info('Done.') -def delete_all_user_roles(apps, schema_editor): - ContentType = apps.get_model('contenttypes', "ContentType") +def rebuild_role_parentage(apps, schema_editor): + ''' + This should be called in any migration when any parent_role entry + is modified so that the cached parent fields will be updated. Ex: + foo_role = ImplicitRoleField( + parent_role=['bar_role'] # change to parent_role=['admin_role'] + ) + + This is like rebuild_role_hierarchy, but that method updates ancestors, + whereas this method updates parents. + ''' + start = time() + seen_models = set() + updated_ct = 0 + model_ct = 0 + noop_ct = 0 Role = apps.get_model('main', "Role") - User = apps.get_model('auth', "User") - user_content_type = ContentType.objects.get_for_model(User) - for role in Role.objects.filter(content_type=user_content_type).iterator(): - role.delete() + for role in Role.objects.iterator(): + if not role.object_id: + noop_ct += 1 + continue + model_tuple = (role.content_type_id, role.object_id) + if model_tuple in seen_models: + continue + seen_models.add(model_tuple) + + # The GenericForeignKey does not work right in migrations + # with the usage as role.content_object + # so we do the lookup ourselves with current migration models + ct = role.content_type + app = ct.app_label + ct_model = apps.get_model(app, ct.model) + content_object = ct_model.objects.get(pk=role.object_id) + + updated = update_role_parentage_for_instance(content_object) + if updated: + model_ct += 1 + logger.debug('Updated parents of {} roles of {}'.format(updated, content_object)) + else: + noop_ct += 1 + updated_ct += updated + + logger.debug('No changes to role parents for {} roles'.format(noop_ct)) + if updated_ct: + logger.info('Updated parentage for {} roles of {} resources'.format(updated_ct, model_ct)) + + logger.info('Rebuild parentage completed in %f seconds' % (time() - start)) + + if updated_ct: + rebuild_role_hierarchy(apps, schema_editor) diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 9ced56cc4a..d94b428581 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -426,9 +426,9 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin): ''' def _get_related_jobs(self): return UnifiedJob.objects.non_polymorphic().filter( - Q(Job___inventory=self) | - Q(InventoryUpdate___inventory_source__inventory=self) | - Q(AdHocCommand___inventory=self) + Q(job__inventory=self) | + Q(inventoryupdate__inventory=self) | + Q(adhoccommand__inventory=self) ) @@ -808,8 +808,8 @@ class Group(CommonModelNameNotUnique, RelatedJobsMixin): ''' def _get_related_jobs(self): return UnifiedJob.objects.non_polymorphic().filter( - Q(Job___inventory=self.inventory) | - Q(InventoryUpdate___inventory_source__groups=self) + Q(job__inventory=self.inventory) | + Q(inventoryupdate__inventory_source__groups=self) ) @@ -1277,10 +1277,14 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions, CustomVirtualE @classmethod def _get_unified_job_field_names(cls): return set(f.name for f in InventorySourceOptions._meta.fields) | set( - ['name', 'description', 'credentials', 'inventory'] + ['name', 'description', 'organization', 'credentials', 'inventory'] ) def save(self, *args, **kwargs): + # if this is a new object, inherit organization from its inventory + if not self.pk and self.inventory and self.inventory.organization_id and not self.organization_id: + self.organization_id = self.inventory.organization_id + # If update_fields has been specified, add our field names to it, # if it hasn't been specified, then we're just doing a normal save. update_fields = kwargs.get('update_fields', []) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 77b3871626..829720afa1 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -199,7 +199,7 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour 'labels', 'instance_groups', 'credentials', 'survey_spec' ] FIELDS_TO_DISCARD_AT_COPY = ['vault_credential', 'credential'] - SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name')] + SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name', 'organization')] class Meta: app_label = 'main' @@ -262,13 +262,17 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour ) admin_role = ImplicitRoleField( - parent_role=['project.organization.job_template_admin_role', 'inventory.organization.job_template_admin_role'] + parent_role=['organization.job_template_admin_role'] ) execute_role = ImplicitRoleField( - parent_role=['admin_role', 'project.organization.execute_role', 'inventory.organization.execute_role'], + parent_role=['admin_role', 'organization.execute_role'], ) read_role = ImplicitRoleField( - parent_role=['project.organization.auditor_role', 'inventory.organization.auditor_role', 'execute_role', 'admin_role'], + parent_role=[ + 'organization.auditor_role', + 'inventory.organization.auditor_role', # partial support for old inheritance via inventory + 'execute_role', 'admin_role' + ], ) @@ -279,7 +283,7 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour @classmethod def _get_unified_job_field_names(cls): return set(f.name for f in JobOptions._meta.fields) | set( - ['name', 'description', 'survey_passwords', 'labels', 'credentials', + ['name', 'description', 'organization', 'survey_passwords', 'labels', 'credentials', 'job_slice_number', 'job_slice_count'] ) @@ -479,13 +483,13 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour success_notification_templates = list(base_notification_templates.filter( unifiedjobtemplate_notification_templates_for_success__in=[self, self.project])) # Get Organization NotificationTemplates - if self.project is not None and self.project.organization is not None: + if self.organization is not None: error_notification_templates = set(error_notification_templates + list(base_notification_templates.filter( - organization_notification_templates_for_errors=self.project.organization))) + organization_notification_templates_for_errors=self.organization))) started_notification_templates = set(started_notification_templates + list(base_notification_templates.filter( - organization_notification_templates_for_started=self.project.organization))) + organization_notification_templates_for_started=self.organization))) success_notification_templates = set(success_notification_templates + list(base_notification_templates.filter( - organization_notification_templates_for_success=self.project.organization))) + organization_notification_templates_for_success=self.organization))) return dict(error=list(error_notification_templates), started=list(started_notification_templates), success=list(success_notification_templates)) @@ -588,7 +592,7 @@ class Job(UnifiedJob, JobOptions, SurveyJobMixin, JobNotificationMixin, TaskMana for virtualenv in ( self.job_template.custom_virtualenv if self.job_template else None, self.project.custom_virtualenv, - self.project.organization.custom_virtualenv if self.project.organization else None + self.organization.custom_virtualenv if self.organization else None ): if virtualenv: return virtualenv @@ -741,8 +745,8 @@ class Job(UnifiedJob, JobOptions, SurveyJobMixin, JobNotificationMixin, TaskMana @property def preferred_instance_groups(self): - if self.project is not None and self.project.organization is not None: - organization_groups = [x for x in self.project.organization.instance_groups.all()] + if self.organization is not None: + organization_groups = [x for x in self.organization.instance_groups.all()] else: organization_groups = [] if self.inventory is not None: @@ -1144,7 +1148,7 @@ class SystemJobTemplate(UnifiedJobTemplate, SystemJobOptions): @classmethod def _get_unified_job_field_names(cls): - return ['name', 'description', 'job_type', 'extra_vars'] + return ['name', 'description', 'organization', 'job_type', 'extra_vars'] def get_absolute_url(self, request=None): return reverse('api:system_job_template_detail', kwargs={'pk': self.pk}, request=request) diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index df5d491d20..23ce65f5e9 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -6,7 +6,6 @@ # Django from django.conf import settings from django.db import models -from django.db.models import Q from django.contrib.auth.models import User from django.contrib.sessions.models import Session from django.utils.timezone import now as tz_now @@ -106,12 +105,7 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi RelatedJobsMixin ''' def _get_related_jobs(self): - project_ids = self.projects.all().values_list('id') - return UnifiedJob.objects.non_polymorphic().filter( - Q(Job___project__in=project_ids) | - Q(ProjectUpdate___project__in=project_ids) | - Q(InventoryUpdate___inventory_source__inventory__organization=self) - ) + return UnifiedJob.objects.non_polymorphic().filter(organization=self) class Team(CommonModelNameNotUnique, ResourceMixin): diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index cc31842d4d..c9bf9762dd 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -254,13 +254,6 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn app_label = 'main' ordering = ('id',) - organization = models.ForeignKey( - 'Organization', - blank=True, - null=True, - on_delete=models.CASCADE, - related_name='projects', - ) scm_update_on_launch = models.BooleanField( default=False, help_text=_('Update the project when a job is launched that uses the project.'), @@ -329,7 +322,7 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn @classmethod def _get_unified_job_field_names(cls): return set(f.name for f in ProjectOptions._meta.fields) | set( - ['name', 'description'] + ['name', 'description', 'organization'] ) def save(self, *args, **kwargs): @@ -450,8 +443,8 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn ''' def _get_related_jobs(self): return UnifiedJob.objects.non_polymorphic().filter( - models.Q(Job___project=self) | - models.Q(ProjectUpdate___project=self) + models.Q(job__project=self) | + models.Q(projectupdate__project=self) ) def delete(self, *args, **kwargs): diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 1b17291fba..1c2c6ffeed 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -157,6 +157,14 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio default='ok', editable=False, ) + organization = models.ForeignKey( + 'Organization', + blank=True, + null=True, + on_delete=polymorphic.SET_NULL, + related_name='%(class)ss', + help_text=_('The organization used to determine access to this template.'), + ) credentials = models.ManyToManyField( 'Credential', related_name='%(class)ss', @@ -700,6 +708,14 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique on_delete=polymorphic.SET_NULL, help_text=_('The Rampart/Instance group the job was run under'), ) + organization = models.ForeignKey( + 'Organization', + blank=True, + null=True, + on_delete=polymorphic.SET_NULL, + related_name='%(class)ss', + help_text=_('The organization used to determine access to this unified job.'), + ) credentials = models.ManyToManyField( 'Credential', related_name='%(class)ss', diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index ff75f260d7..df4772a20d 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -335,7 +335,7 @@ class WorkflowJobOptions(LaunchTimeConfigBase): @classmethod def _get_unified_job_field_names(cls): r = set(f.name for f in WorkflowJobOptions._meta.fields) | set( - ['name', 'description', 'survey_passwords', 'labels', 'limit', 'scm_branch'] + ['name', 'description', 'organization', 'survey_passwords', 'labels', 'limit', 'scm_branch'] ) r.remove('char_prompts') # needed due to copying launch config to launch config return r @@ -382,13 +382,6 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl class Meta: app_label = 'main' - organization = models.ForeignKey( - 'Organization', - blank=True, - null=True, - on_delete=models.SET_NULL, - related_name='workflows', - ) ask_inventory_on_launch = AskForField( blank=True, default=False, diff --git a/awx/main/signals.py b/awx/main/signals.py index bc39fa85d0..27a6426eba 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -183,7 +183,6 @@ def connect_computed_field_signals(): connect_computed_field_signals() -post_save.connect(save_related_job_templates, sender=Project) post_save.connect(save_related_job_templates, sender=Inventory) m2m_changed.connect(rebuild_role_ancestor_list, Role.parents.through) m2m_changed.connect(rbac_activity_stream, Role.members.through) diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py index 2f8cbe6934..0952738174 100644 --- a/awx/main/tests/factories/fixtures.py +++ b/awx/main/tests/factories/fixtures.py @@ -159,7 +159,8 @@ def mk_job_template(name, job_type='run', extra_vars = json.dumps(extra_vars) jt = JobTemplate(name=name, job_type=job_type, extra_vars=extra_vars, - webhook_service=webhook_service, playbook='helloworld.yml') + webhook_service=webhook_service, playbook='helloworld.yml', + organization=organization) jt.inventory = inventory if jt.inventory is None: diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py index bfa7f9fc1b..dd412571e1 100644 --- a/awx/main/tests/factories/tower.py +++ b/awx/main/tests/factories/tower.py @@ -255,7 +255,7 @@ def create_job_template(name, roles=None, persisted=True, webhook_service='', ** jt = mk_job_template(name, project=proj, inventory=inv, credential=cred, network_credential=net_cred, cloud_credential=cloud_cred, job_type=job_type, spec=spec, extra_vars=extra_vars, - persisted=persisted, webhook_service=webhook_service) + persisted=persisted, webhook_service=webhook_service, organization=org) if 'jobs' in kwargs: for i in kwargs['jobs']: diff --git a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py index 880b7ff892..b837af5a84 100644 --- a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py +++ b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py @@ -278,6 +278,7 @@ def test_multi_vault_preserved_on_put(get, put, admin_user, job_template, vault_ job_template.credentials.add(vault_credential, vault2) assert job_template.credentials.count() == 2 # sanity check r = get(job_template.get_absolute_url(), admin_user, expect=200) + r.data.pop('organization') # so that it passes validation # should be a no-op PUT request put( job_template.get_absolute_url(), diff --git a/awx/main/tests/functional/api/test_job.py b/awx/main/tests/functional/api/test_job.py index 0692d70ffb..683f62dc43 100644 --- a/awx/main/tests/functional/api/test_job.py +++ b/awx/main/tests/functional/api/test_job.py @@ -39,6 +39,26 @@ def test_extra_credentials(get, organization_factory, job_template_factory, cred @pytest.mark.django_db def test_job_relaunch_permission_denied_response( post, get, inventory, project, credential, net_credential, machine_credential): + jt = JobTemplate.objects.create(name='testjt', inventory=inventory, project=project, ask_credential_on_launch=True) + jt.credentials.add(machine_credential) + jt_user = User.objects.create(username='jobtemplateuser') + jt.execute_role.members.add(jt_user) + with impersonate(jt_user): + job = jt.create_unified_job() + + # User capability is shown for this + r = get(job.get_absolute_url(), jt_user, expect=200) + assert r.data['summary_fields']['user_capabilities']['start'] + + # Job has prompted extra_credential, launch denied w/ message + job.launch_config.credentials.add(net_credential) + r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403) + assert 'launched with prompted fields which you do not have access to' in r.data['detail'] + + +@pytest.mark.django_db +def test_job_relaunch_prompts_not_accepted_response( + post, get, inventory, project, credential, net_credential, machine_credential): jt = JobTemplate.objects.create(name='testjt', inventory=inventory, project=project) jt.credentials.add(machine_credential) jt_user = User.objects.create(username='jobtemplateuser') @@ -53,8 +73,7 @@ def test_job_relaunch_permission_denied_response( # Job has prompted extra_credential, launch denied w/ message job.launch_config.credentials.add(net_credential) r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403) - assert 'launched with prompted fields' in r.data['detail'] - assert 'do not have permission' in r.data['detail'] + assert 'no longer accepts the prompts provided for this job' in r.data['detail'] @pytest.mark.django_db @@ -201,7 +220,8 @@ def test_block_unprocessed_events(delete, admin_user, mocker): def test_block_related_unprocessed_events(mocker, organization, project, delete, admin_user): job_template = JobTemplate.objects.create( project=project, - playbook='helloworld.yml' + playbook='helloworld.yml', + organization=organization ) time_of_finish = parse("Thu Feb 23 14:17:24 2012 -0500") Job.objects.create( @@ -209,7 +229,8 @@ def test_block_related_unprocessed_events(mocker, organization, project, delete, status='finished', finished=time_of_finish, job_template=job_template, - project=project + project=project, + organization=organization ) view = RelatedJobsPreventDeleteMixin() time_of_request = time_of_finish + relativedelta(seconds=2) diff --git a/awx/main/tests/functional/api/test_job_template.py b/awx/main/tests/functional/api/test_job_template.py index 8ad864ee8e..2e56d8253b 100644 --- a/awx/main/tests/functional/api/test_job_template.py +++ b/awx/main/tests/functional/api/test_job_template.py @@ -6,7 +6,7 @@ import pytest # AWX from awx.api.serializers import JobTemplateSerializer from awx.api.versioning import reverse -from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate +from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate, Organization from awx.main.migrations import _save_password_keys as save_password_keys # Django @@ -30,16 +30,55 @@ def test_create(post, project, machine_credential, inventory, alice, grant_proje project.use_role.members.add(alice) if grant_inventory: inventory.use_role.members.add(alice) + project.organization.job_template_admin_role.members.add(alice) r = post(reverse('api:job_template_list'), { 'name': 'Some name', 'project': project.id, 'inventory': inventory.id, 'playbook': 'helloworld.yml', + 'organization': project.organization_id }, alice) assert r.status_code == expect +@pytest.mark.django_db +def test_creation_uniqueness_rules(post, project, inventory, admin_user): + orgA = Organization.objects.create(name='orga') + orgB = Organization.objects.create(name='orgb') + create_data = { + 'name': 'this_unique_name', + 'project': project.pk, + 'inventory': inventory.pk, + 'playbook': 'helloworld.yml', + 'organization': orgA.pk + } + post( + url=reverse('api:job_template_list'), + data=create_data, + user=admin_user, + expect=201 + ) + r = post( + url=reverse('api:job_template_list'), + data=create_data, + user=admin_user, + expect=400 + ) + msg = str(r.data['__all__'][0]) + assert "JobTemplate with this (" in msg + assert ") combination already exists" in msg + + # can create JT with same name, only if it is in different org + create_data['organization'] = orgB.pk + post( + url=reverse('api:job_template_list'), + data=create_data, + user=admin_user, + expect=201 + ) + + @pytest.mark.django_db def test_extra_credential_creation(get, post, organization_factory, job_template_factory, credentialtype_aws): objs = organization_factory("org", superusers=['admin']) @@ -524,13 +563,14 @@ def test_callback_disallowed_null_inventory(project): @pytest.mark.django_db -def test_job_template_branch_error(project, inventory, post, admin_user): +def test_job_template_branch_error(project, inventory, organization, post, admin_user): r = post( url=reverse('api:job_template_list'), data={ "name": "fooo", "inventory": inventory.pk, "project": project.pk, + "organization": organization.pk, "playbook": "helloworld.yml", "scm_branch": "foobar" }, @@ -541,13 +581,14 @@ def test_job_template_branch_error(project, inventory, post, admin_user): @pytest.mark.django_db -def test_job_template_branch_prompt_error(project, inventory, post, admin_user): +def test_job_template_branch_prompt_error(project, inventory, post, organization, admin_user): r = post( url=reverse('api:job_template_list'), data={ "name": "fooo", "inventory": inventory.pk, "project": project.pk, + "organization": organization.pk, "playbook": "helloworld.yml", "ask_scm_branch_on_launch": True }, diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 9c4f536b09..f5221ef5f1 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -2,6 +2,8 @@ import pytest from awx.api.versioning import reverse +from awx.main.models import Project + @pytest.fixture def organization_resource_creator(organization, user): @@ -19,21 +21,26 @@ def organization_resource_creator(organization, user): for i in range(inventories): inventory = organization.inventories.create(name="associated-inv %s" % i) for i in range(projects): - organization.projects.create(name="test-proj %s" % i, - description="test-proj-desc") + Project.objects.create( + name="test-proj %s" % i, + description="test-proj-desc", + organization=organization + ) # Mix up the inventories and projects used by the job templates i_proj = 0 i_inv = 0 for i in range(job_templates): - project = organization.projects.all()[i_proj] + project = Project.objects.filter(organization=organization)[i_proj] + # project = organization.projects.all()[i_proj] inventory = organization.inventories.all()[i_inv] project.jobtemplates.create(name="test-jt %s" % i, description="test-job-template-desc", inventory=inventory, - playbook="test_playbook.yml") + playbook="test_playbook.yml", + organization=organization) i_proj += 1 i_inv += 1 - if i_proj >= organization.projects.count(): + if i_proj >= Project.objects.filter(organization=organization).count(): i_proj = 0 if i_inv >= organization.inventories.count(): i_inv = 0 @@ -179,12 +186,14 @@ def test_scan_JT_counted(resourced_organization, user, get): @pytest.mark.django_db def test_JT_not_double_counted(resourced_organization, user, get): admin_user = user('admin', True) + proj = Project.objects.filter(organization=resourced_organization).all()[0] # Add a run job template to the org - resourced_organization.projects.all()[0].jobtemplates.create( + proj.jobtemplates.create( job_type='run', inventory=resourced_organization.inventories.all()[0], - project=resourced_organization.projects.all()[0], - name='double-linked-job-template') + project=proj, + name='double-linked-job-template', + organization=resourced_organization) counts_dict = COUNTS_PRIMES counts_dict['job_templates'] += 1 @@ -197,38 +206,3 @@ def test_JT_not_double_counted(resourced_organization, user, get): detail_response = get(reverse('api:organization_detail', kwargs={'pk': resourced_organization.pk}), admin_user) assert detail_response.status_code == 200 assert detail_response.data['summary_fields']['related_field_counts'] == counts_dict - - -@pytest.mark.django_db -def test_JT_associated_with_project(organizations, project, user, get): - # Check that adding a project to an organization gets the project's JT - # included in the organization's JT count - external_admin = user('admin', True) - two_orgs = organizations(2) - organization = two_orgs[0] - other_org = two_orgs[1] - - unrelated_inv = other_org.inventories.create(name='not-in-organization') - organization.projects.add(project) - project.jobtemplates.create(name="test-jt", - description="test-job-template-desc", - inventory=unrelated_inv, - playbook="test_playbook.yml") - - response = get(reverse('api:organization_list'), external_admin) - assert response.status_code == 200 - - org_id = organization.id - counts = {} - for org_json in response.data['results']: - working_id = org_json['id'] - counts[working_id] = org_json['summary_fields']['related_field_counts'] - - assert counts[org_id] == { - 'users': 0, - 'admins': 0, - 'job_templates': 1, - 'projects': 1, - 'inventories': 0, - 'teams': 0 - } diff --git a/awx/main/tests/functional/api/test_rbac_displays.py b/awx/main/tests/functional/api/test_rbac_displays.py index 4180647d44..c3dd65d9c4 100644 --- a/awx/main/tests/functional/api/test_rbac_displays.py +++ b/awx/main/tests/functional/api/test_rbac_displays.py @@ -61,7 +61,7 @@ class TestJobTemplateCopyEdit: def jt_copy_edit(self, job_template_factory, project): objects = job_template_factory( 'copy-edit-job-template', - project=project) + project=project, organization=project.organization) return objects.job_template def fake_context(self, user): @@ -129,9 +129,8 @@ class TestJobTemplateCopyEdit: # random user given JT and project admin abilities jt_copy_edit.admin_role.members.add(rando) - jt_copy_edit.save() jt_copy_edit.project.admin_role.members.add(rando) - jt_copy_edit.project.save() + jt_copy_edit.organization.job_template_admin_role.members.add(rando) serializer = JobTemplateSerializer(jt_copy_edit, context=self.fake_context(rando)) response = serializer.to_representation(jt_copy_edit) diff --git a/awx/main/tests/functional/api/test_unified_job_template.py b/awx/main/tests/functional/api/test_unified_job_template.py index faae3cce3c..1febd2f50e 100644 --- a/awx/main/tests/functional/api/test_unified_job_template.py +++ b/awx/main/tests/functional/api/test_unified_job_template.py @@ -1,6 +1,8 @@ import pytest from awx.api.versioning import reverse +from awx.main import models +from awx.main.utils import get_type_for_model @pytest.mark.django_db @@ -9,3 +11,111 @@ def test_aliased_forward_reverse_field_searches(instance, options, get, admin): response = options(url, None, admin) assert 'job_template__search' in response.data['related_search_fields'] get(reverse("api:unified_job_template_list") + "?job_template__search=anything", user=admin, expect=200) + + +@pytest.mark.django_db +@pytest.mark.parametrize('model', ( + 'Project', + 'JobTemplate', + 'WorkflowJobTemplate' +)) +class TestUnifiedOrganization: + + def data_for_model(self, model, orm_style=False): + data = { + 'name': 'foo', + 'organization': None + } + if model == 'JobTemplate': + proj = models.Project.objects.create( + name="test-proj", + playbook_files=['helloworld.yml'] + ) + if orm_style: + data['project_id'] = proj.id + else: + data['project'] = proj.id + data['playbook'] = 'helloworld.yml' + data['ask_inventory_on_launch'] = True + return data + + def test_organization_required_on_creation(self, model, admin_user, post): + cls = getattr(models, model) + data = self.data_for_model(model) + r = post( + url=reverse('api:{}_list'.format(get_type_for_model(cls))), + data=data, + user=admin_user, + expect=400 + ) + assert 'organization' in r.data + assert 'required for new object' in r.data['organization'][0] + # Surprising behavior - not providing the key can often give + # different behavior from giving it as null on create + data.pop('organization') + r = post( + url=reverse('api:{}_list'.format(get_type_for_model(cls))), + data=data, + user=admin_user, + expect=400 + ) + assert 'organization' in r.data + assert 'required' in r.data['organization'][0] + + def test_organization_blank_on_edit_of_orphan(self, model, admin_user, patch): + cls = getattr(models, model) + data = self.data_for_model(model, orm_style=True) + obj = cls.objects.create(**data) + patch( + url=obj.get_absolute_url(), + data={'name': 'foooooo'}, + user=admin_user, + expect=200 + ) + obj.refresh_from_db() + assert obj.name == 'foooooo' + + def test_organization_blank_on_edit_of_orphan_as_nonsuperuser(self, model, rando, patch): + """Test case reflects historical bug where ordinary users got weird error + message when editing an orphaned project + """ + cls = getattr(models, model) + data = self.data_for_model(model, orm_style=True) + obj = cls.objects.create(**data) + if model == 'JobTemplate': + obj.project.admin_role.members.add(rando) + obj.admin_role.members.add(rando) + patch( + url=obj.get_absolute_url(), + data={'name': 'foooooo'}, + user=rando, + expect=200 + ) + obj.refresh_from_db() + assert obj.name == 'foooooo' + + def test_organization_blank_on_edit_of_normal(self, model, admin_user, patch, organization): + cls = getattr(models, model) + data = self.data_for_model(model, orm_style=True) + data['organization'] = organization + obj = cls.objects.create(**data) + patch( + url=obj.get_absolute_url(), + data={'name': 'foooooo'}, + user=admin_user, + expect=200 + ) + obj.refresh_from_db() + assert obj.name == 'foooooo' + + def test_organization_cannot_change_to_null(self, model, admin_user, patch, organization): + cls = getattr(models, model) + data = self.data_for_model(model, orm_style=True) + data['organization'] = organization + obj = cls.objects.create(**data) + patch( + url=obj.get_absolute_url(), + data={'organization': None}, + user=admin_user, + expect=400 + ) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 1b680eee8d..cae55c8562 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -75,24 +75,26 @@ def user(): @pytest.fixture -def check_jobtemplate(project, inventory, credential): +def check_jobtemplate(project, inventory, credential, organization): jt = JobTemplate.objects.create( job_type='check', project=project, inventory=inventory, - name='check-job-template' + name='check-job-template', + organization=organization ) jt.credentials.add(credential) return jt @pytest.fixture -def deploy_jobtemplate(project, inventory, credential): +def deploy_jobtemplate(project, inventory, credential, organization): jt = JobTemplate.objects.create( job_type='run', project=project, inventory=inventory, - name='deploy-job-template' + name='deploy-job-template', + organization=organization ) jt.credentials.add(credential) return jt @@ -180,8 +182,8 @@ def project_factory(organization): @pytest.fixture -def job_factory(job_template, admin): - def factory(job_template=job_template, initial_state='new', created_by=admin): +def job_factory(jt_linked, admin): + def factory(job_template=jt_linked, initial_state='new', created_by=admin): return job_template.create_unified_job(_eager_fields={ 'status': initial_state, 'created_by': created_by}) return factory @@ -701,11 +703,8 @@ def ad_hoc_command_factory(inventory, machine_credential, admin): @pytest.fixture -def job_template(organization): - jt = JobTemplate(name='test-job_template') - jt.save() - - return jt +def job_template(): + return JobTemplate.objects.create(name='test-job_template') @pytest.fixture @@ -717,20 +716,16 @@ def job_template_labels(organization, job_template): @pytest.fixture -def jt_linked(job_template_factory, credential, net_credential, vault_credential): +def jt_linked(organization, project, inventory, machine_credential, credential, net_credential, vault_credential): ''' A job template with a reasonably complete set of related objects to test RBAC and other functionality affected by related objects ''' - objects = job_template_factory( - 'testJT', organization='org1', project='proj1', inventory='inventory1', - credential='cred1') - jt = objects.job_template - jt.credentials.add(vault_credential) - jt.save() - # Add AWS cloud credential and network credential - jt.credentials.add(credential) - jt.credentials.add(net_credential) + jt = JobTemplate.objects.create( + project=project, inventory=inventory, playbook='helloworld.yml', + organization=organization + ) + jt.credentials.add(machine_credential, vault_credential, credential, net_credential) return jt diff --git a/awx/main/tests/functional/models/test_activity_stream.py b/awx/main/tests/functional/models/test_activity_stream.py index 0529b5377b..f220641759 100644 --- a/awx/main/tests/functional/models/test_activity_stream.py +++ b/awx/main/tests/functional/models/test_activity_stream.py @@ -12,6 +12,7 @@ from awx.main.models import ( CredentialType, Inventory, InventorySource, + Project, User ) @@ -99,8 +100,8 @@ class TestRolesAssociationEntries: ).count() == 1, 'In loop %s' % i def test_model_associations_are_recorded(self, organization): - proj1 = organization.projects.create(name='proj1') - proj2 = organization.projects.create(name='proj2') + proj1 = Project.objects.create(name='proj1', organization=organization) + proj2 = Project.objects.create(name='proj2', organization=organization) proj2.use_role.parents.add(proj1.admin_role) assert ActivityStream.objects.filter(role=proj1.admin_role, project=proj2).count() == 1 diff --git a/awx/main/tests/functional/models/test_job.py b/awx/main/tests/functional/models/test_job.py index 31b430d268..b097f85548 100644 --- a/awx/main/tests/functional/models/test_job.py +++ b/awx/main/tests/functional/models/test_job.py @@ -29,18 +29,19 @@ def test_prevent_slicing(): @pytest.mark.django_db -def test_awx_custom_virtualenv(inventory, project, machine_credential): +def test_awx_custom_virtualenv(inventory, project, machine_credential, organization): jt = JobTemplate.objects.create( name='my-jt', inventory=inventory, project=project, - playbook='helloworld.yml' + playbook='helloworld.yml', + organization=organization ) jt.credentials.add(machine_credential) job = jt.create_unified_job() - job.project.organization.custom_virtualenv = '/venv/fancy-org' - job.project.organization.save() + job.organization.custom_virtualenv = '/venv/fancy-org' + job.organization.save() assert job.ansible_virtualenv_path == '/venv/fancy-org' job.project.custom_virtualenv = '/venv/fancy-proj' diff --git a/awx/main/tests/functional/models/test_project.py b/awx/main/tests/functional/models/test_project.py index 719c37436e..3f57691ac3 100644 --- a/awx/main/tests/functional/models/test_project.py +++ b/awx/main/tests/functional/models/test_project.py @@ -39,3 +39,9 @@ def test_foreign_key_change_changes_modified_by(project, organization): assert project._get_fields_snapshot()['organization_id'] == organization.id project.organization = Organization(name='foo', pk=41) assert project._get_fields_snapshot()['organization_id'] == 41 + + +@pytest.mark.django_db +def test_project_related_jobs(project): + update = project.create_unified_job() + assert update.id in [u.id for u in project._get_related_jobs()] diff --git a/awx/main/tests/functional/test_copy.py b/awx/main/tests/functional/test_copy.py index 7be582d6c8..747f7754c6 100644 --- a/awx/main/tests/functional/test_copy.py +++ b/awx/main/tests/functional/test_copy.py @@ -11,10 +11,11 @@ from awx.main.tasks import deep_copy_model_obj @pytest.mark.django_db -def test_job_template_copy(post, get, project, inventory, machine_credential, vault_credential, +def test_job_template_copy(post, get, project, inventory, organization, machine_credential, vault_credential, credential, alice, job_template_with_survey_passwords, admin): job_template_with_survey_passwords.project = project job_template_with_survey_passwords.inventory = inventory + job_template_with_survey_passwords.organization = organization job_template_with_survey_passwords.save() job_template_with_survey_passwords.credentials.add(credential) job_template_with_survey_passwords.credentials.add(machine_credential) @@ -22,6 +23,7 @@ def test_job_template_copy(post, get, project, inventory, machine_credential, va job_template_with_survey_passwords.admin_role.members.add(alice) project.admin_role.members.add(alice) inventory.admin_role.members.add(alice) + organization.job_template_admin_role.members.add(alice) assert get( reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}), alice, expect=200 diff --git a/awx/main/tests/functional/test_instances.py b/awx/main/tests/functional/test_instances.py index 404b557227..a2a0a646ec 100644 --- a/awx/main/tests/functional/test_instances.py +++ b/awx/main/tests/functional/test_instances.py @@ -1,7 +1,7 @@ import pytest from unittest import mock -from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate +from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate, Organization from awx.main.models.ha import Instance, InstanceGroup from awx.main.tasks import apply_cluster_membership_policies from awx.api.versioning import reverse @@ -253,7 +253,7 @@ def test_inherited_instance_group_membership(instance_group_factory, default_ins j.inventory = inventory ig_org = instance_group_factory("basicA", [default_instance_group.instances.first()]) ig_inv = instance_group_factory("basicB", [default_instance_group.instances.first()]) - j.project.organization.instance_groups.add(ig_org) + j.organization.instance_groups.add(ig_org) j.inventory.instance_groups.add(ig_inv) assert ig_org in j.preferred_instance_groups assert ig_inv in j.preferred_instance_groups @@ -320,13 +320,14 @@ class TestInstanceGroupOrdering: assert pu.preferred_instance_groups == [ig_tmp, ig_org] def test_job_instance_groups(self, instance_group_factory, inventory, project, default_instance_group): - jt = JobTemplate.objects.create(inventory=inventory, project=project) - job = Job.objects.create(inventory=inventory, job_template=jt, project=project) + org = Organization.objects.create(name='foo') + jt = JobTemplate.objects.create(inventory=inventory, project=project, organization=org) + job = Job.objects.create(inventory=inventory, job_template=jt, project=project, organization=org) assert job.preferred_instance_groups == [default_instance_group] ig_org = instance_group_factory("OrgIstGrp", [default_instance_group.instances.first()]) ig_inv = instance_group_factory("InvIstGrp", [default_instance_group.instances.first()]) ig_tmp = instance_group_factory("TmpIstGrp", [default_instance_group.instances.first()]) - project.organization.instance_groups.add(ig_org) + jt.organization.instance_groups.add(ig_org) inventory.instance_groups.add(ig_inv) assert job.preferred_instance_groups == [ig_inv, ig_org] job.job_template.instance_groups.add(ig_tmp) diff --git a/awx/main/tests/functional/test_named_url.py b/awx/main/tests/functional/test_named_url.py index 6ad6512d48..2f921470a5 100644 --- a/awx/main/tests/functional/test_named_url.py +++ b/awx/main/tests/functional/test_named_url.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- import pytest from django.core.exceptions import ImproperlyConfigured @@ -26,7 +27,7 @@ def setup_module(module): def teardown_module(module): - # settings_registry will be persistent states unless we explicitly clean them up. + # settings_registry will be persistent states unless we explicitly clean them up. settings_registry.unregister('NAMED_URL_FORMATS') settings_registry.unregister('NAMED_URL_GRAPH_NODES') @@ -58,10 +59,25 @@ def test_organization(get, admin_user): @pytest.mark.django_db def test_job_template(get, admin_user): - test_jt = JobTemplate.objects.create(name='test_jt') + test_org = Organization.objects.create(name='test_org') + test_jt = JobTemplate.objects.create(name='test_jt', organization=test_org) url = reverse('api:job_template_detail', kwargs={'pk': test_jt.pk}) response = get(url, user=admin_user, expect=200) - assert response.data['related']['named_url'].endswith('/test_jt/') + assert response.data['related']['named_url'].endswith('/test_jt++test_org/') + + +@pytest.mark.django_db +def test_job_template_old_way(get, admin_user, mocker): + test_org = Organization.objects.create(name='test_org') + test_jt = JobTemplate.objects.create(name='test_jt ♥', organization=test_org) + url = reverse('api:job_template_detail', kwargs={'pk': test_jt.pk}) + + response = get(url, user=admin_user, expect=200) + new_url = response.data['related']['named_url'] + old_url = '/'.join([url.rsplit('/', 2)[0], test_jt.name, '']) + + assert URLModificationMiddleware._convert_named_url(new_url) == url + assert URLModificationMiddleware._convert_named_url(old_url) == url @pytest.mark.django_db diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index 2106c7d3f7..ef4b59630d 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/awx/main/tests/functional/test_projects.py @@ -213,34 +213,14 @@ def test_project_credential_protection(post, put, project, organization, scm_cre }, org_admin, expect=403 ) post( - reverse('api:project_list'), { - 'name': 'should not create', - 'organization':organization.id, + reverse('api:project_list'), { + 'name': 'should not create', + 'organization':organization.id, 'credential': scm_credential.id }, org_admin, expect=403 ) -@pytest.mark.django_db() -def test_create_project_null_organization(post, organization, admin): - post(reverse('api:project_list'), { 'name': 't', 'organization': None}, admin, expect=201) - - -@pytest.mark.django_db() -def test_create_project_null_organization_xfail(post, organization, org_admin): - post(reverse('api:project_list'), { 'name': 't', 'organization': None}, org_admin, expect=403) - - -@pytest.mark.django_db() -def test_patch_project_null_organization(patch, organization, project, admin): - patch(reverse('api:project_detail', kwargs={'pk':project.id,}), { 'name': 't', 'organization': organization.id}, admin, expect=200) - - -@pytest.mark.django_db() -def test_patch_project_null_organization_xfail(patch, project, org_admin): - patch(reverse('api:project_detail', kwargs={'pk':project.id,}), { 'name': 't', 'organization': None}, org_admin, expect=400) - - @pytest.mark.django_db def test_cannot_schedule_manual_project(manual_project, admin_user, post): response = post( diff --git a/awx/main/tests/functional/test_rbac_job.py b/awx/main/tests/functional/test_rbac_job.py index 5dbe797f6c..16ff68e0ef 100644 --- a/awx/main/tests/functional/test_rbac_job.py +++ b/awx/main/tests/functional/test_rbac_job.py @@ -29,7 +29,8 @@ def normal_job(deploy_jobtemplate): return Job.objects.create( job_template=deploy_jobtemplate, project=deploy_jobtemplate.project, - inventory=deploy_jobtemplate.inventory + inventory=deploy_jobtemplate.inventory, + organization=deploy_jobtemplate.organization ) diff --git a/awx/main/tests/functional/test_rbac_job_start.py b/awx/main/tests/functional/test_rbac_job_start.py index d205d992d3..3c6d74a0a8 100644 --- a/awx/main/tests/functional/test_rbac_job_start.py +++ b/awx/main/tests/functional/test_rbac_job_start.py @@ -89,8 +89,8 @@ def test_slice_job(slice_job_factory, rando): @pytest.mark.django_db class TestJobRelaunchAccess: @pytest.fixture - def job_no_prompts(self, machine_credential, inventory): - jt = JobTemplate.objects.create(name='test-job_template', inventory=inventory) + def job_no_prompts(self, machine_credential, inventory, organization): + jt = JobTemplate.objects.create(name='test-job_template', inventory=inventory, organization=organization) jt.credentials.add(machine_credential) return jt.create_unified_job() @@ -119,6 +119,13 @@ class TestJobRelaunchAccess: job_no_prompts.job_template.execute_role.members.add(rando) assert rando.can_access(Job, 'start', job_no_prompts) + def test_orphan_relaunch_via_organization(self, job_no_prompts, rando, organization): + "JT for job has been deleted, relevant organization roles will allow management" + organization.execute_role.members.add(rando) + job_no_prompts.job_template.delete() + job_no_prompts.job_template = None # Django should do this for us, but it does not + assert rando.can_access(Job, 'start', job_no_prompts) + def test_no_relaunch_without_prompted_fields_access(self, job_with_prompts, rando): "Has JT execute_role but no use_role on inventory & credential - deny relaunch" job_with_prompts.job_template.execute_role.members.add(rando) diff --git a/awx/main/tests/functional/test_rbac_job_templates.py b/awx/main/tests/functional/test_rbac_job_templates.py index 26d3628f9e..29910db3ae 100644 --- a/awx/main/tests/functional/test_rbac_job_templates.py +++ b/awx/main/tests/functional/test_rbac_job_templates.py @@ -24,6 +24,29 @@ def test_job_template_access_superuser(check_license, user, deploy_jobtemplate): assert access.can_add({}) +@pytest.mark.django_db +class TestImplicitAccess: + def test_org_execute(self, jt_linked, rando): + assert rando not in jt_linked.execute_role + jt_linked.organization.execute_role.members.add(rando) + assert rando in jt_linked.execute_role + + def test_org_admin(self, jt_linked, rando): + assert rando not in jt_linked.execute_role + jt_linked.organization.job_template_admin_role.members.add(rando) + assert rando in jt_linked.execute_role + + def test_org_auditor(self, jt_linked, rando): + assert rando not in jt_linked.read_role + jt_linked.organization.auditor_role.members.add(rando) + assert rando in jt_linked.read_role + + def test_deprecated_inventory_read(self, jt_linked, rando): + assert rando not in jt_linked.read_role + jt_linked.inventory.organization.execute_role.members.add(rando) + assert rando in jt_linked.read_role + + @pytest.mark.django_db def test_job_template_access_read_level(jt_linked, rando): ssh_cred = jt_linked.machine_credential @@ -45,22 +68,21 @@ def test_job_template_access_read_level(jt_linked, rando): @pytest.mark.django_db def test_job_template_access_use_level(jt_linked, rando): - ssh_cred = jt_linked.machine_credential - vault_cred = jt_linked.vault_credentials[0] - access = JobTemplateAccess(rando) jt_linked.project.use_role.members.add(rando) jt_linked.inventory.use_role.members.add(rando) - ssh_cred.use_role.members.add(rando) - vault_cred.use_role.members.add(rando) - + jt_linked.organization.job_template_admin_role.members.add(rando) proj_pk = jt_linked.project.pk - assert access.can_add(dict(inventory=jt_linked.inventory.pk, project=proj_pk)) - assert access.can_add(dict(credential=ssh_cred.pk, project=proj_pk)) - assert access.can_add(dict(vault_credential=vault_cred.pk, project=proj_pk)) + org_pk = jt_linked.organization_id + + assert access.can_change(jt_linked, {'job_type': 'check', 'project': proj_pk}) + assert access.can_change(jt_linked, {'job_type': 'check', 'inventory': None}) for cred in jt_linked.credentials.all(): - assert not access.can_unattach(jt_linked, cred, 'credentials', {}) + assert access.can_unattach(jt_linked, cred, 'credentials', {}) + + assert access.can_add(dict(inventory=jt_linked.inventory.pk, project=proj_pk, organization=org_pk)) + assert access.can_add(dict(project=proj_pk, organization=org_pk)) @pytest.mark.django_db @@ -69,22 +91,21 @@ def test_job_template_access_admin(role_names, jt_linked, rando): ssh_cred = jt_linked.machine_credential access = JobTemplateAccess(rando) - # Appoint this user as admin of the organization - #jt_linked.inventory.organization.admin_role.members.add(rando) + assert not access.can_read(jt_linked) assert not access.can_delete(jt_linked) - for role_name in role_names: - role = getattr(jt_linked.inventory.organization, role_name) - role.members.add(rando) + # Appoint this user as admin of the organization + jt_linked.organization.admin_role.members.add(rando) + org_pk = jt_linked.organization.id # Assign organization permission in the same way the create view does organization = jt_linked.inventory.organization ssh_cred.admin_role.parents.add(organization.admin_role) proj_pk = jt_linked.project.pk - assert access.can_add(dict(inventory=jt_linked.inventory.pk, project=proj_pk)) - assert access.can_add(dict(credential=ssh_cred.pk, project=proj_pk)) + assert access.can_add(dict(inventory=jt_linked.inventory.pk, project=proj_pk, organization=org_pk)) + assert access.can_add(dict(credential=ssh_cred.pk, project=proj_pk, organization=org_pk)) for cred in jt_linked.credentials.all(): assert access.can_unattach(jt_linked, cred, 'credentials', {}) @@ -148,26 +169,46 @@ class TestOrphanJobTemplate: @pytest.mark.django_db @pytest.mark.job_permissions -def test_job_template_creator_access(project, rando, post): +def test_job_template_creator_access(project, organization, rando, post): + project.use_role.members.add(rando) + organization.job_template_admin_role.members.add(rando) + response = post(url=reverse('api:job_template_list'), data=dict( + name='newly-created-jt', + ask_inventory_on_launch=True, + project=project.pk, + organization=organization.id, + playbook='helloworld.yml' + ), user=rando, expect=201) - project.admin_role.members.add(rando) - with mock.patch( - 'awx.main.models.projects.ProjectOptions.playbooks', - new_callable=mock.PropertyMock(return_value=['helloworld.yml'])): - response = post(reverse('api:job_template_list'), dict( - name='newly-created-jt', - job_type='run', - ask_inventory_on_launch=True, - ask_credential_on_launch=True, - project=project.pk, - playbook='helloworld.yml' - ), rando) - - assert response.status_code == 201 jt_pk = response.data['id'] jt_obj = JobTemplate.objects.get(pk=jt_pk) # Creating a JT should place the creator in the admin role - assert rando in jt_obj.admin_role + assert rando in jt_obj.admin_role.members.all() + + +@pytest.mark.django_db +@pytest.mark.job_permissions +@pytest.mark.parametrize('lacking', ['project', 'inventory', 'organization']) +def test_job_template_insufficient_creator_permissions(lacking, project, inventory, organization, rando, post): + if lacking != 'project': + project.use_role.members.add(rando) + else: + project.read_role.members.add(rando) + if lacking != 'organization': + organization.job_template_admin_role.members.add(rando) + else: + organization.member_role.members.add(rando) + if lacking != 'inventory': + inventory.use_role.members.add(rando) + else: + inventory.read_role.members.add(rando) + post(url=reverse('api:job_template_list'), data=dict( + name='newly-created-jt', + inventory=inventory.id, + project=project.pk, + organization=organization.id, + playbook='helloworld.yml' + ), user=rando, expect=403) @pytest.mark.django_db @@ -239,7 +280,7 @@ class TestJobTemplateSchedules: @pytest.mark.django_db def test_jt_org_ownership_change(user, jt_linked): admin1 = user('admin1') - org1 = jt_linked.project.organization + org1 = jt_linked.organization org1.admin_role.members.add(admin1) a1_access = JobTemplateAccess(admin1) @@ -254,10 +295,8 @@ def test_jt_org_ownership_change(user, jt_linked): assert not a2_access.can_read(jt_linked) - jt_linked.project.organization = org2 - jt_linked.project.save() - jt_linked.inventory.organization = org2 - jt_linked.inventory.save() + jt_linked.organization = org2 + jt_linked.save() assert a2_access.can_read(jt_linked) assert not a1_access.can_read(jt_linked) diff --git a/awx/main/tests/functional/test_rbac_migration.py b/awx/main/tests/functional/test_rbac_migration.py new file mode 100644 index 0000000000..19693ed5e7 --- /dev/null +++ b/awx/main/tests/functional/test_rbac_migration.py @@ -0,0 +1,64 @@ +import pytest + +from awx.main.migrations import _rbac as rbac +from awx.main.models import ( + UnifiedJobTemplate, + InventorySource, Inventory, + JobTemplate, Project, + Organization +) + + +@pytest.mark.django_db +def test_implied_organization_subquery_inventory(): + orgs = [] + for i in range(3): + orgs.append(Organization.objects.create(name='foo{}'.format(i))) + orgs.append(orgs[0]) + for i in range(4): + org = orgs[i] + if i == 2: + inventory = Inventory.objects.create(name='foo{}'.format(i)) + else: + inventory = Inventory.objects.create(name='foo{}'.format(i), organization=org) + inv_src = InventorySource.objects.create(name='foo{}'.format(i), inventory=inventory) + sources = UnifiedJobTemplate.objects.annotate( + test_field=rbac.implicit_org_subquery(UnifiedJobTemplate, InventorySource) + ) + for inv_src in sources: + assert inv_src.test_field == inv_src.inventory.organization_id + + +@pytest.mark.django_db +def test_implied_organization_subquery_job_template(): + jts = [] + for i in range(5): + if i <= 3: + org = Organization.objects.create(name='foo{}'.format(i)) + else: + org = None + if i <= 4: + proj = Project.objects.create( + name='foo{}'.format(i), + organization=org + ) + else: + proj = None + jts.append(JobTemplate.objects.create( + name='foo{}'.format(i), + project=proj + )) + # test case of sharing same org + jts[2].project.organization = jts[3].project.organization + jts[2].save() + ujts = UnifiedJobTemplate.objects.annotate( + test_field=rbac.implicit_org_subquery(UnifiedJobTemplate, JobTemplate) + ) + for jt in ujts: + if not isinstance(jt, JobTemplate): # some are projects + assert jt.test_field is None + else: + if jt.project is None: + assert jt.test_field is None + else: + assert jt.test_field == jt.project.organization_id diff --git a/awx/main/tests/functional/test_rbac_workflow.py b/awx/main/tests/functional/test_rbac_workflow.py index a53d769324..6e92082358 100644 --- a/awx/main/tests/functional/test_rbac_workflow.py +++ b/awx/main/tests/functional/test_rbac_workflow.py @@ -62,10 +62,11 @@ class TestWorkflowJobTemplateAccess: @pytest.mark.django_db class TestWorkflowJobTemplateNodeAccess: - def test_no_jt_access_to_edit(self, wfjt_node, org_admin): + def test_no_jt_access_to_edit(self, wfjt_node, rando): # without access to the related job template, admin to the WFJT can # not change the prompted parameters - access = WorkflowJobTemplateNodeAccess(org_admin) + wfjt_node.workflow_job_template.admin_role.members.add(rando) + access = WorkflowJobTemplateNodeAccess(rando) assert not access.can_change(wfjt_node, {'job_type': 'check'}) def test_node_edit_allowed(self, wfjt_node, org_admin): diff --git a/awx/main/tests/unit/api/serializers/test_job_template_serializers.py b/awx/main/tests/unit/api/serializers/test_job_template_serializers.py index 4c0751ffbe..2287348d5f 100644 --- a/awx/main/tests/unit/api/serializers/test_job_template_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_job_template_serializers.py @@ -30,6 +30,7 @@ def job_template(mocker): mock_jt.host_config_key = '9283920492' mock_jt.validation_errors = mock_JT_resource_data mock_jt.webhook_service = '' + mock_jt.organization_id = None return mock_jt diff --git a/awx/main/tests/unit/models/test_unified_job_unit.py b/awx/main/tests/unit/models/test_unified_job_unit.py index 328b695371..4442770188 100644 --- a/awx/main/tests/unit/models/test_unified_job_unit.py +++ b/awx/main/tests/unit/models/test_unified_job_unit.py @@ -65,6 +65,14 @@ def test_cancel_job_explanation(unified_job): unified_job.save.assert_called_with(update_fields=['cancel_flag', 'start_args', 'status', 'job_explanation']) +def test_organization_copy_to_jobs(): + ''' + All unified job types should infer their organization from their template organization + ''' + for cls in UnifiedJobTemplate.__subclasses__(): + assert 'organization' in cls._get_unified_job_field_names() + + def test_log_representation(): ''' Common representation used inside of log messages diff --git a/awx/main/tests/unit/test_access.py b/awx/main/tests/unit/test_access.py index f95e6dbd4d..abef7df728 100644 --- a/awx/main/tests/unit/test_access.py +++ b/awx/main/tests/unit/test_access.py @@ -148,7 +148,9 @@ def job_template_with_ids(job_template_factory): 'testJT', project=proj, inventory=inv, credential=credential, cloud_credential=cloud_cred, network_credential=net_cred, persisted=False) - return jt_objects.job_template + jt = jt_objects.job_template + jt.organization = Organization(id=1, pk=1, name='fooOrg') + return jt def test_superuser(mocker): @@ -180,21 +182,24 @@ def test_jt_existing_values_are_nonsensitive(job_template_with_ids, user_unit): def test_change_jt_sensitive_data(job_template_with_ids, mocker, user_unit): """Assure that can_add is called with all ForeignKeys.""" - job_template_with_ids.admin_role = Role() + class RoleReturnsTrue(Role): + def __contains__(self, accessor): + return True + + job_template_with_ids.admin_role = RoleReturnsTrue() + job_template_with_ids.organization.job_template_admin_role = RoleReturnsTrue() + + inv2 = Inventory() + inv2.use_role = RoleReturnsTrue() + data = {'inventory': inv2} - data = {'inventory': job_template_with_ids.inventory.id + 1} access = JobTemplateAccess(user_unit) - mock_add = mock.MagicMock(return_value=False) - with mock.patch('awx.main.models.rbac.Role.__contains__', return_value=True): - with mocker.patch('awx.main.access.JobTemplateAccess.can_add', mock_add): - with mocker.patch('awx.main.access.JobTemplateAccess.can_read', return_value=True): - assert not access.can_change(job_template_with_ids, data) + assert not access.changes_are_non_sensitive(job_template_with_ids, data) - mock_add.assert_called_once_with({ - 'inventory': data['inventory'], - 'project': job_template_with_ids.project.id - }) + job_template_with_ids.inventory.use_role = RoleReturnsTrue() + job_template_with_ids.project.use_role = RoleReturnsTrue() + assert access.can_change(job_template_with_ids, data) def mock_raise_none(self, add_host=False, feature=None, check_expiration=True): diff --git a/awx/main/tests/unit/test_fields.py b/awx/main/tests/unit/test_fields.py index 479d4728b3..429ab6faa0 100644 --- a/awx/main/tests/unit/test_fields.py +++ b/awx/main/tests/unit/test_fields.py @@ -2,10 +2,17 @@ import pytest from django.core.exceptions import ValidationError +from django.apps import apps +from django.db.models.fields.related import ForeignKey +from django.db.models.fields.related_descriptors import ( + ReverseManyToOneDescriptor, + ForwardManyToOneDescriptor +) + from rest_framework.serializers import ValidationError as DRFValidationError from awx.main.models import Credential, CredentialType, BaseModel -from awx.main.fields import JSONSchemaField +from awx.main.fields import JSONSchemaField, ImplicitRoleField, ImplicitRoleDescriptor @pytest.mark.parametrize('schema, given, message', [ @@ -194,3 +201,57 @@ def test_credential_creation_validation_failure(inputs): with pytest.raises(Exception) as e: field.validate(inputs, cred) assert e.type in (ValidationError, DRFValidationError) + + +def test_implicit_role_field_parents(): + """This assures that every ImplicitRoleField only references parents + which are relationships that actually exist + """ + app_models = apps.get_app_config('main').get_models() + for cls in app_models: + for field in cls._meta.get_fields(): + if not isinstance(field, ImplicitRoleField): + continue + + if not field.parent_role: + continue + + field_names = field.parent_role + if type(field_names) is not list: + field_names = [field_names] + + for field_name in field_names: + # this type of specification appears to have been considered + # at some point, but does not exist in the app and would + # need support and tests built out for it + assert not isinstance(field_name, tuple) + # also used to be a thing before py3 upgrade + assert not isinstance(field_name, bytes) + # this is always coherent + if field_name.startswith('singleton:'): + continue + # separate out parent role syntax + field_name, sep, field_attr = field_name.partition('.') + # now make primary assertion, that specified paths exist + assert hasattr(cls, field_name) + + # inspect in greater depth + second_field = cls._meta.get_field(field_name) + second_field_descriptor = getattr(cls, field_name) + # all supported linkage types + assert isinstance(second_field_descriptor, ( + ReverseManyToOneDescriptor, # not currently used + ImplicitRoleDescriptor, + ForwardManyToOneDescriptor + )) + # only these links are supported + if field_attr: + if isinstance(second_field_descriptor, ReverseManyToOneDescriptor): + assert type(second_field) is ForeignKey + rel_model = cls._meta.get_field(field_name).related_model + third_field = getattr(rel_model, field_attr) + # expecting for related_model.foo_role, test role field type + assert isinstance(third_field, ImplicitRoleDescriptor) + else: + # expecting simple format of foo_role + assert type(second_field) is ImplicitRoleField diff --git a/awx/ui/client/src/templates/job_templates/job-template.form.js b/awx/ui/client/src/templates/job_templates/job-template.form.js index 6f1f8cf025..953dcbbd55 100644 --- a/awx/ui/client/src/templates/job_templates/job-template.form.js +++ b/awx/ui/client/src/templates/job_templates/job-template.form.js @@ -43,6 +43,19 @@ function(NotificationsList, i18n) { column: 1, ngDisabled: '!(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)' }, + organization: { + label: i18n._('Organization'), + type: 'lookup', + list: 'OrganizationList', + sourceModel: 'organization', + basePath: 'organizations', + sourceField: 'name', + dataTitle: i18n._('Organization'), + dataContainer: 'body', + dataPlacement: 'right', + ngDisabled: '!(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)', + awLookupWhen: '(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)' + }, job_type: { label: i18n._('Job Type'), type: 'select', diff --git a/awxkit/awxkit/api/pages/job_templates.py b/awxkit/awxkit/api/pages/job_templates.py index 11d46cfbfa..4060da5f2c 100644 --- a/awxkit/awxkit/api/pages/job_templates.py +++ b/awxkit/awxkit/api/pages/job_templates.py @@ -7,7 +7,7 @@ from awxkit.utils import ( suppress, update_payload, PseudoNamespace) -from awxkit.api.pages import Credential, Inventory, Project, UnifiedJobTemplate +from awxkit.api.pages import Credential, Inventory, Project, UnifiedJobTemplate, Organization from awxkit.api.mixins import HasCreate, HasInstanceGroups, HasNotifications, HasSurvey, HasCopy, DSAdapter from awxkit.api.resources import resources import awxkit.exceptions as exc @@ -23,7 +23,7 @@ class JobTemplate( HasSurvey, UnifiedJobTemplate): - optional_dependencies = [Inventory, Credential, Project] + optional_dependencies = [Organization, Inventory, Credential, Project] def launch(self, payload={}): """Launch the job_template using related->launch endpoint.""" @@ -129,6 +129,7 @@ class JobTemplate( playbook='ping.yml', credential=Credential, inventory=Inventory, + organization=Organization, project=None, **kwargs): if not project: @@ -148,12 +149,18 @@ class JobTemplate( project = self.ds.project if project else None inventory = self.ds.inventory if inventory else None credential = self.ds.credential if credential else None + # if the created project has an organization, and the parameters + # specified no organization, then borrow the one from the project + if hasattr(project.ds, 'organization') and organization is Organization: + self.ds.organization = project.ds.organization + organization = self.ds.organization payload = self.payload( name=name, description=description, job_type=job_type, playbook=playbook, + organization=organization, credential=credential, inventory=inventory, project=project, @@ -169,11 +176,12 @@ class JobTemplate( playbook='ping.yml', credential=Credential, inventory=Inventory, + organization=Organization, project=None, **kwargs): payload, credential = self.create_payload(name=name, description=description, job_type=job_type, playbook=playbook, credential=credential, inventory=inventory, - project=project, **kwargs) + project=project, organization=organization, **kwargs) ret = self.update_identity( JobTemplates( self.connection).post(payload)) diff --git a/awxkit/awxkit/api/pages/workflow_job_templates.py b/awxkit/awxkit/api/pages/workflow_job_templates.py index 1d67a0a171..b5b169340e 100644 --- a/awxkit/awxkit/api/pages/workflow_job_templates.py +++ b/awxkit/awxkit/api/pages/workflow_job_templates.py @@ -12,7 +12,7 @@ from . import page class WorkflowJobTemplate(HasCopy, HasCreate, HasNotifications, HasSurvey, UnifiedJobTemplate): - optional_dependencies = [Organization] + dependencies = [Organization] def launch(self, payload={}): """Launch using related->launch endpoint.""" @@ -71,14 +71,14 @@ class WorkflowJobTemplate(HasCopy, HasCreate, HasNotifications, HasSurvey, Unifi return payload - def create_payload(self, name='', description='', organization=None, **kwargs): + def create_payload(self, name='', description='', organization=Organization, **kwargs): self.create_and_update_dependencies(*filter_by_class((organization, Organization))) organization = self.ds.organization if organization else None payload = self.payload(name=name, description=description, organization=organization, **kwargs) payload.ds = DSAdapter(self.__class__.__name__, self._dependency_store) return payload - def create(self, name='', description='', organization=None, **kwargs): + def create(self, name='', description='', organization=Organization, **kwargs): payload = self.create_payload(name=name, description=description, organization=organization, **kwargs) return self.update_identity(WorkflowJobTemplates(self.connection).post(payload)) From 7d0b20757181dde280fd3b4eac15b03d01898219 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 21 Jan 2020 11:12:08 -0500 Subject: [PATCH 2/6] Organization on JT as read-only field Set JT.organization with value from its project Remove validation requiring JT.organization Undo some of the additional org definitions in tests Revert some tests no longer needed for feature exclude workflow approvals from unified organization field revert awxkit changes for providing organization Roll back additional JT creation permission requirement Fix up more issues by persisting organization field when project is removed Restrict project org editing, logging, and testing Grant removed inventory org admin permissions in migration Add special validate_unique for job templates this deals with enforcing name-organization uniqueness Add back in special message where config is unknown when receiving 403 on job relaunch Fix logical and performance bugs with data migration within JT.inventory.organization make-permission-explicit migration remove nested loops so we do .iterator() on JT queryset in reverse migration, carefully remove execute role on JT held by org admins of inventory organization, as well as the execute_role holders Use current state of Role model in logic, with 1 notable exception that is used to filter on ancestors the ancestor and descentent relationship in the migration model is not reliable output of this is saved as an integer list to avoid future compatibility errors make the parents rebuilding logic skip over irrelevant models this is the largest performance gain for small resource numbers --- awx/api/serializers.py | 15 +- awx/main/access.py | 16 +- awx/main/analytics/collectors.py | 6 +- awx/main/fields.py | 14 +- ...7_v370_job_template_organization_field.py} | 19 ++- awx/main/migrations/_rbac.py | 119 ++++++++++++--- awx/main/models/jobs.py | 35 +++++ awx/main/models/projects.py | 7 + awx/main/models/unified_jobs.py | 2 +- awx/main/models/workflow.py | 2 +- awx/main/signals.py | 15 +- awx/main/tests/factories/fixtures.py | 3 +- awx/main/tests/factories/tower.py | 2 +- awx/main/tests/functional/api/test_job.py | 8 +- .../tests/functional/api/test_job_template.py | 144 +++++++++++------- .../functional/api/test_rbac_displays.py | 5 +- .../api/test_unified_job_template.py | 35 ----- awx/main/tests/functional/conftest.py | 10 +- awx/main/tests/functional/models/test_job.py | 21 ++- .../functional/models/test_unified_job.py | 24 +++ awx/main/tests/functional/test_copy.py | 4 +- awx/main/tests/functional/test_instances.py | 11 +- awx/main/tests/functional/test_rbac_job.py | 16 +- .../tests/functional/test_rbac_job_start.py | 15 +- .../functional/test_rbac_job_templates.py | 125 +++++++++++---- .../tests/functional/test_rbac_migration.py | 39 ++++- .../unit/models/test_unified_job_unit.py | 5 +- awx/main/utils/named_url_graph.py | 5 + awxkit/awxkit/api/pages/job_templates.py | 14 +- .../api/pages/workflow_job_templates.py | 6 +- awxkit/awxkit/api/resources.py | 1 + 31 files changed, 517 insertions(+), 226 deletions(-) rename awx/main/migrations/{0085_v360_job_template_organization_field.py => 0107_v370_job_template_organization_field.py} (82%) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 1240d00291..3e1ea7702f 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -72,6 +72,7 @@ from awx.main.utils import ( prefetch_page_capabilities, get_external_account, truncate_stdout, ) from awx.main.utils.filters import SmartFilter +from awx.main.utils.named_url_graph import reset_counters from awx.main.redact import UriCleaner, REPLACE_STR from awx.main.validators import vars_validate_or_raise @@ -347,6 +348,7 @@ class BaseSerializer(serializers.ModelSerializer, metaclass=BaseSerializerMetacl def _generate_named_url(self, url_path, obj, node): url_units = url_path.split('/') + reset_counters() named_url = node.generate_named_url(obj) url_units[4] = named_url return '/'.join(url_units) @@ -700,18 +702,6 @@ class UnifiedJobTemplateSerializer(BaseSerializer): else: return super(UnifiedJobTemplateSerializer, self).to_representation(obj) - def validate(self, attrs): - if 'organization' in self.fields: - # Do not allow setting template organization to null - # otherwise be as non-restrictive as possible for PATCH or PUT, even with orphans - # does not correspond with any REST framework field construct - if self.instance is None and attrs.get('organization', None) is None: - raise serializers.ValidationError({'organization': _('Organization required for new object.')}) - if self.instance and self.instance.organization_id and attrs.get('organization', 'blank') is None: - raise serializers.ValidationError({'organization': _('Organization can not be set to null.')}) - - return super(UnifiedJobTemplateSerializer, self).validate(attrs) - class UnifiedJobSerializer(BaseSerializer): show_capabilities = ['start', 'delete'] @@ -2741,6 +2731,7 @@ class JobOptionsSerializer(LabelsListMixin, BaseSerializer): 'forks', 'limit', 'verbosity', 'extra_vars', 'job_tags', 'force_handlers', 'skip_tags', 'start_at_task', 'timeout', 'use_fact_cache', 'organization',) + read_only_fields = ('organization',) def get_related(self, obj): res = super(JobOptionsSerializer, self).get_related(obj) diff --git a/awx/main/access.py b/awx/main/access.py index d6ae9c0082..d4c830f108 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1465,10 +1465,6 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess): if self.user not in inventory.use_role: return False - organization = get_value(Organization, 'organization') - if (not organization) or (self.user not in organization.job_template_admin_role): - return False - project = get_value(Project, 'project') # If the user has admin access to the project (as an org admin), should # be able to proceed without additional checks. @@ -1651,7 +1647,7 @@ class JobAccess(BaseAccess): except JobLaunchConfig.DoesNotExist: config = None - # Standard permissions model (1) + # Standard permissions model if obj.job_template and (self.user not in obj.job_template.execute_role): return False @@ -1666,13 +1662,15 @@ class JobAccess(BaseAccess): if JobLaunchConfigAccess(self.user).can_add({'reference_obj': config}): return True - # Standard permissions model (2) + # Standard permissions model without job template involved if obj.organization and self.user in obj.organization.execute_role: - # Respect organization ownership of orphaned jobs return True elif not (obj.job_template or obj.organization): - if self.save_messages: - self.messages['detail'] = _('Job has been orphaned from its job template and organization.') + raise PermissionDenied(_('Job has been orphaned from its job template and organization.')) + elif obj.job_template and config is not None: + raise PermissionDenied(_('Job was launched with prompted fields you do not have access to.')) + elif obj.job_template and config is None: + raise PermissionDenied(_('Job was launched with unknown prompted fields. Organization admin permissions required.')) return False diff --git a/awx/main/analytics/collectors.py b/awx/main/analytics/collectors.py index d633e41aeb..173b19fe7b 100644 --- a/awx/main/analytics/collectors.py +++ b/awx/main/analytics/collectors.py @@ -257,7 +257,7 @@ def copy_tables(since, full_path): unified_job_query = '''COPY (SELECT main_unifiedjob.id, main_unifiedjob.polymorphic_ctype_id, django_content_type.model, - main_project.organization_id, + main_unifiedjob.organization_id, main_organization.name as organization_name, main_unifiedjob.created, main_unifiedjob.name, @@ -275,10 +275,8 @@ def copy_tables(since, full_path): main_unifiedjob.job_explanation, main_unifiedjob.instance_group_id FROM main_unifiedjob - JOIN main_job ON main_unifiedjob.id = main_job.unifiedjob_ptr_id JOIN django_content_type ON main_unifiedjob.polymorphic_ctype_id = django_content_type.id - JOIN main_project ON main_project.unifiedjobtemplate_ptr_id = main_job.project_id - JOIN main_organization ON main_organization.id = main_project.organization_id + JOIN main_organization ON main_organization.id = main_unifiedjob.organization_id WHERE main_unifiedjob.created > {} AND main_unifiedjob.launch_type != 'sync' ORDER BY main_unifiedjob.id ASC) TO STDOUT WITH CSV HEADER'''.format(since.strftime("'%Y-%m-%d %H:%M:%S'")) diff --git a/awx/main/fields.py b/awx/main/fields.py index 1038673eb2..36ba0c8394 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -200,29 +200,27 @@ def update_role_parentage_for_instance(instance): of a given instance if they have changed ''' changed_ct = 0 + parents_removed = set() + parents_added = set() for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): - changed = False cur_role = getattr(instance, implicit_role_field.name) original_parents = set(json.loads(cur_role.implicit_parents)) new_parents = implicit_role_field._resolve_parent_roles(instance) removals = original_parents - new_parents if removals: - changed = True cur_role.parents.remove(*list(removals)) + parents_removed.add(cur_role.pk) additions = new_parents - original_parents if additions: - changed = True cur_role.parents.add(*list(additions)) + parents_added.add(cur_role.pk) new_parents_list = list(new_parents) new_parents_list.sort() new_parents_json = json.dumps(new_parents_list) if cur_role.implicit_parents != new_parents_json: - changed = True cur_role.implicit_parents = new_parents_json - cur_role.save() - if changed: - changed_ct += 1 - return changed_ct + cur_role.save(update_fields=['implicit_parents']) + return (parents_added, parents_removed) class ImplicitRoleDescriptor(ForwardManyToOneDescriptor): diff --git a/awx/main/migrations/0085_v360_job_template_organization_field.py b/awx/main/migrations/0107_v370_job_template_organization_field.py similarity index 82% rename from awx/main/migrations/0085_v360_job_template_organization_field.py rename to awx/main/migrations/0107_v370_job_template_organization_field.py index a454083e98..11fd247f5c 100644 --- a/awx/main/migrations/0085_v360_job_template_organization_field.py +++ b/awx/main/migrations/0107_v370_job_template_organization_field.py @@ -5,18 +5,26 @@ import awx.main.fields from django.db import migrations, models import django.db.models.deletion -from awx.main.migrations._rbac import rebuild_role_parentage, migrate_ujt_organization, migrate_ujt_organization_backward +from awx.main.migrations._rbac import ( + rebuild_role_parentage, rebuild_role_hierarchy, + migrate_ujt_organization, migrate_ujt_organization_backward, + restore_inventory_admins, restore_inventory_admins_backward +) + + +def rebuild_jt_parents(apps, schema_editor): + rebuild_role_parentage(apps, schema_editor, models=('jobtemplate',)) class Migration(migrations.Migration): dependencies = [ - ('main', '0084_v360_token_description'), + ('main', '0106_v370_remove_inventory_groups_with_active_failures'), ] operations = [ # backwards parents and ancestors caching - migrations.RunPython(migrations.RunPython.noop, rebuild_role_parentage), + migrations.RunPython(migrations.RunPython.noop, rebuild_jt_parents), # add new organization field for JT and all other unified jobs migrations.AddField( model_name='unifiedjob', @@ -67,6 +75,7 @@ class Migration(migrations.Migration): field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['organization.auditor_role', 'inventory.organization.auditor_role', 'execute_role', 'admin_role'], related_name='+', to='main.Role'), ), # Re-compute the role parents and ancestors caching - # this may be a no-op because field post_save hooks from migrate_jt_organization - migrations.RunPython(rebuild_role_parentage, migrations.RunPython.noop), + migrations.RunPython(rebuild_jt_parents, migrations.RunPython.noop), + # for all permissions that will be removed, make them explicit + migrations.RunPython(restore_inventory_admins, restore_inventory_admins_backward), ] diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 0b052c2350..4a1798176c 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -1,7 +1,7 @@ import logging from time import time -from django.db.models import Subquery, OuterRef +from django.db.models import Subquery, OuterRef, F from awx.main.fields import update_role_parentage_for_instance from awx.main.models.rbac import Role, batch_role_ancestor_rebuilding @@ -115,7 +115,7 @@ def _migrate_unified_organization(apps, unified_cls_name, backward=False): if backward and UNIFIED_ORG_LOOKUPS.get(cls_name, 'not-found') is not None: logger.debug('Not reverse migrating {}, existing data should remain valid'.format(cls_name)) continue - logger.debug('Migrating {} to new organization field'.format(cls_name)) + logger.debug('{}Migrating {} to new organization field'.format('Reverse ' if backward else '', cls_name)) sub_qs = implicit_org_subquery(UnifiedClass, cls, backward=backward) if sub_qs is None: @@ -129,7 +129,7 @@ def _migrate_unified_organization(apps, unified_cls_name, backward=False): r = UnifiedClass.objects.order_by().filter(polymorphic_ctype=this_ct).update(tmp_organization=sub_qs) if r: logger.info('Organization migration on {} affected {} rows.'.format(cls_name, r)) - logger.info('Unified organization migration completed in %f seconds' % (time() - start)) + logger.info('Unified organization migration completed in {:.4f} seconds'.format(time() - start)) def migrate_ujt_organization(apps, schema_editor): @@ -144,6 +144,74 @@ def migrate_ujt_organization_backward(apps, schema_editor): _migrate_unified_organization(apps, 'UnifiedJob', backward=True) +def _restore_inventory_admins(apps, schema_editor, backward=False): + """With the JT.organization changes, admins of organizations connected to + job templates via inventory will have their permissions demoted. + This maintains current permissions over the migration by granting the + permissions they used to have explicitly on the JT itself. + """ + start = time() + JobTemplate = apps.get_model('main', 'JobTemplate') + User = apps.get_model('auth', 'User') + changed_ct = 0 + jt_qs = JobTemplate.objects.filter(inventory__isnull=False) + jt_qs = jt_qs.exclude(inventory__organization=F('project__organization')) + jt_qs = jt_qs.only('id', 'admin_role_id', 'execute_role_id', 'inventory_id') + for jt in jt_qs.iterator(): + org = jt.inventory.organization + for role_name in ('admin_role', 'execute_role'): + role_id = getattr(jt, '{}_id'.format(role_name)) + + user_qs = User.objects + if not backward: + # In this specific case, the name for the org role and JT roles were the same + org_role_id = getattr(org, '{}_id'.format(role_name)) + user_qs = user_qs.filter(roles=org_role_id) + # bizarre migration behavior - ancestors / descendents of + # migration version of Role model is reversed, using current model briefly + ancestor_ids = list( + Role.objects.filter(descendents=role_id).values_list('id', flat=True) + ) + # same as Role.__contains__, filter for "user in jt.admin_role" + user_qs = user_qs.exclude(roles__in=ancestor_ids) + else: + # use the database to filter intersection of users without access + # to the JT role and either organization role + user_qs = user_qs.filter(roles__in=[org.admin_role_id, org.execute_role_id]) + # in reverse, intersection of users who have both + user_qs = user_qs.filter(roles=role_id) + + user_ids = list(user_qs.values_list('id', flat=True)) + if not user_ids: + continue + + role = getattr(jt, role_name) + logger.debug('{} {} on jt {} for users {} via inventory.organization {}'.format( + 'Removing' if backward else 'Setting', + role_name, jt.pk, user_ids, org.pk + )) + if not backward: + # in reverse, explit role becomes redundant + role.members.add(*user_ids) + else: + role.members.remove(*user_ids) + changed_ct += len(user_ids) + + if changed_ct: + logger.info('{} explicit JT permission for {} users in {:.4f} seconds'.format( + 'Removed' if backward else 'Added', + changed_ct, time() - start + )) + + +def restore_inventory_admins(apps, schema_editor): + _restore_inventory_admins(apps, schema_editor) + + +def restore_inventory_admins_backward(apps, schema_editor): + _restore_inventory_admins(apps, schema_editor, backward=True) + + def rebuild_role_hierarchy(apps, schema_editor): ''' This should be called in any migration when ownerships are changed. @@ -164,7 +232,7 @@ def rebuild_role_hierarchy(apps, schema_editor): logger.info('Done.') -def rebuild_role_parentage(apps, schema_editor): +def rebuild_role_parentage(apps, schema_editor, models=None): ''' This should be called in any migration when any parent_role entry is modified so that the cached parent fields will be updated. Ex: @@ -177,13 +245,23 @@ def rebuild_role_parentage(apps, schema_editor): ''' start = time() seen_models = set() - updated_ct = 0 model_ct = 0 noop_ct = 0 - Role = apps.get_model('main', "Role") - for role in Role.objects.iterator(): + ContentType = apps.get_model('contenttypes', "ContentType") + additions = set() + removals = set() + + role_qs = Role.objects + if models: + # update_role_parentage_for_instance is expensive + # if the models have been downselected, ignore those which are not in the list + ct_ids = list(ContentType.objects.filter( + model__in=[name.lower() for name in models] + ).values_list('id', flat=True)) + role_qs = role_qs.filter(content_type__in=ct_ids) + + for role in role_qs.iterator(): if not role.object_id: - noop_ct += 1 continue model_tuple = (role.content_type_id, role.object_id) if model_tuple in seen_models: @@ -198,19 +276,26 @@ def rebuild_role_parentage(apps, schema_editor): ct_model = apps.get_model(app, ct.model) content_object = ct_model.objects.get(pk=role.object_id) - updated = update_role_parentage_for_instance(content_object) - if updated: + parents_added, parents_removed = update_role_parentage_for_instance(content_object) + additions.update(parents_added) + removals.update(parents_removed) + if parents_added: model_ct += 1 - logger.debug('Updated parents of {} roles of {}'.format(updated, content_object)) + logger.debug('Added to parents of roles {} of {}'.format(parents_added, content_object)) + if parents_removed: + model_ct += 1 + logger.debug('Removed from parents of roles {} of {}'.format(parents_removed, content_object)) else: noop_ct += 1 - updated_ct += updated - logger.debug('No changes to role parents for {} roles'.format(noop_ct)) - if updated_ct: - logger.info('Updated parentage for {} roles of {} resources'.format(updated_ct, model_ct)) + logger.debug('No changes to role parents for {} resources'.format(noop_ct)) + logger.debug('Added parents to {} roles'.format(len(additions))) + logger.debug('Removed parents from {} roles'.format(len(removals))) + if model_ct: + logger.info('Updated implicit parents of {} resources'.format(model_ct)) logger.info('Rebuild parentage completed in %f seconds' % (time() - start)) - if updated_ct: - rebuild_role_hierarchy(apps, schema_editor) + # this is ran because the ordinary signals for + # Role.parents.add and Role.parents.remove not called in migration + Role.rebuild_role_ancestor_list(list(additions), list(removals)) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 829720afa1..e67478a8e8 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -323,6 +323,41 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour else: return self.job_slice_count + def save(self, *args, **kwargs): + update_fields = kwargs.get('update_fields', []) + # if project is deleted for some reason, then keep the old organization + # to retain ownership for organization admins + if self.project and self.project.organization_id != self.organization_id: + self.organization_id = self.project.organization_id + if 'organization' not in update_fields and 'organization_id' not in update_fields: + update_fields.append('organization_id') + return super(JobTemplate, self).save(*args, **kwargs) + + def validate_unique(self, exclude=None): + """Custom over-ride for JT specifically + because organization is inferred from project after full_clean is finished + thus the organization field is not yet set when validation happens + """ + errors = [] + for ut in JobTemplate.SOFT_UNIQUE_TOGETHER: + kwargs = {'name': self.name} + if self.project: + kwargs['organization'] = self.project.organization_id + else: + kwargs['organization'] = None + qs = JobTemplate.objects.filter(**kwargs) + if self.pk: + qs = qs.exclude(pk=self.pk) + if qs.exists(): + errors.append( + '%s with this (%s) combination already exists.' % ( + JobTemplate.__name__, + ', '.join(set(ut) - {'polymorphic_ctype'}) + ) + ) + if errors: + raise ValidationError(errors) + def create_unified_job(self, **kwargs): prevent_slicing = kwargs.pop('_prevent_slicing', False) slice_ct = self.get_effective_slice_ct(kwargs) diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index c9bf9762dd..aad612ebd8 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -325,6 +325,13 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn ['name', 'description', 'organization'] ) + def clean_organization(self): + if self.pk: + old_org_id = getattr(self, '_prior_values_store', {}).get('organization_id', None) + if self.organization_id != old_org_id and self.jobtemplates.exists(): + raise ValidationError({'organization': _('Organization cannot be changed when in use by job templates.')}) + return self.organization + def save(self, *args, **kwargs): new_instance = not bool(self.pk) pre_save_vals = getattr(self, '_prior_values_store', {}) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 1c2c6ffeed..253eb7b57f 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -102,7 +102,7 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio ordering = ('name',) # unique_together here is intentionally commented out. Please make sure sub-classes of this model # contain at least this uniqueness restriction: SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name')] - #unique_together = [('polymorphic_ctype', 'name')] + #unique_together = [('polymorphic_ctype', 'name', 'organization')] old_pk = models.PositiveIntegerField( null=True, diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index df4772a20d..87888b9f92 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -376,7 +376,7 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name', 'organization')] FIELDS_TO_PRESERVE_AT_COPY = [ - 'labels', 'instance_groups', 'workflow_job_template_nodes', 'credentials', 'survey_spec' + 'labels', 'organization', 'instance_groups', 'workflow_job_template_nodes', 'credentials', 'survey_spec' ] class Meta: diff --git a/awx/main/signals.py b/awx/main/signals.py index 27a6426eba..64a35c1f1d 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -157,17 +157,26 @@ def cleanup_detached_labels_on_deleted_parent(sender, instance, **kwargs): def save_related_job_templates(sender, instance, **kwargs): '''save_related_job_templates loops through all of the - job templates that use an Inventory or Project that have had their + job templates that use an Inventory that have had their Organization updated. This triggers the rebuilding of the RBAC hierarchy and ensures the proper access restrictions. ''' - if sender not in (Project, Inventory): + if sender is not Inventory: raise ValueError('This signal callback is only intended for use with Project or Inventory') + update_fields = kwargs.get('update_fields', None) + if ((update_fields and not ('organization' in update_fields or 'organization_id' in update_fields)) or + kwargs.get('created', False)): + return + if instance._prior_values_store.get('organization_id') != instance.organization_id: jtq = JobTemplate.objects.filter(**{sender.__name__.lower(): instance}) for jt in jtq: - update_role_parentage_for_instance(jt) + parents_added, parents_removed = update_role_parentage_for_instance(jt) + if parents_added or parents_removed: + logger.info('Permissions on JT {} changed due to inventory {} organization change from {} to {}.'.format( + jt.pk, instance.pk, instance._prior_values_store.get('organization_id'), instance.organization_id + )) def connect_computed_field_signals(): diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py index 0952738174..2f8cbe6934 100644 --- a/awx/main/tests/factories/fixtures.py +++ b/awx/main/tests/factories/fixtures.py @@ -159,8 +159,7 @@ def mk_job_template(name, job_type='run', extra_vars = json.dumps(extra_vars) jt = JobTemplate(name=name, job_type=job_type, extra_vars=extra_vars, - webhook_service=webhook_service, playbook='helloworld.yml', - organization=organization) + webhook_service=webhook_service, playbook='helloworld.yml') jt.inventory = inventory if jt.inventory is None: diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py index dd412571e1..bfa7f9fc1b 100644 --- a/awx/main/tests/factories/tower.py +++ b/awx/main/tests/factories/tower.py @@ -255,7 +255,7 @@ def create_job_template(name, roles=None, persisted=True, webhook_service='', ** jt = mk_job_template(name, project=proj, inventory=inv, credential=cred, network_credential=net_cred, cloud_credential=cloud_cred, job_type=job_type, spec=spec, extra_vars=extra_vars, - persisted=persisted, webhook_service=webhook_service, organization=org) + persisted=persisted, webhook_service=webhook_service) if 'jobs' in kwargs: for i in kwargs['jobs']: diff --git a/awx/main/tests/functional/api/test_job.py b/awx/main/tests/functional/api/test_job.py index 683f62dc43..db32c0dd7f 100644 --- a/awx/main/tests/functional/api/test_job.py +++ b/awx/main/tests/functional/api/test_job.py @@ -53,7 +53,7 @@ def test_job_relaunch_permission_denied_response( # Job has prompted extra_credential, launch denied w/ message job.launch_config.credentials.add(net_credential) r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403) - assert 'launched with prompted fields which you do not have access to' in r.data['detail'] + assert 'launched with prompted fields you do not have access to' in r.data['detail'] @pytest.mark.django_db @@ -73,7 +73,6 @@ def test_job_relaunch_prompts_not_accepted_response( # Job has prompted extra_credential, launch denied w/ message job.launch_config.credentials.add(net_credential) r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403) - assert 'no longer accepts the prompts provided for this job' in r.data['detail'] @pytest.mark.django_db @@ -220,8 +219,7 @@ def test_block_unprocessed_events(delete, admin_user, mocker): def test_block_related_unprocessed_events(mocker, organization, project, delete, admin_user): job_template = JobTemplate.objects.create( project=project, - playbook='helloworld.yml', - organization=organization + playbook='helloworld.yml' ) time_of_finish = parse("Thu Feb 23 14:17:24 2012 -0500") Job.objects.create( @@ -230,7 +228,7 @@ def test_block_related_unprocessed_events(mocker, organization, project, delete, finished=time_of_finish, job_template=job_template, project=project, - organization=organization + organization=project.organization ) view = RelatedJobsPreventDeleteMixin() time_of_request = time_of_finish + relativedelta(seconds=2) diff --git a/awx/main/tests/functional/api/test_job_template.py b/awx/main/tests/functional/api/test_job_template.py index 2e56d8253b..6ad0271042 100644 --- a/awx/main/tests/functional/api/test_job_template.py +++ b/awx/main/tests/functional/api/test_job_template.py @@ -6,7 +6,7 @@ import pytest # AWX from awx.api.serializers import JobTemplateSerializer from awx.api.versioning import reverse -from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate, Organization +from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate, Organization, Project from awx.main.migrations import _save_password_keys as save_password_keys # Django @@ -32,50 +32,16 @@ def test_create(post, project, machine_credential, inventory, alice, grant_proje inventory.use_role.members.add(alice) project.organization.job_template_admin_role.members.add(alice) - r = post(reverse('api:job_template_list'), { - 'name': 'Some name', - 'project': project.id, - 'inventory': inventory.id, - 'playbook': 'helloworld.yml', - 'organization': project.organization_id - }, alice) - assert r.status_code == expect - - -@pytest.mark.django_db -def test_creation_uniqueness_rules(post, project, inventory, admin_user): - orgA = Organization.objects.create(name='orga') - orgB = Organization.objects.create(name='orgb') - create_data = { - 'name': 'this_unique_name', - 'project': project.pk, - 'inventory': inventory.pk, - 'playbook': 'helloworld.yml', - 'organization': orgA.pk - } post( url=reverse('api:job_template_list'), - data=create_data, - user=admin_user, - expect=201 - ) - r = post( - url=reverse('api:job_template_list'), - data=create_data, - user=admin_user, - expect=400 - ) - msg = str(r.data['__all__'][0]) - assert "JobTemplate with this (" in msg - assert ") combination already exists" in msg - - # can create JT with same name, only if it is in different org - create_data['organization'] = orgB.pk - post( - url=reverse('api:job_template_list'), - data=create_data, - user=admin_user, - expect=201 + data={ + 'name': 'Some name', + 'project': project.id, + 'inventory': inventory.id, + 'playbook': 'helloworld.yml' + }, + user=alice, + expect=expect ) @@ -162,14 +128,18 @@ def test_create_with_forks_exceeding_maximum_xfail(alice, post, project, invento project.use_role.members.add(alice) inventory.use_role.members.add(alice) settings.MAX_FORKS = 10 - response = post(reverse('api:job_template_list'), { - 'name': 'Some name', - 'project': project.id, - 'inventory': inventory.id, - 'playbook': 'helloworld.yml', - 'forks': 11, - }, alice) - assert response.status_code == 400 + response = post( + url=reverse('api:job_template_list'), + data={ + 'name': 'Some name', + 'project': project.id, + 'inventory': inventory.id, + 'playbook': 'helloworld.yml', + 'forks': 11, + }, + user=alice, + expect=400 + ) assert 'Maximum number of forks (10) exceeded' in str(response.data) @@ -549,6 +519,72 @@ def test_job_template_unset_custom_virtualenv(get, patch, organization_factory, assert resp.data['custom_virtualenv'] is None +@pytest.mark.django_db +def test_jt_organization_follows_project(post, patch, admin_user): + org1 = Organization.objects.create(name='foo1') + org2 = Organization.objects.create(name='foo2') + project_common = dict(scm_type='git', playbook_files=['helloworld.yml']) + project1 = Project.objects.create(name='proj1', organization=org1, **project_common) + project2 = Project.objects.create(name='proj2', organization=org2, **project_common) + r = post( + url=reverse('api:job_template_list'), + data={ + "name": "fooo", + "ask_inventory_on_launch": True, + "project": project1.pk, + "playbook": "helloworld.yml" + }, + user=admin_user, + expect=201 + ) + data = r.data + assert data['organization'] == project1.organization_id + data['project'] = project2.id + jt = JobTemplate.objects.get(pk=data['id']) + r = patch( + url=jt.get_absolute_url(), + data=data, + user=admin_user, + expect=200 + ) + assert r.data['organization'] == project2.organization_id + + +@pytest.mark.django_db +def test_jt_organization_field_is_read_only(patch, post, project, admin_user): + org = project.organization + jt = JobTemplate.objects.create( + name='foo_jt', + ask_inventory_on_launch=True, + project=project, playbook='helloworld.yml' + ) + org2 = Organization.objects.create(name='foo2') + r = patch( + url=jt.get_absolute_url(), + data={'organization': org2.id}, + user=admin_user, + expect=200 + ) + assert r.data['organization'] == org.id + assert JobTemplate.objects.get(pk=jt.pk).organization == org + + # similar test, but on creation + r = post( + url=reverse('api:job_template_list'), + data={ + 'name': 'foobar', + 'project': project.id, + 'organization': org2.id, + 'ask_inventory_on_launch': True, + 'playbook': 'helloworld.yml' + }, + user=admin_user, + expect=201 + ) + assert r.data['organization'] == org.id + assert JobTemplate.objects.get(pk=r.data['id']).organization == org + + @pytest.mark.django_db def test_callback_disallowed_null_inventory(project): jt = JobTemplate.objects.create( @@ -563,14 +599,13 @@ def test_callback_disallowed_null_inventory(project): @pytest.mark.django_db -def test_job_template_branch_error(project, inventory, organization, post, admin_user): +def test_job_template_branch_error(project, inventory, post, admin_user): r = post( url=reverse('api:job_template_list'), data={ "name": "fooo", "inventory": inventory.pk, "project": project.pk, - "organization": organization.pk, "playbook": "helloworld.yml", "scm_branch": "foobar" }, @@ -581,14 +616,13 @@ def test_job_template_branch_error(project, inventory, organization, post, admin @pytest.mark.django_db -def test_job_template_branch_prompt_error(project, inventory, post, organization, admin_user): +def test_job_template_branch_prompt_error(project, inventory, post, admin_user): r = post( url=reverse('api:job_template_list'), data={ "name": "fooo", "inventory": inventory.pk, "project": project.pk, - "organization": organization.pk, "playbook": "helloworld.yml", "ask_scm_branch_on_launch": True }, diff --git a/awx/main/tests/functional/api/test_rbac_displays.py b/awx/main/tests/functional/api/test_rbac_displays.py index c3dd65d9c4..4180647d44 100644 --- a/awx/main/tests/functional/api/test_rbac_displays.py +++ b/awx/main/tests/functional/api/test_rbac_displays.py @@ -61,7 +61,7 @@ class TestJobTemplateCopyEdit: def jt_copy_edit(self, job_template_factory, project): objects = job_template_factory( 'copy-edit-job-template', - project=project, organization=project.organization) + project=project) return objects.job_template def fake_context(self, user): @@ -129,8 +129,9 @@ class TestJobTemplateCopyEdit: # random user given JT and project admin abilities jt_copy_edit.admin_role.members.add(rando) + jt_copy_edit.save() jt_copy_edit.project.admin_role.members.add(rando) - jt_copy_edit.organization.job_template_admin_role.members.add(rando) + jt_copy_edit.project.save() serializer = JobTemplateSerializer(jt_copy_edit, context=self.fake_context(rando)) response = serializer.to_representation(jt_copy_edit) diff --git a/awx/main/tests/functional/api/test_unified_job_template.py b/awx/main/tests/functional/api/test_unified_job_template.py index 1febd2f50e..c2df65b49f 100644 --- a/awx/main/tests/functional/api/test_unified_job_template.py +++ b/awx/main/tests/functional/api/test_unified_job_template.py @@ -39,29 +39,6 @@ class TestUnifiedOrganization: data['ask_inventory_on_launch'] = True return data - def test_organization_required_on_creation(self, model, admin_user, post): - cls = getattr(models, model) - data = self.data_for_model(model) - r = post( - url=reverse('api:{}_list'.format(get_type_for_model(cls))), - data=data, - user=admin_user, - expect=400 - ) - assert 'organization' in r.data - assert 'required for new object' in r.data['organization'][0] - # Surprising behavior - not providing the key can often give - # different behavior from giving it as null on create - data.pop('organization') - r = post( - url=reverse('api:{}_list'.format(get_type_for_model(cls))), - data=data, - user=admin_user, - expect=400 - ) - assert 'organization' in r.data - assert 'required' in r.data['organization'][0] - def test_organization_blank_on_edit_of_orphan(self, model, admin_user, patch): cls = getattr(models, model) data = self.data_for_model(model, orm_style=True) @@ -107,15 +84,3 @@ class TestUnifiedOrganization: ) obj.refresh_from_db() assert obj.name == 'foooooo' - - def test_organization_cannot_change_to_null(self, model, admin_user, patch, organization): - cls = getattr(models, model) - data = self.data_for_model(model, orm_style=True) - data['organization'] = organization - obj = cls.objects.create(**data) - patch( - url=obj.get_absolute_url(), - data={'organization': None}, - user=admin_user, - expect=400 - ) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index cae55c8562..54149a6419 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -75,26 +75,24 @@ def user(): @pytest.fixture -def check_jobtemplate(project, inventory, credential, organization): +def check_jobtemplate(project, inventory, credential): jt = JobTemplate.objects.create( job_type='check', project=project, inventory=inventory, - name='check-job-template', - organization=organization + name='check-job-template' ) jt.credentials.add(credential) return jt @pytest.fixture -def deploy_jobtemplate(project, inventory, credential, organization): +def deploy_jobtemplate(project, inventory, credential): jt = JobTemplate.objects.create( job_type='run', project=project, inventory=inventory, - name='deploy-job-template', - organization=organization + name='deploy-job-template' ) jt.credentials.add(credential) return jt diff --git a/awx/main/tests/functional/models/test_job.py b/awx/main/tests/functional/models/test_job.py index b097f85548..ac8912506f 100644 --- a/awx/main/tests/functional/models/test_job.py +++ b/awx/main/tests/functional/models/test_job.py @@ -1,6 +1,9 @@ import pytest -from awx.main.models import JobTemplate, Job, JobHostSummary, WorkflowJob, Inventory +from awx.main.models import ( + JobTemplate, Job, JobHostSummary, + WorkflowJob, Inventory, Project, Organization +) @pytest.mark.django_db @@ -79,6 +82,22 @@ def test_job_host_summary_representation(host): assert 'N/A changed=1 dark=2 failures=3 ignored=4 ok=5 processed=6 rescued=7 skipped=8' == str(jhs) +@pytest.mark.django_db +def test_jt_organization_follows_project(): + org1 = Organization.objects.create(name='foo1') + org2 = Organization.objects.create(name='foo2') + project1 = Project.objects.create(name='proj1', organization=org1) + project2 = Project.objects.create(name='proj2', organization=org2) + jt = JobTemplate.objects.create( + name='foo', playbook='helloworld.yml', + project=project1 + ) + assert jt.organization == org1 + jt.project = project2 + jt.save() + assert JobTemplate.objects.get(pk=jt.id).organization == org2 + + @pytest.mark.django_db class TestSlicingModels: diff --git a/awx/main/tests/functional/models/test_unified_job.py b/awx/main/tests/functional/models/test_unified_job.py index 90fc4ce37e..7b5f6d432b 100644 --- a/awx/main/tests/functional/models/test_unified_job.py +++ b/awx/main/tests/functional/models/test_unified_job.py @@ -13,6 +13,7 @@ from awx.main.models import ( WorkflowApprovalTemplate, Project, WorkflowJob, Schedule, Credential ) +from awx.api.versioning import reverse @pytest.mark.django_db @@ -26,6 +27,29 @@ def test_subclass_types(rando): ]) +@pytest.mark.django_db +def test_soft_unique_together(post, project, admin_user): + """This tests that SOFT_UNIQUE_TOGETHER restrictions are applied correctly. + """ + jt1 = JobTemplate.objects.create( + name='foo_jt', + project=project + ) + assert jt1.organization == project.organization + r = post( + url=reverse('api:job_template_list'), + data=dict( + name='foo_jt', # same as first + project=project.id, + ask_inventory_on_launch=True, + playbook='helloworld.yml' + ), + user=admin_user, + expect=400 + ) + assert 'combination already exists' in str(r.data) + + @pytest.mark.django_db class TestCreateUnifiedJob: ''' diff --git a/awx/main/tests/functional/test_copy.py b/awx/main/tests/functional/test_copy.py index 747f7754c6..7be582d6c8 100644 --- a/awx/main/tests/functional/test_copy.py +++ b/awx/main/tests/functional/test_copy.py @@ -11,11 +11,10 @@ from awx.main.tasks import deep_copy_model_obj @pytest.mark.django_db -def test_job_template_copy(post, get, project, inventory, organization, machine_credential, vault_credential, +def test_job_template_copy(post, get, project, inventory, machine_credential, vault_credential, credential, alice, job_template_with_survey_passwords, admin): job_template_with_survey_passwords.project = project job_template_with_survey_passwords.inventory = inventory - job_template_with_survey_passwords.organization = organization job_template_with_survey_passwords.save() job_template_with_survey_passwords.credentials.add(credential) job_template_with_survey_passwords.credentials.add(machine_credential) @@ -23,7 +22,6 @@ def test_job_template_copy(post, get, project, inventory, organization, machine_ job_template_with_survey_passwords.admin_role.members.add(alice) project.admin_role.members.add(alice) inventory.admin_role.members.add(alice) - organization.job_template_admin_role.members.add(alice) assert get( reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}), alice, expect=200 diff --git a/awx/main/tests/functional/test_instances.py b/awx/main/tests/functional/test_instances.py index a2a0a646ec..67331a6973 100644 --- a/awx/main/tests/functional/test_instances.py +++ b/awx/main/tests/functional/test_instances.py @@ -1,7 +1,7 @@ import pytest from unittest import mock -from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate, Organization +from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate from awx.main.models.ha import Instance, InstanceGroup from awx.main.tasks import apply_cluster_membership_policies from awx.api.versioning import reverse @@ -253,7 +253,7 @@ def test_inherited_instance_group_membership(instance_group_factory, default_ins j.inventory = inventory ig_org = instance_group_factory("basicA", [default_instance_group.instances.first()]) ig_inv = instance_group_factory("basicB", [default_instance_group.instances.first()]) - j.organization.instance_groups.add(ig_org) + j.project.organization.instance_groups.add(ig_org) j.inventory.instance_groups.add(ig_inv) assert ig_org in j.preferred_instance_groups assert ig_inv in j.preferred_instance_groups @@ -320,14 +320,13 @@ class TestInstanceGroupOrdering: assert pu.preferred_instance_groups == [ig_tmp, ig_org] def test_job_instance_groups(self, instance_group_factory, inventory, project, default_instance_group): - org = Organization.objects.create(name='foo') - jt = JobTemplate.objects.create(inventory=inventory, project=project, organization=org) - job = Job.objects.create(inventory=inventory, job_template=jt, project=project, organization=org) + jt = JobTemplate.objects.create(inventory=inventory, project=project) + job = jt.create_unified_job() assert job.preferred_instance_groups == [default_instance_group] ig_org = instance_group_factory("OrgIstGrp", [default_instance_group.instances.first()]) ig_inv = instance_group_factory("InvIstGrp", [default_instance_group.instances.first()]) ig_tmp = instance_group_factory("TmpIstGrp", [default_instance_group.instances.first()]) - jt.organization.instance_groups.add(ig_org) + project.organization.instance_groups.add(ig_org) inventory.instance_groups.add(ig_inv) assert job.preferred_instance_groups == [ig_inv, ig_org] job.job_template.instance_groups.add(ig_tmp) diff --git a/awx/main/tests/functional/test_rbac_job.py b/awx/main/tests/functional/test_rbac_job.py index 16ff68e0ef..1c096e74c7 100644 --- a/awx/main/tests/functional/test_rbac_job.py +++ b/awx/main/tests/functional/test_rbac_job.py @@ -1,5 +1,7 @@ import pytest +from rest_framework.exceptions import PermissionDenied + from awx.main.access import ( JobAccess, JobLaunchConfigAccess, @@ -171,9 +173,11 @@ class TestJobRelaunchAccess: machine_credential.use_role.members.add(u) access = JobAccess(u) - assert access.can_start(job_with_links, validate_license=False) == can_start, ( - "Inventory access: {}\nCredential access: {}\n Expected access: {}".format(inv_access, cred_access, can_start) - ) + if can_start: + assert access.can_start(job_with_links, validate_license=False) + else: + with pytest.raises(PermissionDenied): + access.can_start(job_with_links, validate_license=False) def test_job_relaunch_credential_access( self, inventory, project, credential, net_credential): @@ -188,7 +192,8 @@ class TestJobRelaunchAccess: # Job has prompted net credential, launch denied w/ message job = jt.create_unified_job(credentials=[net_credential]) - assert not jt_user.can_access(Job, 'start', job, validate_license=False) + with pytest.raises(PermissionDenied): + jt_user.can_access(Job, 'start', job, validate_license=False) def test_prompted_credential_relaunch_denied( self, inventory, project, net_credential, rando): @@ -201,7 +206,8 @@ class TestJobRelaunchAccess: # Job has prompted net credential, rando lacks permission to use it job = jt.create_unified_job(credentials=[net_credential]) - assert not rando.can_access(Job, 'start', job, validate_license=False) + with pytest.raises(PermissionDenied): + rando.can_access(Job, 'start', job, validate_license=False) def test_prompted_credential_relaunch_allowed( self, inventory, project, net_credential, rando): diff --git a/awx/main/tests/functional/test_rbac_job_start.py b/awx/main/tests/functional/test_rbac_job_start.py index 3c6d74a0a8..6fa34cc874 100644 --- a/awx/main/tests/functional/test_rbac_job_start.py +++ b/awx/main/tests/functional/test_rbac_job_start.py @@ -1,5 +1,7 @@ import pytest +from rest_framework.exceptions import PermissionDenied + from awx.main.models.inventory import Inventory from awx.main.models.credential import Credential from awx.main.models.jobs import JobTemplate, Job @@ -121,6 +123,7 @@ class TestJobRelaunchAccess: def test_orphan_relaunch_via_organization(self, job_no_prompts, rando, organization): "JT for job has been deleted, relevant organization roles will allow management" + assert job_no_prompts.organization == organization organization.execute_role.members.add(rando) job_no_prompts.job_template.delete() job_no_prompts.job_template = None # Django should do this for us, but it does not @@ -129,7 +132,9 @@ class TestJobRelaunchAccess: def test_no_relaunch_without_prompted_fields_access(self, job_with_prompts, rando): "Has JT execute_role but no use_role on inventory & credential - deny relaunch" job_with_prompts.job_template.execute_role.members.add(rando) - assert not rando.can_access(Job, 'start', job_with_prompts) + with pytest.raises(PermissionDenied) as exc: + rando.can_access(Job, 'start', job_with_prompts) + assert 'Job was launched with prompted fields you do not have access to' in str(exc) def test_can_relaunch_with_prompted_fields_access(self, job_with_prompts, rando): "Has use_role on the prompted inventory & credential - allow relaunch" @@ -148,11 +153,15 @@ class TestJobRelaunchAccess: jt.ask_limit_on_launch = False jt.save() jt.execute_role.members.add(rando) - assert not rando.can_access(Job, 'start', job_with_prompts) + with pytest.raises(PermissionDenied): + rando.can_access(Job, 'start', job_with_prompts) def test_can_relaunch_if_limit_was_prompt(self, job_with_prompts, rando): "Job state differs from JT, but only on prompted fields - allow relaunch" job_with_prompts.job_template.execute_role.members.add(rando) job_with_prompts.limit = 'webservers' job_with_prompts.save() - assert not rando.can_access(Job, 'start', job_with_prompts) + job_with_prompts.inventory.use_role.members.add(rando) + for cred in job_with_prompts.credentials.all(): + cred.use_role.members.add(rando) + assert rando.can_access(Job, 'start', job_with_prompts) diff --git a/awx/main/tests/functional/test_rbac_job_templates.py b/awx/main/tests/functional/test_rbac_job_templates.py index 29910db3ae..a4fbcbaf36 100644 --- a/awx/main/tests/functional/test_rbac_job_templates.py +++ b/awx/main/tests/functional/test_rbac_job_templates.py @@ -8,8 +8,7 @@ from awx.main.access import ( ScheduleAccess ) from awx.main.models.jobs import JobTemplate -from awx.main.models.organization import Organization -from awx.main.models.schedules import Schedule +from awx.main.models import Project, Organization, Inventory, Schedule, User @mock.patch.object(BaseAccess, 'check_license', return_value=None) @@ -126,11 +125,11 @@ def test_job_template_extra_credentials_prompts_access( ) jt.credentials.add(machine_credential) jt.execute_role.members.add(rando) - r = post( + post( reverse('api:job_template_launch', kwargs={'pk': jt.id}), - {'credentials': [machine_credential.pk, vault_credential.pk]}, rando + {'credentials': [machine_credential.pk, vault_credential.pk]}, rando, + expect=403 ) - assert r.status_code == 403 @pytest.mark.django_db @@ -188,16 +187,12 @@ def test_job_template_creator_access(project, organization, rando, post): @pytest.mark.django_db @pytest.mark.job_permissions -@pytest.mark.parametrize('lacking', ['project', 'inventory', 'organization']) +@pytest.mark.parametrize('lacking', ['project', 'inventory']) def test_job_template_insufficient_creator_permissions(lacking, project, inventory, organization, rando, post): if lacking != 'project': project.use_role.members.add(rando) else: project.read_role.members.add(rando) - if lacking != 'organization': - organization.job_template_admin_role.members.add(rando) - else: - organization.member_role.members.add(rando) if lacking != 'inventory': inventory.use_role.members.add(rando) else: @@ -206,7 +201,6 @@ def test_job_template_insufficient_creator_permissions(lacking, project, invento name='newly-created-jt', inventory=inventory.id, project=project.pk, - organization=organization.id, playbook='helloworld.yml' ), user=rando, expect=403) @@ -278,25 +272,104 @@ class TestJobTemplateSchedules: @pytest.mark.django_db -def test_jt_org_ownership_change(user, jt_linked): - admin1 = user('admin1') - org1 = jt_linked.organization - org1.admin_role.members.add(admin1) - a1_access = JobTemplateAccess(admin1) +class TestProjectOrganization: + """Tests stories related to management of JT organization via its project + which have some bearing on RBAC integrity + """ - assert a1_access.can_read(jt_linked) + def test_new_project_org_change(self, project, patch, admin_user): + org2 = Organization.objects.create(name='bar') + patch( + url=project.get_absolute_url(), + data={'organization': org2.id}, + user=admin_user, + expect=200 + ) + assert Project.objects.get(pk=project.id).organization_id == org2.id + def test_jt_org_cannot_change(self, project, post, patch, admin_user): + post( + url=reverse('api:job_template_list'), + data={ + 'name': 'foo_template', + 'project': project.id, + 'playbook': 'helloworld.yml', + 'ask_inventory_on_launch': True + }, + user=admin_user, + expect=201 + ) + org2 = Organization.objects.create(name='bar') + r = patch( + url=project.get_absolute_url(), + data={'organization': org2.id}, + user=admin_user, + expect=400 + ) + assert 'Organization cannot be changed' in str(r.data) - admin2 = user('admin2') - org2 = Organization.objects.create(name='mrroboto', description='domo') - org2.admin_role.members.add(admin2) - a2_access = JobTemplateAccess(admin2) + def test_orphan_JT_adoption(self, project, patch, admin_user, org_admin): + jt = JobTemplate.objects.create( + name='bar', + ask_inventory_on_launch=True, + playbook='helloworld.yml' + ) + assert org_admin not in jt.admin_role + patch( + url=jt.get_absolute_url(), + data={'project': project.id}, + user=admin_user, + expect=200 + ) + assert org_admin in jt.admin_role - assert not a2_access.can_read(jt_linked) + def test_inventory_read_transfer_direct(self, patch): + orgs = [] + invs = [] + admins = [] + for i in range(2): + org = Organization.objects.create(name='org{}'.format(i)) + org_admin = User.objects.create(username='user{}'.format(i)) + inv = Inventory.objects.create( + organization=org, + name='inv{}'.format(i) + ) + org.auditor_role.members.add(org_admin) + orgs.append(org) + admins.append(org_admin) + invs.append(inv) - jt_linked.organization = org2 - jt_linked.save() + jt = JobTemplate.objects.create(name='foo', inventory=invs[0]) + assert admins[0] in jt.read_role + assert admins[1] not in jt.read_role - assert a2_access.can_read(jt_linked) - assert not a1_access.can_read(jt_linked) + jt.inventory = invs[1] + jt.save(update_fields=['inventory']) + assert admins[0] not in jt.read_role + assert admins[1] in jt.read_role + + def test_inventory_read_transfer_indirect(self, patch): + orgs = [] + admins = [] + for i in range(2): + org = Organization.objects.create(name='org{}'.format(i)) + org_admin = User.objects.create(username='user{}'.format(i)) + org.auditor_role.members.add(org_admin) + + orgs.append(org) + admins.append(org_admin) + + inv = Inventory.objects.create( + organization=orgs[0], + name='inv{}'.format(i) + ) + + jt = JobTemplate.objects.create(name='foo', inventory=inv) + assert admins[0] in jt.read_role + assert admins[1] not in jt.read_role + + inv.organization = orgs[1] + inv.save(update_fields=['organization']) + assert admins[0] not in jt.read_role + assert admins[1] in jt.read_role diff --git a/awx/main/tests/functional/test_rbac_migration.py b/awx/main/tests/functional/test_rbac_migration.py index 19693ed5e7..48a757f5ae 100644 --- a/awx/main/tests/functional/test_rbac_migration.py +++ b/awx/main/tests/functional/test_rbac_migration.py @@ -1,11 +1,14 @@ import pytest +from django.apps import apps + from awx.main.migrations import _rbac as rbac from awx.main.models import ( UnifiedJobTemplate, InventorySource, Inventory, JobTemplate, Project, - Organization + Organization, + User ) @@ -62,3 +65,37 @@ def test_implied_organization_subquery_job_template(): assert jt.test_field is None else: assert jt.test_field == jt.project.organization_id + + +@pytest.mark.django_db +def test_give_explicit_inventory_permission(): + dual_admin = User.objects.create(username='alice') + inv_admin = User.objects.create(username='bob') + inv_org = Organization.objects.create(name='inv-org') + proj_org = Organization.objects.create(name='proj-org') + + inv_org.admin_role.members.add(inv_admin, dual_admin) + proj_org.admin_role.members.add(dual_admin) + + proj = Project.objects.create( + name="test-proj", + organization=proj_org + ) + inv = Inventory.objects.create( + name='test-inv', + organization=inv_org + ) + + jt = JobTemplate.objects.create( + name='foo', + project=proj, + inventory=inv + ) + + assert dual_admin in jt.admin_role + + rbac.restore_inventory_admins(apps, None) + + assert inv_admin in jt.admin_role.members.all() + assert dual_admin not in jt.admin_role.members.all() + assert dual_admin in jt.admin_role diff --git a/awx/main/tests/unit/models/test_unified_job_unit.py b/awx/main/tests/unit/models/test_unified_job_unit.py index 4442770188..a3f9123f37 100644 --- a/awx/main/tests/unit/models/test_unified_job_unit.py +++ b/awx/main/tests/unit/models/test_unified_job_unit.py @@ -6,6 +6,7 @@ from awx.main.models import ( UnifiedJobTemplate, WorkflowJob, WorkflowJobNode, + WorkflowApprovalTemplate, Job, User, Project, @@ -70,7 +71,9 @@ def test_organization_copy_to_jobs(): All unified job types should infer their organization from their template organization ''' for cls in UnifiedJobTemplate.__subclasses__(): - assert 'organization' in cls._get_unified_job_field_names() + if cls is WorkflowApprovalTemplate: + continue # these do not track organization + assert 'organization' in cls._get_unified_job_field_names(), cls def test_log_representation(): diff --git a/awx/main/utils/named_url_graph.py b/awx/main/utils/named_url_graph.py index f17c503d4c..6a9aeb2b85 100644 --- a/awx/main/utils/named_url_graph.py +++ b/awx/main/utils/named_url_graph.py @@ -315,3 +315,8 @@ def generate_graph(models): settings.NAMED_URL_GRAPH = largest_graph for node in settings.NAMED_URL_GRAPH.values(): node.add_bindings() + + +def reset_counters(): + for node in settings.NAMED_URL_GRAPH.values(): + node.counter = 0 diff --git a/awxkit/awxkit/api/pages/job_templates.py b/awxkit/awxkit/api/pages/job_templates.py index 4060da5f2c..11d46cfbfa 100644 --- a/awxkit/awxkit/api/pages/job_templates.py +++ b/awxkit/awxkit/api/pages/job_templates.py @@ -7,7 +7,7 @@ from awxkit.utils import ( suppress, update_payload, PseudoNamespace) -from awxkit.api.pages import Credential, Inventory, Project, UnifiedJobTemplate, Organization +from awxkit.api.pages import Credential, Inventory, Project, UnifiedJobTemplate from awxkit.api.mixins import HasCreate, HasInstanceGroups, HasNotifications, HasSurvey, HasCopy, DSAdapter from awxkit.api.resources import resources import awxkit.exceptions as exc @@ -23,7 +23,7 @@ class JobTemplate( HasSurvey, UnifiedJobTemplate): - optional_dependencies = [Organization, Inventory, Credential, Project] + optional_dependencies = [Inventory, Credential, Project] def launch(self, payload={}): """Launch the job_template using related->launch endpoint.""" @@ -129,7 +129,6 @@ class JobTemplate( playbook='ping.yml', credential=Credential, inventory=Inventory, - organization=Organization, project=None, **kwargs): if not project: @@ -149,18 +148,12 @@ class JobTemplate( project = self.ds.project if project else None inventory = self.ds.inventory if inventory else None credential = self.ds.credential if credential else None - # if the created project has an organization, and the parameters - # specified no organization, then borrow the one from the project - if hasattr(project.ds, 'organization') and organization is Organization: - self.ds.organization = project.ds.organization - organization = self.ds.organization payload = self.payload( name=name, description=description, job_type=job_type, playbook=playbook, - organization=organization, credential=credential, inventory=inventory, project=project, @@ -176,12 +169,11 @@ class JobTemplate( playbook='ping.yml', credential=Credential, inventory=Inventory, - organization=Organization, project=None, **kwargs): payload, credential = self.create_payload(name=name, description=description, job_type=job_type, playbook=playbook, credential=credential, inventory=inventory, - project=project, organization=organization, **kwargs) + project=project, **kwargs) ret = self.update_identity( JobTemplates( self.connection).post(payload)) diff --git a/awxkit/awxkit/api/pages/workflow_job_templates.py b/awxkit/awxkit/api/pages/workflow_job_templates.py index b5b169340e..1d67a0a171 100644 --- a/awxkit/awxkit/api/pages/workflow_job_templates.py +++ b/awxkit/awxkit/api/pages/workflow_job_templates.py @@ -12,7 +12,7 @@ from . import page class WorkflowJobTemplate(HasCopy, HasCreate, HasNotifications, HasSurvey, UnifiedJobTemplate): - dependencies = [Organization] + optional_dependencies = [Organization] def launch(self, payload={}): """Launch using related->launch endpoint.""" @@ -71,14 +71,14 @@ class WorkflowJobTemplate(HasCopy, HasCreate, HasNotifications, HasSurvey, Unifi return payload - def create_payload(self, name='', description='', organization=Organization, **kwargs): + def create_payload(self, name='', description='', organization=None, **kwargs): self.create_and_update_dependencies(*filter_by_class((organization, Organization))) organization = self.ds.organization if organization else None payload = self.payload(name=name, description=description, organization=organization, **kwargs) payload.ds = DSAdapter(self.__class__.__name__, self._dependency_store) return payload - def create(self, name='', description='', organization=Organization, **kwargs): + def create(self, name='', description='', organization=None, **kwargs): payload = self.create_payload(name=name, description=description, organization=organization, **kwargs) return self.update_identity(WorkflowJobTemplates(self.connection).post(payload)) diff --git a/awxkit/awxkit/api/resources.py b/awxkit/awxkit/api/resources.py index 97a9c31c90..d317bcc55d 100644 --- a/awxkit/awxkit/api/resources.py +++ b/awxkit/awxkit/api/resources.py @@ -63,6 +63,7 @@ class Resources(object): _inventory_related_root_groups = r'inventories/\d+/root_groups/' _inventory_related_script = r'inventories/\d+/script/' _inventory_related_update_inventory_sources = r'inventories/\d+/update_inventory_sources/' + _inventory_scan_job_templates = r'inventories/\d+/scan_job_templates/' _inventory_script = r'inventory_scripts/\d+/' _inventory_script_copy = r'inventory_scripts/\d+/copy/' _inventory_scripts = 'inventory_scripts/' From 75477937927826c23ce6c9615041dc47abc6b4d6 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Wed, 5 Feb 2020 15:51:55 -0500 Subject: [PATCH 3/6] add organization to template views in old and new ui --- .../client/features/templates/templates.strings.js | 1 + .../features/templates/templatesList.view.html | 5 +++++ awx/ui/client/src/projects/projects.form.js | 1 + .../JobTemplateDetail/JobTemplateDetail.jsx | 14 ++++++++++++++ 4 files changed, 21 insertions(+) diff --git a/awx/ui/client/features/templates/templates.strings.js b/awx/ui/client/features/templates/templates.strings.js index 68c97c57f7..80dc845a4f 100644 --- a/awx/ui/client/features/templates/templates.strings.js +++ b/awx/ui/client/features/templates/templates.strings.js @@ -13,6 +13,7 @@ function TemplatesStrings (BaseString) { ADD_DD_JT_LABEL: t.s('Job Template'), ADD_DD_WF_LABEL: t.s('Workflow Template'), OPEN_WORKFLOW_VISUALIZER: t.s('Click here to open the workflow visualizer'), + ROW_ITEM_LABEL_ORGANIZATION: t.s('Organization'), ROW_ITEM_LABEL_DESCRIPTION: t.s('Description'), ROW_ITEM_LABEL_ACTIVITY: t.s('Activity'), ROW_ITEM_LABEL_INVENTORY: t.s('Inventory'), diff --git a/awx/ui/client/features/templates/templatesList.view.html b/awx/ui/client/features/templates/templatesList.view.html index 8ed34895a7..a50b040316 100644 --- a/awx/ui/client/features/templates/templatesList.view.html +++ b/awx/ui/client/features/templates/templatesList.view.html @@ -91,6 +91,11 @@ label-value="{{:: vm.strings.get('list.ROW_ITEM_LABEL_DESCRIPTION') }}" value="{{ template.description | sanitize }}"> + + ' + i18n._('When this project is used by a Job Template, Organization cannot be changed.') + '

', dataContainer: 'body', dataPlacement: 'right', ngDisabled: '!(project_obj.summary_fields.user_capabilities.edit || canAdd) || !canEditOrg', diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 0b8a118f4a..d5a14bce6f 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -168,6 +168,20 @@ function JobTemplateDetail({ i18n, template }) { + {summary_fields.organization ? ( + + {summary_fields.organization.name} + + } + /> + ) : ( + renderMissingDataDetail(i18n._(t`Project`)) + )} {summary_fields.inventory ? ( Date: Thu, 6 Feb 2020 11:37:45 -0500 Subject: [PATCH 4/6] Various JT.organization cleanup items cleanup from PR review suggestions bump migration number fix test revert change to old-app JT form no longer needed --- awx/main/fields.py | 1 - ...=> 0109_v370_job_template_organization_field.py} | 2 +- awx/main/models/projects.py | 4 ++-- .../api/test_deprecated_credential_assignment.py | 1 - .../functional/api/test_unified_job_template.py | 1 - awx/main/tests/functional/test_instances.py | 4 ++-- awx/main/tests/functional/test_rbac_job.py | 2 -- .../templates/job_templates/job-template.form.js | 13 ------------- awxkit/awxkit/api/resources.py | 1 - 9 files changed, 5 insertions(+), 24 deletions(-) rename awx/main/migrations/{0107_v370_job_template_organization_field.py => 0109_v370_job_template_organization_field.py} (98%) diff --git a/awx/main/fields.py b/awx/main/fields.py index 36ba0c8394..4e854bbb8b 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -199,7 +199,6 @@ def update_role_parentage_for_instance(instance): updates the parents listing for all the roles of a given instance if they have changed ''' - changed_ct = 0 parents_removed = set() parents_added = set() for implicit_role_field in getattr(instance.__class__, '__implicit_role_fields'): diff --git a/awx/main/migrations/0107_v370_job_template_organization_field.py b/awx/main/migrations/0109_v370_job_template_organization_field.py similarity index 98% rename from awx/main/migrations/0107_v370_job_template_organization_field.py rename to awx/main/migrations/0109_v370_job_template_organization_field.py index 11fd247f5c..505538594a 100644 --- a/awx/main/migrations/0107_v370_job_template_organization_field.py +++ b/awx/main/migrations/0109_v370_job_template_organization_field.py @@ -19,7 +19,7 @@ def rebuild_jt_parents(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('main', '0106_v370_remove_inventory_groups_with_active_failures'), + ('main', '0108_v370_unifiedjob_dependencies_processed'), ] operations = [ diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index aad612ebd8..0207fec97b 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -584,8 +584,8 @@ class ProjectUpdate(UnifiedJob, ProjectOptions, JobNotificationMixin, TaskManage @property def preferred_instance_groups(self): - if self.project is not None and self.project.organization is not None: - organization_groups = [x for x in self.project.organization.instance_groups.all()] + if self.organization is not None: + organization_groups = [x for x in self.organization.instance_groups.all()] else: organization_groups = [] template_groups = [x for x in super(ProjectUpdate, self).preferred_instance_groups] diff --git a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py index b837af5a84..880b7ff892 100644 --- a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py +++ b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py @@ -278,7 +278,6 @@ def test_multi_vault_preserved_on_put(get, put, admin_user, job_template, vault_ job_template.credentials.add(vault_credential, vault2) assert job_template.credentials.count() == 2 # sanity check r = get(job_template.get_absolute_url(), admin_user, expect=200) - r.data.pop('organization') # so that it passes validation # should be a no-op PUT request put( job_template.get_absolute_url(), diff --git a/awx/main/tests/functional/api/test_unified_job_template.py b/awx/main/tests/functional/api/test_unified_job_template.py index c2df65b49f..3aa6c4024e 100644 --- a/awx/main/tests/functional/api/test_unified_job_template.py +++ b/awx/main/tests/functional/api/test_unified_job_template.py @@ -2,7 +2,6 @@ import pytest from awx.api.versioning import reverse from awx.main import models -from awx.main.utils import get_type_for_model @pytest.mark.django_db diff --git a/awx/main/tests/functional/test_instances.py b/awx/main/tests/functional/test_instances.py index 67331a6973..927e5fb440 100644 --- a/awx/main/tests/functional/test_instances.py +++ b/awx/main/tests/functional/test_instances.py @@ -1,7 +1,7 @@ import pytest from unittest import mock -from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate +from awx.main.models import AdHocCommand, InventoryUpdate, JobTemplate, ProjectUpdate from awx.main.models.ha import Instance, InstanceGroup from awx.main.tasks import apply_cluster_membership_policies from awx.api.versioning import reverse @@ -310,7 +310,7 @@ class TestInstanceGroupOrdering: assert iu.preferred_instance_groups == [ig_inv, ig_org] def test_project_update_instance_groups(self, instance_group_factory, project, default_instance_group): - pu = ProjectUpdate.objects.create(project=project) + pu = ProjectUpdate.objects.create(project=project, organization=project.organization) assert pu.preferred_instance_groups == [default_instance_group] ig_org = instance_group_factory("OrgIstGrp", [default_instance_group.instances.first()]) ig_tmp = instance_group_factory("TmpIstGrp", [default_instance_group.instances.first()]) diff --git a/awx/main/tests/functional/test_rbac_job.py b/awx/main/tests/functional/test_rbac_job.py index 1c096e74c7..ed3aee47cb 100644 --- a/awx/main/tests/functional/test_rbac_job.py +++ b/awx/main/tests/functional/test_rbac_job.py @@ -21,8 +21,6 @@ from awx.main.models import ( Credential ) -from rest_framework.exceptions import PermissionDenied - from crum import impersonate diff --git a/awx/ui/client/src/templates/job_templates/job-template.form.js b/awx/ui/client/src/templates/job_templates/job-template.form.js index 953dcbbd55..6f1f8cf025 100644 --- a/awx/ui/client/src/templates/job_templates/job-template.form.js +++ b/awx/ui/client/src/templates/job_templates/job-template.form.js @@ -43,19 +43,6 @@ function(NotificationsList, i18n) { column: 1, ngDisabled: '!(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)' }, - organization: { - label: i18n._('Organization'), - type: 'lookup', - list: 'OrganizationList', - sourceModel: 'organization', - basePath: 'organizations', - sourceField: 'name', - dataTitle: i18n._('Organization'), - dataContainer: 'body', - dataPlacement: 'right', - ngDisabled: '!(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)', - awLookupWhen: '(job_template_obj.summary_fields.user_capabilities.edit || canAddJobTemplate)' - }, job_type: { label: i18n._('Job Type'), type: 'select', diff --git a/awxkit/awxkit/api/resources.py b/awxkit/awxkit/api/resources.py index d317bcc55d..97a9c31c90 100644 --- a/awxkit/awxkit/api/resources.py +++ b/awxkit/awxkit/api/resources.py @@ -63,7 +63,6 @@ class Resources(object): _inventory_related_root_groups = r'inventories/\d+/root_groups/' _inventory_related_script = r'inventories/\d+/script/' _inventory_related_update_inventory_sources = r'inventories/\d+/update_inventory_sources/' - _inventory_scan_job_templates = r'inventories/\d+/scan_job_templates/' _inventory_script = r'inventory_scripts/\d+/' _inventory_script_copy = r'inventory_scripts/\d+/copy/' _inventory_scripts = 'inventory_scripts/' From 7e6def8acc5ecda89f31d42026fc2518663ea877 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 11 Mar 2020 15:50:00 -0400 Subject: [PATCH 5/6] Grant org JT admin role in migration as well --- awx/main/migrations/_rbac.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 4a1798176c..0b1f81953c 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -159,14 +159,17 @@ def _restore_inventory_admins(apps, schema_editor, backward=False): jt_qs = jt_qs.only('id', 'admin_role_id', 'execute_role_id', 'inventory_id') for jt in jt_qs.iterator(): org = jt.inventory.organization - for role_name in ('admin_role', 'execute_role'): - role_id = getattr(jt, '{}_id'.format(role_name)) + for jt_role, org_roles in ( + ('admin_role', ('admin_role', 'job_template_admin_role',)), + ('execute_role', ('execute_role',)) + ): + role_id = getattr(jt, '{}_id'.format(jt_role)) user_qs = User.objects if not backward: # In this specific case, the name for the org role and JT roles were the same - org_role_id = getattr(org, '{}_id'.format(role_name)) - user_qs = user_qs.filter(roles=org_role_id) + org_role_ids = [getattr(org, '{}_id'.format(role_name)) for role_name in org_roles] + user_qs = user_qs.filter(roles__in=org_role_ids) # bizarre migration behavior - ancestors / descendents of # migration version of Role model is reversed, using current model briefly ancestor_ids = list( @@ -185,10 +188,10 @@ def _restore_inventory_admins(apps, schema_editor, backward=False): if not user_ids: continue - role = getattr(jt, role_name) + role = getattr(jt, jt_role) logger.debug('{} {} on jt {} for users {} via inventory.organization {}'.format( 'Removing' if backward else 'Setting', - role_name, jt.pk, user_ids, org.pk + jt_role, jt.pk, user_ids, org.pk )) if not backward: # in reverse, explit role becomes redundant From 521cda878e9d31265dc1b119312a7ab30cffc825 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 16 Mar 2020 10:04:18 -0400 Subject: [PATCH 6/6] Add named URL docs for uniqueness functionality --- docs/named_url.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/named_url.md b/docs/named_url.md index 880b792d57..fc2fe29711 100644 --- a/docs/named_url.md +++ b/docs/named_url.md @@ -8,7 +8,7 @@ There are two named-URL-related Tower configuration settings available under `/a `NAMED_URL_FORMATS` is a *read only* key-value pair list of all available named URL identifier formats. A typical `NAMED_URL_FORMATS` looks like this: ``` "NAMED_URL_FORMATS": { - "job_templates": "", + "job_templates": "++", "workflow_job_templates": "", "inventories": "++", "users": "", @@ -78,6 +78,15 @@ Module `awx.main.utils.named_url_graph` stands at the core of named URL implemen `generate_graph` will run only once for each Tower WSGI process. This is guaranteed by putting the function call inside `__init__` of `URLModificationMiddleware`. When an incoming request enters `URLModificationMiddleware`, the part of its URL path that could contain a valid named URL identifier is extracted and processed to find (possible) corresponding resource objects. The internal process is basically crawling against part of the named URL graph. If the object is found, the identifier part of the URL path is converted to the object's primary key. Going forward, Tower can treat the request with the old-styled URL. +## Job Template Organization Changes + +The `organization` field was added as a read-only field to job templates, derived from its project organization. +This changed the named URL of job templates, to be compatible with multiple job templates with the same +name, but in different organizations. + +To avoid making a backward-incompatible change, using the old named URL is still supported. +That means that you can still reference job templates by the `"job_templates": ""` scheme. +If multiple job templates with the same name exist, the oldest one will be returned. ## Acceptance Criteria