From 17ac2cee42542a3bfdbc95dc6ad0565003347ec5 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 5 Aug 2016 15:11:28 -0400 Subject: [PATCH 1/3] Put survey passwords in job field --- awx/api/serializers.py | 2 +- .../0030_v302_job_survey_passwords.py | 20 +++++++++++++++ .../0031_v302_migrate_survey_passwords.py | 18 +++++++++++++ awx/main/migrations/_save_password_keys.py | 25 +++++++++++++++++++ awx/main/models/jobs.py | 21 ++++++++-------- awx/main/models/unified_jobs.py | 7 ++++++ 6 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 awx/main/migrations/0030_v302_job_survey_passwords.py create mode 100644 awx/main/migrations/0031_v302_migrate_survey_passwords.py create mode 100644 awx/main/migrations/_save_password_keys.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 3def9ae19b..fb8cd89db1 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1980,7 +1980,7 @@ 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 and 'extra_vars' in ret: + if 'extra_vars' in ret: ret['extra_vars'] = obj.display_extra_vars() return ret diff --git a/awx/main/migrations/0030_v302_job_survey_passwords.py b/awx/main/migrations/0030_v302_job_survey_passwords.py new file mode 100644 index 0000000000..fa6c2cd3fe --- /dev/null +++ b/awx/main/migrations/0030_v302_job_survey_passwords.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0029_v302_add_ask_skip_tags'), + ] + + operations = [ + migrations.AddField( + model_name='job', + name='survey_passwords', + field=jsonfield.fields.JSONField(default={}, editable=False, blank=True), + ), + ] diff --git a/awx/main/migrations/0031_v302_migrate_survey_passwords.py b/awx/main/migrations/0031_v302_migrate_survey_passwords.py new file mode 100644 index 0000000000..5eac01b853 --- /dev/null +++ b/awx/main/migrations/0031_v302_migrate_survey_passwords.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from awx.main.migrations import _save_password_keys +from awx.main.migrations import _migration_utils as migration_utils +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0030_v302_job_survey_passwords'), + ] + + operations = [ + migrations.RunPython(migration_utils.set_current_apps_for_migrations), + migrations.RunPython(_save_password_keys.migrate_survey_passwords), + ] diff --git a/awx/main/migrations/_save_password_keys.py b/awx/main/migrations/_save_password_keys.py new file mode 100644 index 0000000000..3ff7b17562 --- /dev/null +++ b/awx/main/migrations/_save_password_keys.py @@ -0,0 +1,25 @@ +def survey_password_variables(survey_spec): + vars = [] + # Get variables that are type password + for survey_element in survey_spec['spec']: + if survey_element['type'] == 'password': + vars.append(survey_element['variable']) + return vars + + +def migrate_survey_passwords(apps, schema_editor): + '''Take the output of the Job Template password list for all that + have a survey enabled, and then save it into the job model. + ''' + Job = apps.get_model('main', 'Job') + for job in Job.objects.iterator(): + if not job.job_template: + continue + jt = job.job_template + if jt.survey_spec is not None and jt.survey_enabled: + password_list = survey_password_variables(jt.survey_spec) + hide_password_dict = {} + for password in password_list: + hide_password_dict[password] = "$encrypted$" + job.survey_passwords = hide_password_dict + job.save() diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index a7c1c6041d..8a4ba4e1d3 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -513,6 +513,11 @@ class Job(UnifiedJob, JobOptions): editable=False, through='JobHostSummary', ) + survey_passwords = JSONField( + blank=True, + default={}, + editable=False, + ) @classmethod def _get_parent_field_name(cls): @@ -721,16 +726,12 @@ class Job(UnifiedJob, JobOptions): ''' 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 + if self.survey_passwords: + extra_vars = json.loads(self.extra_vars) + extra_vars.update(self.survey_passwords) + return json.dumps(extra_vars) + else: + return self.extra_vars def _survey_search_and_replace(self, content): # Use job template survey spec to identify password fields. diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index f7106eb7ab..77cafef746 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -343,6 +343,13 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio create_kwargs[field_name] = getattr(self, field_name) new_kwargs = self._update_unified_job_kwargs(**create_kwargs) unified_job = unified_job_class(**new_kwargs) + # For JobTemplate-based jobs with surveys, save list for perma-redaction + if hasattr(self, 'survey_spec') and getattr(self, 'survey_enabled', False): + password_list = self.survey_password_variables() + hide_password_dict = {} + for password in password_list: + hide_password_dict[password] = REPLACE_STR + unified_job.survey_passwords = hide_password_dict unified_job.save() for field_name, src_field_value in m2m_fields.iteritems(): dest_field = getattr(unified_job, field_name) From 9f3d9fa78a338725594f6b693deed0a98fa4b0d6 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 5 Aug 2016 15:59:11 -0400 Subject: [PATCH 2/3] carry over survey passwords from old relaunched job --- awx/main/models/jobs.py | 4 ++-- awx/main/models/unified_jobs.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 8a4ba4e1d3..a04c1ab98e 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, REPLACE_STR +from awx.main.redact import PlainTextCleaner from awx.main.conf import tower_settings from awx.main.fields import ImplicitRoleField from awx.main.models.mixins import ResourceMixin @@ -248,7 +248,7 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): 'playbook', 'credential', 'cloud_credential', 'network_credential', 'forks', 'schedule', 'limit', 'verbosity', 'job_tags', 'extra_vars', 'launch_type', 'force_handlers', 'skip_tags', 'start_at_task', 'become_enabled', - 'labels',] + 'labels', 'survey_passwords'] def resource_validation_data(self): ''' diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 77cafef746..52aa46904f 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -32,7 +32,7 @@ from djcelery.models import TaskMeta from awx.main.models.base import * # noqa from awx.main.models.schedules import Schedule from awx.main.utils import decrypt_field, emit_websocket_notification, _inventory_updates -from awx.main.redact import UriCleaner +from awx.main.redact import UriCleaner, REPLACE_STR __all__ = ['UnifiedJobTemplate', 'UnifiedJob'] @@ -344,7 +344,8 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio new_kwargs = self._update_unified_job_kwargs(**create_kwargs) unified_job = unified_job_class(**new_kwargs) # For JobTemplate-based jobs with surveys, save list for perma-redaction - if hasattr(self, 'survey_spec') and getattr(self, 'survey_enabled', False): + if (hasattr(self, 'survey_spec') and getattr(self, 'survey_enabled', False) and + not getattr(unified_job, 'survey_passwords', False)): password_list = self.survey_password_variables() hide_password_dict = {} for password in password_list: From 6559118f4002596f12c91276798becb14e48d9c9 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 8 Aug 2016 13:06:07 -0400 Subject: [PATCH 3/3] tests for saving survey passwords to job --- awx/main/migrations/_save_password_keys.py | 4 +++- awx/main/tests/conftest.py | 10 ++++----- .../tests/functional/api/test_job_template.py | 21 ++++++++++++++++++- .../tests/functional/api/test_survey_spec.py | 3 ++- awx/main/tests/functional/conftest.py | 4 ++-- .../unit/models/test_job_template_unit.py | 7 ++++--- awx/main/tests/unit/models/test_job_unit.py | 17 ++++++++++++--- 7 files changed, 50 insertions(+), 16 deletions(-) diff --git a/awx/main/migrations/_save_password_keys.py b/awx/main/migrations/_save_password_keys.py index 3ff7b17562..a5a231a92f 100644 --- a/awx/main/migrations/_save_password_keys.py +++ b/awx/main/migrations/_save_password_keys.py @@ -1,8 +1,10 @@ def survey_password_variables(survey_spec): vars = [] # Get variables that are type password + if 'spec' not in survey_spec: + return vars for survey_element in survey_spec['spec']: - if survey_element['type'] == 'password': + if 'type' in survey_element and survey_element['type'] == 'password': vars.append(survey_element['variable']) return vars diff --git a/awx/main/tests/conftest.py b/awx/main/tests/conftest.py index 470f43e661..1f21905fb9 100644 --- a/awx/main/tests/conftest.py +++ b/awx/main/tests/conftest.py @@ -26,16 +26,16 @@ def survey_spec_factory(): return create_survey_spec @pytest.fixture -def job_with_secret_key_factory(job_template_factory): +def job_template_with_survey_passwords_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] + {'variable': 'SSN', 'type': 'password'}], persisted=persisted) + return objects.job_template return rf @pytest.fixture -def job_with_secret_key_unit(job_with_secret_key_factory): - return job_with_secret_key_factory(persisted=False) +def job_template_with_survey_passwords_unit(job_template_with_survey_passwords_factory): + return job_template_with_survey_passwords_factory(persisted=False) diff --git a/awx/main/tests/functional/api/test_job_template.py b/awx/main/tests/functional/api/test_job_template.py index a5a961f88e..88437a0037 100644 --- a/awx/main/tests/functional/api/test_job_template.py +++ b/awx/main/tests/functional/api/test_job_template.py @@ -3,12 +3,14 @@ import mock # AWX from awx.api.serializers import JobTemplateSerializer, JobLaunchSerializer -from awx.main.models.jobs import JobTemplate +from awx.main.models.jobs import JobTemplate, Job from awx.main.models.projects import ProjectOptions +from awx.main.migrations import _save_password_keys as save_password_keys # Django from django.test.client import RequestFactory from django.core.urlresolvers import reverse +from django.apps import apps @property def project_playbooks(self): @@ -348,3 +350,20 @@ def test_disallow_template_delete_on_running_job(job_template_factory, delete, a objects.job_template.create_unified_job() delete_response = delete(reverse('api:job_template_detail', args=[objects.job_template.pk]), user=admin_user) assert delete_response.status_code == 409 + +@pytest.mark.django_db +def test_save_survey_passwords_to_job(job_template_with_survey_passwords): + """Test that when a new job is created, the survey_passwords field is + given all of the passwords that exist in the JT survey""" + job = job_template_with_survey_passwords.create_unified_job() + assert job.survey_passwords == {'SSN': '$encrypted$', 'secret_key': '$encrypted$'} + +@pytest.mark.django_db +def test_save_survey_passwords_on_migration(job_template_with_survey_passwords): + """Test that when upgrading to 3.0.2, the jobs connected to a JT that has + a survey with passwords in it, the survey passwords get saved to the + job survey_passwords field.""" + Job.objects.create(job_template=job_template_with_survey_passwords) + save_password_keys.migrate_survey_passwords(apps, None) + job = job_template_with_survey_passwords.jobs.all()[0] + assert job.survey_passwords == {'SSN': '$encrypted$', 'secret_key': '$encrypted$'} diff --git a/awx/main/tests/functional/api/test_survey_spec.py b/awx/main/tests/functional/api/test_survey_spec.py index dc7071fc11..d6cc512847 100644 --- a/awx/main/tests/functional/api/test_survey_spec.py +++ b/awx/main/tests/functional/api/test_survey_spec.py @@ -193,7 +193,8 @@ def test_launch_with_non_empty_survey_spec_no_license(job_template_factory, post @pytest.mark.django_db @pytest.mark.survey -def test_redact_survey_passwords_in_activity_stream(job_with_secret_key): +def test_redact_survey_passwords_in_activity_stream(job_template_with_survey_passwords): + job_template_with_survey_passwords.create_unified_job() AS_record = ActivityStream.objects.filter(object1='job').all()[0] changes_dict = json.loads(AS_record.changes) extra_vars = json.loads(changes_dict['extra_vars']) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index f970adc2e7..5e67dda1b5 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -206,8 +206,8 @@ def notification(notification_template): subject='email subject') @pytest.fixture -def job_with_secret_key(job_with_secret_key_factory): - return job_with_secret_key_factory(persisted=True) +def job_template_with_survey_passwords(job_template_with_survey_passwords_factory): + return job_template_with_survey_passwords_factory(persisted=True) @pytest.fixture def admin(user): 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 a25cce6f6c..b9a72edea5 100644 --- a/awx/main/tests/unit/models/test_job_template_unit.py +++ b/awx/main/tests/unit/models/test_job_template_unit.py @@ -35,6 +35,7 @@ def test_inventory_credential_contradictions(job_template_factory): 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'] +def test_job_template_survey_password_redaction(job_template_with_survey_passwords_unit): + """Tests the JobTemplate model's funciton to redact passwords from + extra_vars - used when creating a new job""" + assert job_template_with_survey_passwords_unit.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 index a1791c59d5..1b66681dcf 100644 --- a/awx/main/tests/unit/models/test_job_unit.py +++ b/awx/main/tests/unit/models/test_job_unit.py @@ -2,6 +2,7 @@ import pytest import json from awx.main.tasks import RunJob +from awx.main.models import Job @pytest.fixture @@ -14,9 +15,19 @@ def job(mocker): '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()) == { +def test_job_survey_password_redaction(): + """Tests the Job model's funciton to redact passwords from + extra_vars - used when displaying job information""" + job = Job( + name="test-job-with-passwords", + extra_vars=json.dumps({ + 'submitter_email': 'foobar@redhat.com', + 'secret_key': '6kQngg3h8lgiSTvIEb21', + 'SSN': '123-45-6789'}), + survey_passwords={ + 'secret_key': '$encrypted$', + 'SSN': '$encrypted$'}) + assert json.loads(job.display_extra_vars()) == { 'submitter_email': 'foobar@redhat.com', 'secret_key': '$encrypted$', 'SSN': '$encrypted$'}