From 39b410ae3eb9fb78bc9bda04b1f8ea4cbae4d933 Mon Sep 17 00:00:00 2001
From: AlanCoding <arominger@ansible.com>
Date: Mon, 6 Jun 2016 22:43:18 -0400
Subject: [PATCH 1/4] 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'

From 04b8eb4eaf4035f5a28d8fa595483d553cf9f4ca Mon Sep 17 00:00:00 2001
From: AlanCoding <arominger@ansible.com>
Date: Wed, 8 Jun 2016 13:25:25 -0400
Subject: [PATCH 2/4] add mk_job to factories, use with survey passwords

---
 awx/main/tests/conftest.py                    | 14 ++++++
 awx/main/tests/factories/fixtures.py          | 26 ++++++++++-
 awx/main/tests/factories/tower.py             | 22 +++++++++-
 awx/main/tests/functional/conftest.py         |  9 +---
 .../functional/test_fixture_factories.py      |  4 +-
 .../unit/models/test_job_template_unit.py     | 43 ++++++-------------
 awx/main/tests/unit/models/test_job_unit.py   | 29 +++----------
 7 files changed, 81 insertions(+), 66 deletions(-)

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/conftest.py b/awx/main/tests/functional/conftest.py
index bb6f902802..3aab7a2537 100644
--- a/awx/main/tests/functional/conftest.py
+++ b/awx/main/tests/functional/conftest.py
@@ -187,13 +187,8 @@ def notification_template(organization):
                                                                                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
+def job_with_secret_key(job_with_secret_key_factory):
+    return job_with_secret_key_factory(persisted=True)
 
 @pytest.fixture
 def admin(user):
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/models/test_job_template_unit.py b/awx/main/tests/unit/models/test_job_template_unit.py
index 7ed24ce173..ff9f2c6e67 100644
--- a/awx/main/tests/unit/models/test_job_template_unit.py
+++ b/awx/main/tests/unit/models/test_job_template_unit.py
@@ -1,21 +1,9 @@
 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(
+def test_missing_project_error(job_template_factory):
+    objects = job_template_factory(
         'missing-project-jt',
         organization='org1',
         inventory='inventory1',
@@ -26,8 +14,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)
@@ -35,8 +23,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)
@@ -48,21 +36,14 @@ def test_inventory_credential_contradictions():
     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']
+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']
 
 @pytest.mark.survey
-def test_job_redacted_extra_vars(job_template_sensitive_data):
+def test_job_redacted_extra_vars(job_with_secret_key_unit):
     """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()) == {
+    assert json.loads(job_with_secret_key_unit.display_extra_vars()) == {
         'submitter_email': 'foobar@redhat.com',
         'secret_key': '$encrypted$',
-        'SSN': '$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
index 633f6a0470..6b6946b136 100644
--- a/awx/main/tests/unit/models/test_job_unit.py
+++ b/awx/main/tests/unit/models/test_job_unit.py
@@ -1,40 +1,23 @@
-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):
+def test_job_safe_args_redacted_passwords(job_with_secret_key_unit):
     """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)
+    safe_args = run_job.build_safe_args(job_with_secret_key_unit, **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):
+def test_job_args_unredacted_passwords(job_with_secret_key_unit):
     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])
+    args = run_job.build_args(job_with_secret_key_unit, **kwargs)
+    ev_index = args.index('-e') + 1
+    extra_vars = json.loads(args[ev_index])
     assert extra_vars['secret_key'] == '6kQngg3h8lgiSTvIEb21'
     assert extra_vars['submitter_email'] == 'foobar@redhat.com'

From 2cf0694adf0f15d2ed33f5bd9f5c57f389d3ed67 Mon Sep 17 00:00:00 2001
From: AlanCoding <arominger@ansible.com>
Date: Wed, 8 Jun 2016 14:56:00 -0400
Subject: [PATCH 3/4] move job view test to serializer unit test

---
 .../functional/api/test_unified_jobs_view.py    |  6 ------
 awx/main/tests/unit/api/test_serializers.py     | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 6 deletions(-)

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 a7c1607dd9..ab9487bc38 100644
--- a/awx/main/tests/functional/api/test_unified_jobs_view.py
+++ b/awx/main/tests/functional/api/test_unified_jobs_view.py
@@ -1,5 +1,4 @@
 import pytest
-import json
 
 from awx.main.models import UnifiedJob
 from django.core.urlresolvers import reverse
@@ -16,8 +15,3 @@ 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/unit/api/test_serializers.py b/awx/main/tests/unit/api/test_serializers.py
index 0b2f41d930..01c60adcc4 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,22 @@ 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):

From 6e92bc3668cc303d7a489ad92013633fb4760fde Mon Sep 17 00:00:00 2001
From: AlanCoding <arominger@ansible.com>
Date: Wed, 8 Jun 2016 15:30:25 -0400
Subject: [PATCH 4/4] modularize the job unit tests

---
 awx/main/tests/unit/api/test_serializers.py   |  3 +-
 .../unit/models/test_job_template_unit.py     |  9 ------
 awx/main/tests/unit/models/test_job_unit.py   | 30 ++++++++++++++-----
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/awx/main/tests/unit/api/test_serializers.py b/awx/main/tests/unit/api/test_serializers.py
index 01c60adcc4..a8fb25d005 100644
--- a/awx/main/tests/unit/api/test_serializers.py
+++ b/awx/main/tests/unit/api/test_serializers.py
@@ -147,8 +147,7 @@ class TestJobSerializerGetRelated(GetRelatedMixin):
         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
-})
+    'extra_vars': obj.extra_vars})
 class TestJobSerializerSubstitution():
 
     def test_survey_password_hide(self, mocker):
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 ff9f2c6e67..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,5 +1,4 @@
 import pytest
-import json
 
 
 def test_missing_project_error(job_template_factory):
@@ -39,11 +38,3 @@ def test_inventory_credential_contradictions(job_template_factory):
 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']
-
-@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$'}
diff --git a/awx/main/tests/unit/models/test_job_unit.py b/awx/main/tests/unit/models/test_job_unit.py
index 6b6946b136..a1791c59d5 100644
--- a/awx/main/tests/unit/models/test_job_unit.py
+++ b/awx/main/tests/unit/models/test_job_unit.py
@@ -1,23 +1,39 @@
+import pytest
 import json
 
 from awx.main.tasks import RunJob
 
 
-def test_job_safe_args_redacted_passwords(job_with_secret_key_unit):
+@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_with_secret_key_unit, **kwargs)
+    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$'
-    assert extra_vars['submitter_email'] == 'foobar@redhat.com'
 
-def test_job_args_unredacted_passwords(job_with_secret_key_unit):
+def test_job_args_unredacted_passwords(job):
     kwargs = {'ansible_version': '2.1'}
     run_job = RunJob()
-    args = run_job.build_args(job_with_secret_key_unit, **kwargs)
+    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'] == '6kQngg3h8lgiSTvIEb21'
-    assert extra_vars['submitter_email'] == 'foobar@redhat.com'
+    assert extra_vars['secret_key'] == 'my_password'