diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 7f295596dc..93e936c8c7 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -40,7 +40,6 @@ from awx.main.constants import SCHEDULEABLE_PROVIDERS from awx.main.models import * # noqa from awx.main.fields import ImplicitRoleField from awx.main.utils import get_type_for_model, get_model_for_type, build_url, timestamp_apiformat, camelcase_to_underscore, getattrd -from awx.main.redact import REPLACE_STR from awx.main.conf import tower_settings from awx.api.license import feature_enabled @@ -1936,17 +1935,8 @@ class JobSerializer(UnifiedJobSerializer, JobOptionsSerializer): return ret if 'job_template' in ret and not obj.job_template: ret['job_template'] = None - - if obj.job_template and obj.job_template.survey_enabled: - if 'extra_vars' in ret: - try: - extra_vars = json.loads(ret['extra_vars']) - for key in obj.job_template.survey_password_variables(): - if key in extra_vars: - extra_vars[key] = REPLACE_STR - ret['extra_vars'] = json.dumps(extra_vars) - except ValueError: - pass + if obj.job_template and obj.job_template.survey_enabled and 'extra_vars' in ret: + ret['extra_vars'] = obj.display_extra_vars() return ret diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index ee55126014..2431e3df4a 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -26,7 +26,7 @@ from awx.main.models.unified_jobs import * # noqa from awx.main.models.notifications import NotificationTemplate from awx.main.utils import decrypt_field, ignore_inventory_computed_fields from awx.main.utils import emit_websocket_notification -from awx.main.redact import PlainTextCleaner +from awx.main.redact import PlainTextCleaner, REPLACE_STR from awx.main.conf import tower_settings from awx.main.fields import ImplicitRoleField from awx.main.models.mixins import ResourceMixin @@ -703,6 +703,21 @@ class Job(UnifiedJob, JobOptions): evars.update(extra_vars) self.update_fields(extra_vars=json.dumps(evars)) + def display_extra_vars(self): + ''' + Hides fields marked as passwords in survey. + ''' + if self.extra_vars and self.job_template and self.job_template.survey_enabled: + try: + extra_vars = json.loads(self.extra_vars) + for key in self.job_template.survey_password_variables(): + if key in extra_vars: + extra_vars[key] = REPLACE_STR + return json.dumps(extra_vars) + except ValueError: + pass + return self.extra_vars + def _survey_search_and_replace(self, content): # Use job template survey spec to identify password fields. # Then lookup password fields in extra_vars and save the values diff --git a/awx/main/signals.py b/awx/main/signals.py index 86dff57df2..40382c98b4 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -318,10 +318,15 @@ def activity_stream_create(sender, instance, created, **kwargs): return # TODO: Rethink details of the new instance object1 = camelcase_to_underscore(instance.__class__.__name__) + changes = model_to_dict(instance, model_serializer_mapping) + # Special case where Job survey password variables need to be hidden + if type(instance) == Job: + if 'extra_vars' in changes: + changes['extra_vars'] = instance.display_extra_vars() activity_entry = ActivityStream( operation='create', object1=object1, - changes=json.dumps(model_to_dict(instance, model_serializer_mapping))) + changes=json.dumps(changes)) activity_entry.save() #TODO: Weird situation where cascade SETNULL doesn't work # it might actually be a good idea to remove all of these FK references since diff --git a/awx/main/tasks.py b/awx/main/tasks.py index c2802cc557..91f0655794 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -932,7 +932,10 @@ class RunJob(BaseTask): 'tower_user_name': job.created_by.username, }) if job.extra_vars_dict: - extra_vars.update(job.extra_vars_dict) + if kwargs.get('display', False) and job.job_template and job.job_template.survey_enabled: + extra_vars.update(json.loads(job.display_extra_vars())) + else: + extra_vars.update(job.extra_vars_dict) args.extend(['-e', json.dumps(extra_vars)]) # Add path to playbook (relative to project.local_path). @@ -942,6 +945,9 @@ class RunJob(BaseTask): args.append(job.playbook) return args + def build_safe_args(self, job, **kwargs): + return self.build_args(job, display=True, **kwargs) + def build_cwd(self, job, **kwargs): if job.project is None and job.job_type == PERM_INVENTORY_SCAN: return self.get_path_to('..', 'playbooks') diff --git a/awx/main/tests/conftest.py b/awx/main/tests/conftest.py index d2a29fc220..470f43e661 100644 --- a/awx/main/tests/conftest.py +++ b/awx/main/tests/conftest.py @@ -25,3 +25,17 @@ def notification_template_factory(): def survey_spec_factory(): return create_survey_spec +@pytest.fixture +def job_with_secret_key_factory(job_template_factory): + def rf(persisted): + "Returns job with linked JT survey with password survey questions" + objects = job_template_factory('jt', organization='org1', survey=[ + {'variable': 'submitter_email', 'type': 'text', 'default': 'foobar@redhat.com'}, + {'variable': 'secret_key', 'default': '6kQngg3h8lgiSTvIEb21', 'type': 'password'}, + {'variable': 'SSN', 'type': 'password'}], jobs=[1], persisted=persisted) + return objects.jobs[1] + return rf + +@pytest.fixture +def job_with_secret_key_unit(job_with_secret_key_factory): + return job_with_secret_key_factory(persisted=False) diff --git a/awx/main/tests/factories/fixtures.py b/awx/main/tests/factories/fixtures.py index 19841349d4..008f3fca4b 100644 --- a/awx/main/tests/factories/fixtures.py +++ b/awx/main/tests/factories/fixtures.py @@ -1,3 +1,5 @@ +import json + from django.contrib.auth.models import User from awx.main.models import ( @@ -6,6 +8,7 @@ from awx.main.models import ( Team, Instance, JobTemplate, + Job, NotificationTemplate, Credential, Inventory, @@ -103,11 +106,30 @@ def mk_inventory(name, organization=None, persisted=True): return inv +def mk_job(job_type='run', status='new', job_template=None, inventory=None, + credential=None, project=None, extra_vars={}, + persisted=True): + job = Job(job_type=job_type, status=status, extra_vars=json.dumps(extra_vars)) + + job.job_template = job_template + job.inventory = inventory + job.credential = credential + job.project = project + + if persisted: + job.save() + return job + + def mk_job_template(name, job_type='run', organization=None, inventory=None, - credential=None, persisted=True, + credential=None, persisted=True, extra_vars='', project=None, spec=None): - jt = JobTemplate(name=name, job_type=job_type, playbook='mocked') + if extra_vars: + extra_vars = json.dumps(extra_vars) + + jt = JobTemplate(name=name, job_type=job_type, extra_vars=extra_vars, + playbook='mocked') 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 612ff80644..5247f4195a 100644 --- a/awx/main/tests/factories/tower.py +++ b/awx/main/tests/factories/tower.py @@ -7,6 +7,7 @@ from awx.main.models import ( NotificationTemplate, Credential, Inventory, + Job, Label, ) @@ -21,6 +22,7 @@ from .fixtures import ( mk_team, mk_user, mk_job_template, + mk_job, mk_credential, mk_inventory, mk_project, @@ -173,7 +175,7 @@ def create_survey_spec(variables=None, default_type='integer', required=True): # def create_job_template(name, roles=None, persisted=True, **kwargs): - Objects = generate_objects(["job_template", + Objects = generate_objects(["job_template", "jobs", "organization", "inventory", "project", @@ -186,7 +188,9 @@ def create_job_template(name, roles=None, persisted=True, **kwargs): inv = None cred = None spec = None + jobs = {} job_type = kwargs.get('job_type', 'run') + extra_vars = kwargs.get('extra_vars', '') if 'organization' in kwargs: org = kwargs['organization'] @@ -213,13 +217,27 @@ def create_job_template(name, roles=None, persisted=True, **kwargs): jt = mk_job_template(name, project=proj, inventory=inv, credential=cred, - job_type=job_type, spec=spec, + job_type=job_type, spec=spec, extra_vars=extra_vars, persisted=persisted) + if 'jobs' in kwargs: + for i in kwargs['jobs']: + if type(i) is Job: + jobs[i.pk] = i + else: + # Fill in default survey answers + job_extra_vars = {} + for question in spec['spec']: + job_extra_vars[question['variable']] = question['default'] + jobs[i] = mk_job(job_template=jt, project=proj, inventory=inv, credential=cred, + extra_vars=job_extra_vars, + job_type=job_type, persisted=persisted) + role_objects = generate_role_objects([org, proj, inv, cred]) apply_roles(roles, role_objects, persisted) return Objects(job_template=jt, + jobs=jobs, project=proj, inventory=inv, credential=cred, diff --git a/awx/main/tests/functional/api/test_survey_spec.py b/awx/main/tests/functional/api/test_survey_spec.py index 3cc19b320f..dc7071fc11 100644 --- a/awx/main/tests/functional/api/test_survey_spec.py +++ b/awx/main/tests/functional/api/test_survey_spec.py @@ -1,9 +1,11 @@ import mock import pytest +import json from django.core.urlresolvers import reverse from awx.main.models.jobs import JobTemplate, Job +from awx.main.models.activity_stream import ActivityStream from awx.api.license import LicenseForbids from awx.main.access import JobTemplateAccess @@ -33,7 +35,7 @@ def test_survey_spec_view_denied(job_template_with_survey, get, admin_user): @pytest.mark.django_db @pytest.mark.survey def test_deny_enabling_survey(deploy_jobtemplate, patch, admin_user): - response = patch(url=reverse('api:job_template_detail', args=(deploy_jobtemplate.id,)), + response = patch(url=deploy_jobtemplate.get_absolute_url(), data=dict(survey_enabled=True), user=admin_user, expect=402) assert response.data['detail'] == 'Feature surveys is not enabled in the active license.' @@ -63,6 +65,7 @@ def test_deny_creating_with_survey(project, post, admin_user): user=admin_user, expect=402) assert response.data['detail'] == 'Feature surveys is not enabled in the active license.' +# Test normal operations with survey license work @mock.patch('awx.api.views.feature_enabled', lambda feature: True) @pytest.mark.django_db @pytest.mark.survey @@ -80,6 +83,7 @@ def test_survey_spec_sucessful_creation(survey_spec_factory, job_template, post, updated_jt = JobTemplate.objects.get(pk=job_template.pk) assert updated_jt.survey_spec == survey_input_data +# Tests related to survey content validation @mock.patch('awx.api.views.feature_enabled', lambda feature: True) @pytest.mark.django_db @pytest.mark.survey @@ -102,6 +106,7 @@ def test_survey_spec_dual_names_error(survey_spec_factory, deploy_jobtemplate, p user=user('admin', True), expect=400) assert response.data['error'] == "'variable' 'submitter_email' duplicated in survey question 1." +# Test actions that should be allowed with non-survey license @mock.patch('awx.main.access.BaseAccess.check_license', new=mock_no_surveys) @pytest.mark.django_db @pytest.mark.survey @@ -165,8 +170,9 @@ def test_launch_survey_enabled_but_no_survey_spec(job_template_factory, post, ad obj = objects.job_template obj.survey_enabled = True obj.save() - post(reverse('api:job_template_launch', args=[obj.pk]), - dict(extra_vars=dict(survey_var=7)), admin_user, expect=201) + response = post(reverse('api:job_template_launch', args=[obj.pk]), + dict(extra_vars=dict(survey_var=7)), admin_user, expect=201) + assert 'survey_var' in response.data['ignored_fields']['extra_vars'] @mock.patch('awx.main.access.BaseAccess.check_license', new=mock_no_surveys) @mock.patch('awx.main.models.unified_jobs.UnifiedJobTemplate.create_unified_job', @@ -174,12 +180,21 @@ def test_launch_survey_enabled_but_no_survey_spec(job_template_factory, post, ad @mock.patch('awx.api.serializers.JobSerializer.to_representation', lambda self, obj: {}) @pytest.mark.django_db @pytest.mark.survey -def test_launch_with_non_empty_survey_spec_no_license(survey_spec_factory, job_template_factory, post, admin_user): +def test_launch_with_non_empty_survey_spec_no_license(job_template_factory, post, admin_user): """Assure jobs can still be launched from JTs with a survey_spec when the survey is diabled.""" objects = job_template_factory('jt', organization='org1', project='prj', - inventory='inv', credential='cred') + inventory='inv', credential='cred', + survey='survey_var') obj = objects.job_template - obj.survey_spec = survey_spec_factory('survey_var') + obj.survey_enabled = False obj.save() post(reverse('api:job_template_launch', args=[obj.pk]), {}, admin_user, expect=201) + +@pytest.mark.django_db +@pytest.mark.survey +def test_redact_survey_passwords_in_activity_stream(job_with_secret_key): + AS_record = ActivityStream.objects.filter(object1='job').all()[0] + changes_dict = json.loads(AS_record.changes) + extra_vars = json.loads(changes_dict['extra_vars']) + assert extra_vars['secret_key'] == '$encrypted$' diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 812e4b1495..3aab7a2537 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -185,6 +185,11 @@ def notification_template(organization): notification_type="webhook", notification_configuration=dict(url="http://localhost", headers={"Test": "Header"})) + +@pytest.fixture +def job_with_secret_key(job_with_secret_key_factory): + return job_with_secret_key_factory(persisted=True) + @pytest.fixture def admin(user): return user('admin', True) diff --git a/awx/main/tests/functional/test_fixture_factories.py b/awx/main/tests/functional/test_fixture_factories.py index bd0e85fc50..bb6dd6cd63 100644 --- a/awx/main/tests/functional/test_fixture_factories.py +++ b/awx/main/tests/functional/test_fixture_factories.py @@ -77,7 +77,8 @@ def test_org_factory(organization_factory): def test_job_template_factory(job_template_factory): jt_objects = job_template_factory('testJT', organization='org1', project='proj1', inventory='inventory1', - credential='cred1', survey='test-survey') + credential='cred1', survey='test-survey', + jobs=[1]) assert jt_objects.job_template.name == 'testJT' assert jt_objects.project.name == 'proj1' assert jt_objects.inventory.name == 'inventory1' @@ -85,6 +86,7 @@ def test_job_template_factory(job_template_factory): assert jt_objects.inventory.organization.name == 'org1' assert jt_objects.job_template.survey_enabled is True assert jt_objects.job_template.survey_spec is not None + assert 'test-survey' in jt_objects.jobs[1].extra_vars def test_survey_spec_generator_simple(survey_spec_factory): survey_spec = survey_spec_factory('survey_variable') diff --git a/awx/main/tests/unit/api/test_serializers.py b/awx/main/tests/unit/api/test_serializers.py index 0b2f41d930..a8fb25d005 100644 --- a/awx/main/tests/unit/api/test_serializers.py +++ b/awx/main/tests/unit/api/test_serializers.py @@ -1,6 +1,7 @@ # Python import pytest import mock +import json # AWX from awx.api.serializers import JobTemplateSerializer, JobSerializer, JobOptionsSerializer @@ -145,6 +146,21 @@ class TestJobSerializerGetRelated(GetRelatedMixin): assert 'job_template' in related assert related['job_template'] == '/api/v1/%s/%d/' % ('job_templates', job.job_template.pk) +@mock.patch('awx.api.serializers.BaseSerializer.to_representation', lambda self,obj: { + 'extra_vars': obj.extra_vars}) +class TestJobSerializerSubstitution(): + + def test_survey_password_hide(self, mocker): + job = mocker.MagicMock(**{ + 'display_extra_vars.return_value': '{\"secret_key\": \"$encrypted$\"}', + 'extra_vars.return_value': '{\"secret_key\": \"my_password\"}'}) + serializer = JobSerializer(job) + rep = serializer.to_representation(job) + extra_vars = json.loads(rep['extra_vars']) + assert extra_vars['secret_key'] == '$encrypted$' + job.display_extra_vars.assert_called_once_with() + assert 'my_password' not in extra_vars + @mock.patch('awx.api.serializers.BaseSerializer.get_summary_fields', lambda x,y: {}) class TestJobOptionsSerializerGetSummaryFields(GetSummaryFieldsMixin): def test__summary_field_labels_10_max(self, mocker, job_template, labels): diff --git a/awx/main/tests/unit/models/test_job_template_unit.py b/awx/main/tests/unit/models/test_job_template_unit.py index df45c753b6..a25cce6f6c 100644 --- a/awx/main/tests/unit/models/test_job_template_unit.py +++ b/awx/main/tests/unit/models/test_job_template_unit.py @@ -1,8 +1,8 @@ -from awx.main.tests.factories import create_job_template +import pytest -def test_missing_project_error(): - objects = create_job_template( +def test_missing_project_error(job_template_factory): + objects = job_template_factory( 'missing-project-jt', organization='org1', inventory='inventory1', @@ -13,8 +13,8 @@ def test_missing_project_error(): validation_errors, resources_needed_to_start = obj.resource_validation_data() assert 'project' in validation_errors -def test_inventory_credential_need_to_start(): - objects = create_job_template( +def test_inventory_credential_need_to_start(job_template_factory): + objects = job_template_factory( 'job-template-few-resources', project='project1', persisted=False) @@ -22,8 +22,8 @@ def test_inventory_credential_need_to_start(): assert 'inventory' in obj.resources_needed_to_start assert 'credential' in obj.resources_needed_to_start -def test_inventory_credential_contradictions(): - objects = create_job_template( +def test_inventory_credential_contradictions(job_template_factory): + objects = job_template_factory( 'job-template-paradox', project='project1', persisted=False) @@ -33,3 +33,8 @@ def test_inventory_credential_contradictions(): validation_errors, resources_needed_to_start = obj.resource_validation_data() assert 'inventory' in validation_errors assert 'credential' in validation_errors + +@pytest.mark.survey +def test_survey_password_list(job_with_secret_key_unit): + """Verify that survey_password_variables method gives a list of survey passwords""" + assert job_with_secret_key_unit.job_template.survey_password_variables() == ['secret_key', 'SSN'] diff --git a/awx/main/tests/unit/models/test_job_unit.py b/awx/main/tests/unit/models/test_job_unit.py new file mode 100644 index 0000000000..a1791c59d5 --- /dev/null +++ b/awx/main/tests/unit/models/test_job_unit.py @@ -0,0 +1,39 @@ +import pytest +import json + +from awx.main.tasks import RunJob + + +@pytest.fixture +def job(mocker): + return mocker.MagicMock(**{ + 'display_extra_vars.return_value': '{\"secret_key\": \"$encrypted$\"}', + 'extra_vars_dict': {"secret_key": "my_password"}, + 'pk': 1, 'job_template.pk': 1, 'job_template.name': '', + 'created_by.pk': 1, 'created_by.username': 'admin', + 'launch_type': 'manual'}) + +@pytest.mark.survey +def test_job_redacted_extra_vars(job_with_secret_key_unit): + """Verify that this method redacts vars marked as passwords in a survey""" + assert json.loads(job_with_secret_key_unit.display_extra_vars()) == { + 'submitter_email': 'foobar@redhat.com', + 'secret_key': '$encrypted$', + 'SSN': '$encrypted$'} + +def test_job_safe_args_redacted_passwords(job): + """Verify that safe_args hides passwords in the job extra_vars""" + kwargs = {'ansible_version': '2.1'} + run_job = RunJob() + safe_args = run_job.build_safe_args(job, **kwargs) + ev_index = safe_args.index('-e') + 1 + extra_vars = json.loads(safe_args[ev_index]) + assert extra_vars['secret_key'] == '$encrypted$' + +def test_job_args_unredacted_passwords(job): + kwargs = {'ansible_version': '2.1'} + run_job = RunJob() + args = run_job.build_args(job, **kwargs) + ev_index = args.index('-e') + 1 + extra_vars = json.loads(args[ev_index]) + assert extra_vars['secret_key'] == 'my_password'