From ffc0b05a80e32f3dba16d445b1554f3c31d704a0 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 14 Jun 2016 12:20:02 -0400 Subject: [PATCH] fix non-sensitive change becoming sensitive, add network cred in JT can_change --- awx/main/access.py | 4 +- awx/main/tests/factories/fixtures.py | 6 ++- awx/main/tests/factories/tower.py | 1 + awx/main/tests/unit/test_access.py | 55 ++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index a12c4b1426..c076a90224 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -841,7 +841,7 @@ class JobTemplateAccess(BaseAccess): self.check_license(feature='surveys') return True - for required_field in ('credential', 'cloud_credential', 'inventory', 'project'): + for required_field in ('credential', 'cloud_credential', 'network_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 @@ -863,7 +863,7 @@ class JobTemplateAccess(BaseAccess): for k, v in data.items(): if hasattr(obj, k) and getattr(obj, k) != v: - if k not in field_whitelist: + if k not in field_whitelist and v != getattr(obj, '%s_id' % k, None): return False return True diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py index 008f3fca4b..feca114410 100644 --- a/awx/main/tests/factories/fixtures.py +++ b/awx/main/tests/factories/fixtures.py @@ -123,7 +123,8 @@ def mk_job(job_type='run', status='new', job_template=None, inventory=None, def mk_job_template(name, job_type='run', organization=None, inventory=None, - credential=None, persisted=True, extra_vars='', + credential=None, network_credential=None, + cloud_credential=None, persisted=True, extra_vars='', project=None, spec=None): if extra_vars: extra_vars = json.dumps(extra_vars) @@ -139,6 +140,9 @@ def mk_job_template(name, job_type='run', if jt.credential is None: jt.ask_credential_on_launch = True + jt.network_credential = network_credential + jt.cloud_credential = cloud_credential + jt.project = project jt.survey_spec = spec diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py index 59af0b997d..8116ec83bf 100644 --- a/awx/main/tests/factories/tower.py +++ b/awx/main/tests/factories/tower.py @@ -229,6 +229,7 @@ def create_job_template(name, roles=None, persisted=True, **kwargs): 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) diff --git a/awx/main/tests/unit/test_access.py b/awx/main/tests/unit/test_access.py index 4885f334cd..289c923f0d 100644 --- a/awx/main/tests/unit/test_access.py +++ b/awx/main/tests/unit/test_access.py @@ -1,10 +1,32 @@ +import pytest +import mock + from django.contrib.auth.models import User +from django.forms.models import model_to_dict + from awx.main.access import ( BaseAccess, check_superuser, + JobTemplateAccess, ) +from awx.main.models import Credential, Inventory, Project, Role +@pytest.fixture +def job_template_with_ids(job_template_factory): + # Create non-persisted objects with IDs to send to job_template_factory + credential = Credential(id=1, pk=1, name='testcred', kind='ssh') + net_cred = Credential(id=2, pk=2, name='testnetcred', kind='net') + cloud_cred = Credential(id=3, pk=3, name='testcloudcred', kind='aws') + inv = Inventory(id=11, pk=11, name='testinv') + proj = Project(id=14, pk=14, name='testproj') + + jt_objects = 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 + def test_superuser(mocker): user = mocker.MagicMock(spec=User, id=1, is_superuser=True) access = BaseAccess(user) @@ -19,3 +41,36 @@ def test_not_superuser(mocker): can_add = check_superuser(BaseAccess.can_add) assert can_add(access, None) is False +def test_jt_existing_values_are_nonsensitive(job_template_with_ids): + """Assure that permission checks are not required if submitted data is + identical to what the job template already has.""" + + data = model_to_dict(job_template_with_ids) + rando = User(username='rando', password='raginrando', email='rando@redhat.com') + access = JobTemplateAccess(rando) + + assert access.changes_are_non_sensitive(job_template_with_ids, data) + +def test_change_jt_sensitive_data(job_template_with_ids, mocker): + """Assure that can_add is called with all ForeignKeys.""" + + job_template_with_ids.admin_role = Role() + + data = {'inventory': job_template_with_ids.inventory.id + 1} + rando = User(username='rando', password='raginrando', email='rando@redhat.com') + access = JobTemplateAccess(rando) + + 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) + + mock_add.assert_called_once_with({ + 'inventory': data['inventory'], + 'project': job_template_with_ids.project.id, + 'credential': job_template_with_ids.credential.id, + 'cloud_credential': job_template_with_ids.cloud_credential.id, + 'network_credential': job_template_with_ids.network_credential.id + }) +