mirror of
https://github.com/ansible/awx.git
synced 2026-01-14 11:20:39 -03:30
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.
This commit is contained in:
Akita Noek
parent
fed8d49d86
commit
e531bc67e4
@ -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,)),
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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:
|
||||
|
||||
109
awx/main/tests/functional/api/test_job_templates.py
Normal file
109
awx/main/tests/functional/api/test_job_templates.py
Normal file
@ -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'
|
||||
Loading…
x
Reference in New Issue
Block a user