Merge pull request #2075 from anoek/1981

Better control what JT admins are allowed to do

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,)),

awx/main/access.py

@@ -817,13 +817,62 @@ 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 obj.job_type != data['job_type'] and data['job_type'] == PERM_INVENTORY_SCAN:
+                    self.check_license(feature='system_tracking')
+
+                if 'survey_enabled' in data and obj.survey_enabled != data['survey_enabled'] and data['survey_enabled']:
+                    self.check_license(feature='surveys')
+                return True
+
             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
 

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:

awx/main/tests/functional/api/test_job_templates.py

@@ -0,0 +1,111 @@
+import mock # noqa
+import pytest
+from awx.main.models.projects import ProjectOptions
+
+
+from django.core.urlresolvers import reverse
+
+@property
+def project_playbooks(self):
+    return ['mocked.yml', 'alt-mocked.yml']
+
+@pytest.mark.django_db
+@mock.patch.object(ProjectOptions, "playbooks", project_playbooks)
+@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.yml',
+    }, alice, expect=expect)
+
+@pytest.mark.django_db
+@mock.patch.object(ProjectOptions, "playbooks", project_playbooks)
+@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': 'alt-mocked.yml',
+    }, alice, expect=expect)
+
+@pytest.mark.django_db
+@mock.patch.object(ProjectOptions, "playbooks", project_playbooks)
+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': 'alt-mocked.yml',
+    }, alice, expect=200)
+
+    objs.inventory.use_role.members.remove(alice)
+    patch(reverse('api:job_template_detail', args=(objs.job_template.id,)), {
+        'playbook': 'mocked.yml',
+    }, alice, expect=403)
+
+@pytest.mark.django_db
+@mock.patch.object(ProjectOptions, "playbooks", project_playbooks)
+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.playbook = 'mocked.yml'
+    jt.save()
+    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,
+    }, alice, expect=200)
+    print(res.data)
+    assert res.data['name'] == 'updated'