From e531bc67e448cb9e2fb2a86107345d19effabeb5 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 26 May 2016 14:39:16 -0400 Subject: [PATCH] Better control what JT admins are allowed to do This addresses #1981 which says that JT admins can make modifications to a job template freely if they're just changing non functional things like name, description, forks, verbosity, etc, while requiring them to have access to all functional components if they're going to make any changes to the functionality - in specific, any changes to the inventory, project, playbook, or credentials requires that the user have the appropriate use access on all of those things in order to make the change. --- awx/api/serializers.py | 2 +- awx/main/access.py | 53 ++++++++- awx/main/tests/factories/fixtures.py | 2 +- .../functional/api/test_job_templates.py | 109 ++++++++++++++++++ 4 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 awx/main/tests/functional/api/test_job_templates.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 0280b0f09f..ce203c533c 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1766,7 +1766,7 @@ class JobTemplateSerializer(UnifiedJobTemplateSerializer, JobOptionsSerializer): notification_templates_any = reverse('api:job_template_notification_templates_any_list', args=(obj.pk,)), notification_templates_success = reverse('api:job_template_notification_templates_success_list', args=(obj.pk,)), notification_templates_error = reverse('api:job_template_notification_templates_error_list', args=(obj.pk,)), - access_list = reverse('api:job_template_access_list', args=(obj.pk,)), + access_list = reverse('api:job_template_access_list', args=(obj.pk,)), survey_spec = reverse('api:job_template_survey_spec', args=(obj.pk,)), labels = reverse('api:job_template_label_list', args=(obj.pk,)), roles = reverse('api:job_template_roles_list', args=(obj.pk,)), diff --git a/awx/main/access.py b/awx/main/access.py index 7e2289df42..c58da4bcd9 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -817,13 +817,64 @@ class JobTemplateAccess(BaseAccess): if self.user not in obj.admin_role: return False if data is not None: - data_for_change = dict(data) + data = dict(data) + + if self.changes_are_non_sensitive(obj, data): + if 'job_type' in data and data['job_type'] == PERM_INVENTORY_SCAN: + self.check_license(feature='system_tracking') + + if 'survey_enabled' in data and data['survey_enabled']: + self.check_license(feature='surveys') + print('Changes are non senstivie') + return True + print('Changes were senstivie') + for required_field in ('credential', 'cloud_credential', '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) + def changes_are_non_sensitive(self, obj, data): + ''' + Returne true if the changes being made are considered nonsensitive, and + thus can be made by a job template administrator which may not have access + to the any inventory, project, or credentials associated with the template. + ''' + # We are white listing fields that can + field_whitelist = [ + 'name', 'description', 'forks', 'limit', 'verbosity', 'extra_vars', + 'job_tags', 'force_handlers', 'skip_tags', 'ask_variables_on_launch', + 'ask_tags_on_launch', 'ask_job_type_on_launch', 'ask_inventory_on_launch', + 'ask_credential_on_launch', 'survey_enabled' + ] + + for k, v in data.items(): + if hasattr(obj, k) and getattr(obj, k) != v: + if k not in field_whitelist: + return False + return True + + def can_update_sensitive_fields(self, obj, data): + project_id = data.get('project', obj.project.id if obj.project else None) + inventory_id = data.get('inventory', obj.inventory.id if obj.inventory else None) + credential_id = data.get('credential', obj.credential.id if obj.credential else None) + cloud_credential_id = data.get('cloud_credential', obj.cloud_credential.id if obj.cloud_credential else None) + network_credential_id = data.get('network_credential', obj.network_credential.id if obj.network_credential else None) + + if project_id and self.user not in Project.objects.get(pk=project_id).use_role: + return False + if inventory_id and self.user not in Inventory.objects.get(pk=inventory_id).use_role: + return False + if credential_id and self.user not in Credential.objects.get(pk=credential_id).use_role: + return False + if cloud_credential_id and self.user not in Credential.objects.get(pk=cloud_credential_id).use_role: + return False + if network_credential_id and self.user not in Credential.objects.get(pk=network_credential_id).use_role: + return False + + return True + def can_delete(self, obj): return self.user in obj.admin_role diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py index 91ead460db..7273997ba5 100644 --- a/awx/main/tests/factories/fixtures.py +++ b/awx/main/tests/factories/fixtures.py @@ -107,7 +107,7 @@ def mk_job_template(name, job_type='run', organization=None, inventory=None, credential=None, persisted=True, project=None): - jt = JobTemplate(name=name, job_type=job_type) + jt = JobTemplate(name=name, job_type=job_type, playbook='mocked') jt.inventory = inventory if jt.inventory is None: diff --git a/awx/main/tests/functional/api/test_job_templates.py b/awx/main/tests/functional/api/test_job_templates.py new file mode 100644 index 0000000000..cb09ced593 --- /dev/null +++ b/awx/main/tests/functional/api/test_job_templates.py @@ -0,0 +1,109 @@ +import mock # noqa +import pytest +from awx.main.models.projects import ProjectOptions + + +from django.core.urlresolvers import reverse + +def decorators(func): + @property + def project_playbooks(self): + return ['mocked', 'othermocked'] + + return pytest.mark.django_db(mock.patch.object(ProjectOptions, "playbooks", project_playbooks)(func)) + +@decorators +@pytest.mark.parametrize( + "grant_project, grant_credential, grant_inventory, expect", [ + (True, True, True, 201), + (True, True, False, 403), + (True, False, True, 403), + (False, True, True, 403), + ] +) +def test_create(post, project, machine_credential, inventory, alice, grant_project, grant_credential, grant_inventory, expect): + if grant_project: + project.use_role.members.add(alice) + if grant_credential: + machine_credential.use_role.members.add(alice) + if grant_inventory: + inventory.use_role.members.add(alice) + + post(reverse('api:job_template_list'), { + 'name': 'Some name', + 'project': project.id, + 'credential': machine_credential.id, + 'inventory': inventory.id, + 'playbook': 'mocked', + }, alice, expect=expect) + +@decorators +@pytest.mark.parametrize( + "grant_project, grant_credential, grant_inventory, expect", [ + (True, True, True, 200), + (True, True, False, 403), + (True, False, True, 403), + (False, True, True, 403), + ] +) +def test_edit_sensitive_fields(patch, job_template_factory, alice, grant_project, grant_credential, grant_inventory, expect): + objs = job_template_factory('jt', organization='org1', project='prj', inventory='inv', credential='cred') + objs.job_template.admin_role.members.add(alice) + + if grant_project: + objs.project.use_role.members.add(alice) + if grant_credential: + objs.credential.use_role.members.add(alice) + if grant_inventory: + objs.inventory.use_role.members.add(alice) + + patch(reverse('api:job_template_detail', args=(objs.job_template.id,)), { + 'name': 'Some name', + 'project': objs.project.id, + 'credential': objs.credential.id, + 'inventory': objs.inventory.id, + 'playbook': 'othermocked', + }, alice, expect=expect) + +@decorators +def test_edit_playbook(patch, job_template_factory, alice): + objs = job_template_factory('jt', organization='org1', project='prj', inventory='inv', credential='cred') + objs.job_template.admin_role.members.add(alice) + objs.project.use_role.members.add(alice) + objs.credential.use_role.members.add(alice) + objs.inventory.use_role.members.add(alice) + + patch(reverse('api:job_template_detail', args=(objs.job_template.id,)), { + 'playbook': 'othermocked', + }, alice, expect=200) + + objs.inventory.use_role.members.remove(alice) + patch(reverse('api:job_template_detail', args=(objs.job_template.id,)), { + 'playbook': 'mocked', + }, alice, expect=403) + +@decorators +def test_edit_nonsenstive(patch, job_template_factory, alice): + objs = job_template_factory('jt', organization='org1', project='prj', inventory='inv', credential='cred') + jt = objs.job_template + jt.admin_role.members.add(alice) + + res = patch(reverse('api:job_template_detail', args=(jt.id,)), { + 'name': 'updated', + 'description': 'bar', + 'forks': 14, + 'limit': 'something', + 'verbosity': 5, + 'extra_vars': '--', + 'job_tags': 'sometags', + 'force_handlers': True, + 'skip_tags': True, + 'ask_variables_on_launch':True, + 'ask_tags_on_launch':True, + 'ask_job_type_on_launch':True, + 'ask_inventory_on_launch':True, + 'ask_credential_on_launch': True, + 'survey_enabled':True, + }, alice, expect=200) + print(res.data) + assert res.data['name'] == 'updated'