From 39b410ae3eb9fb78bc9bda04b1f8ea4cbae4d933 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 6 Jun 2016 22:43:18 -0400 Subject: [PATCH] hide passwords in job_args and activity stream --- awx/api/serializers.py | 14 +------ awx/main/models/jobs.py | 17 +++++++- awx/main/signals.py | 7 +++- awx/main/tasks.py | 8 +++- .../tests/functional/api/test_survey_spec.py | 27 ++++++++++--- .../functional/api/test_unified_jobs_view.py | 6 +++ awx/main/tests/functional/conftest.py | 10 +++++ .../unit/models/test_job_template_unit.py | 35 +++++++++++++++- awx/main/tests/unit/models/test_job_unit.py | 40 +++++++++++++++++++ 9 files changed, 142 insertions(+), 22 deletions(-) create mode 100644 awx/main/tests/unit/models/test_job_unit.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 7f486bcd5d..4c4201a32c 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 @@ -1934,17 +1933,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/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/api/test_unified_jobs_view.py b/awx/main/tests/functional/api/test_unified_jobs_view.py index ab9487bc38..a7c1607dd9 100644 --- a/awx/main/tests/functional/api/test_unified_jobs_view.py +++ b/awx/main/tests/functional/api/test_unified_jobs_view.py @@ -1,4 +1,5 @@ import pytest +import json from awx.main.models import UnifiedJob from django.core.urlresolvers import reverse @@ -15,3 +16,8 @@ def test_options_fields_choices(instance, options, user): assert 'choice' == response.data['actions']['GET']['status']['type'] assert UnifiedJob.STATUS_CHOICES == response.data['actions']['GET']['status']['choices'] +@pytest.mark.django_db +@pytest.mark.survey +def test_job_redacted_survey_passwords(job_with_secret_key, get, admin_user): + response = get(reverse('api:job_detail', args=(job_with_secret_key.pk,)), admin_user) + assert json.loads(response.data['extra_vars'])['secret_key'] == '$encrypted$' diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 812e4b1495..bb6f902802 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -185,6 +185,16 @@ def notification_template(organization): notification_type="webhook", notification_configuration=dict(url="http://localhost", headers={"Test": "Header"})) + +@pytest.fixture +def job_with_secret_key(job_template_factory): + "Returns a job from a Job Template with secret_key as password question" + objects = job_template_factory( + 'jt', organization='org1', survey=[{'variable': 'secret_key', 'type': 'password'}]) + job = objects.job_template.jobs.create( + job_type="run", extra_vars=json.dumps({'secret_key': '6kQngg3h8lgiSTvIEb21'})) + return job + @pytest.fixture def admin(user): return user('admin', True) 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..7ed24ce173 100644 --- a/awx/main/tests/unit/models/test_job_template_unit.py +++ b/awx/main/tests/unit/models/test_job_template_unit.py @@ -1,5 +1,18 @@ -from awx.main.tests.factories import create_job_template +import pytest +import json +from awx.main.tests.factories import create_job_template +from awx.main.models.jobs import Job + + +@pytest.fixture +def job_template_sensitive_data(): + return create_job_template( + 'jt', project='prj', persisted=False, + survey=['submitter_email', + {'variable': 'secret_key', 'type': 'password'}, + {'variable': 'SSN', 'type': 'password'}] + ).job_template def test_missing_project_error(): objects = create_job_template( @@ -33,3 +46,23 @@ 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_template_sensitive_data): + """Verify the output of the survey_passwords function + gives a list of survey variable names which are passwords""" + assert job_template_sensitive_data.survey_password_variables() == ['secret_key', 'SSN'] + +@pytest.mark.survey +def test_job_redacted_extra_vars(job_template_sensitive_data): + """Verify that this method redacts vars marked as passwords in a survey""" + job_obj = Job( + job_type="run", job_template=job_template_sensitive_data, + extra_vars=json.dumps({'submitter_email': 'foobar@redhat.com', + 'secret_key': 'b86hpFChM2XSb40Zld9x', + 'SSN': '123-45-6789'})) + assert json.loads(job_obj.display_extra_vars()) == { + 'submitter_email': 'foobar@redhat.com', + 'secret_key': '$encrypted$', + 'SSN': '$encrypted$' + } 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..633f6a0470 --- /dev/null +++ b/awx/main/tests/unit/models/test_job_unit.py @@ -0,0 +1,40 @@ +import pytest +import json + +from awx.main.models.jobs import Job +from awx.main.tasks import RunJob + +from awx.main.tests.factories import create_job_template + + +@pytest.fixture +def job_with_secret_vars(): + job_template = create_job_template( + 'jt', persisted=False, + survey=['submitter_email', + {'variable': 'secret_key', 'type': 'password'}] + ).job_template + job = Job(id=1, job_template=job_template, extra_vars=json.dumps({ + 'submitter_email': 'foobar@redhat.com', + 'secret_key': '6kQngg3h8lgiSTvIEb21' + })) + return job + +def test_job_args_redacted_passwords(job_with_secret_vars): + """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_with_secret_vars, **kwargs) + ev_index = safe_args.index('-e') + 1 + extra_vars = json.loads(safe_args[ev_index]) + assert extra_vars['secret_key'] == '$encrypted$' + assert extra_vars['submitter_email'] == 'foobar@redhat.com' + +def test_job_args_unredacted_passwords(job_with_secret_vars): + kwargs = {'ansible_version': '2.1'} + run_job = RunJob() + safe_args = run_job.build_args(job_with_secret_vars, **kwargs) + ev_index = safe_args.index('-e') + 1 + extra_vars = json.loads(safe_args[ev_index]) + assert extra_vars['secret_key'] == '6kQngg3h8lgiSTvIEb21' + assert extra_vars['submitter_email'] == 'foobar@redhat.com'