Merge pull request #606 from ryanpetrello/test-7806

improve validation for empty default passwords

awx/main/models/mixins.py

@@ -143,14 +143,20 @@ class SurveyJobTemplateMixin(models.Model):
                 variable_key = survey_element.get('variable')
 
                 if survey_element.get('type') == 'password':
-                    if variable_key in runtime_extra_vars and default:
+                    if variable_key in runtime_extra_vars:
                         kw_value = runtime_extra_vars[variable_key]
-                        if kw_value == '$encrypted$' and kw_value != default:
+                        if kw_value == '$encrypted$':
-                            runtime_extra_vars[variable_key] = default
+                            runtime_extra_vars.pop(variable_key)
 
                 if default is not None:
-                    data = {variable_key: default}
+                    decrypted_default = default
-                    errors = self._survey_element_validation(survey_element, data)
+                    if (
+                        survey_element['type'] == "password" and
+                        isinstance(decrypted_default, basestring) and
+                        decrypted_default.startswith('$encrypted$')
+                    ):
+                        decrypted_default = decrypt_value(get_encryption_key('value', pk=None), decrypted_default)
+                    errors = self._survey_element_validation(survey_element, {variable_key: decrypted_default})
                     if not errors:
                         survey_defaults[variable_key] = default
         extra_vars.update(survey_defaults)
@@ -162,24 +168,20 @@ class SurveyJobTemplateMixin(models.Model):
         return create_kwargs
 
     def _survey_element_validation(self, survey_element, data):
+        # Don't apply validation to the `$encrypted$` placeholder; the decrypted
+        # default (if any) will be validated against instead
         errors = []
-        # make a copy of the data to break references (so that we don't
+
-        # inadvertently expose unencrypted default passwords as we validate)
+        if (survey_element['type'] == "password"):
-        data = data.copy()
+            password_value = data.get(survey_element['variable'])
-        password_value = data.get(survey_element['variable'])
+            if (
-        if (
+                isinstance(password_value, basestring) and
-            survey_element['type'] == "password" and
+                password_value == '$encrypted$'
-            isinstance(password_value, basestring) and
+            ):
-            password_value.startswith('$encrypted$')
+                if survey_element.get('default') is None and survey_element['required']:
-        ):
+                    errors.append("'%s' value missing" % survey_element['variable'])
-            if password_value == '$encrypted$':
+                return errors
-                # replace encrypted password defaults so we don't validate on
+
-                # $encrypted$
-                password_value = survey_element['default']
-            data[survey_element['variable']] = decrypt_value(
-                get_encryption_key('value', pk=None),
-                password_value
-            )
         if survey_element['variable'] not in data and survey_element['required']:
             errors.append("'%s' value missing" % survey_element['variable'])
         elif survey_element['type'] in ["textarea", "text", "password"]:

awx/main/tests/functional/api/test_survey_spec.py

@@ -166,6 +166,101 @@ def test_survey_spec_passwords_with_empty_default(job_template_factory, post, ad
     }
 
 
+@mock.patch('awx.api.views.feature_enabled', lambda feature: True)
+@pytest.mark.django_db
+@pytest.mark.parametrize('default, launch_value, expected_extra_vars, status', [
+    ['', '$encrypted$', {'secret_value': ''}, 201],
+    ['', 'y', {'secret_value': 'y'}, 201],
+    ['', 'y' * 100, None, 400],
+    [None, '$encrypted$', {}, 201],
+    [None, 'y', {'secret_value': 'y'}, 201],
+    [None, 'y' * 100, {}, 400],
+    ['x', '$encrypted$', {'secret_value': 'x'}, 201],
+    ['x', 'y', {'secret_value': 'y'}, 201],
+    ['x', 'y' * 100, {}, 400],
+    ['x' * 100, '$encrypted$', {}, 201],
+    ['x' * 100, 'y', {'secret_value': 'y'}, 201],
+    ['x' * 100, 'y' * 100, {}, 400],
+])
+def test_survey_spec_passwords_with_default_optional(job_template_factory, post, admin_user,
+                                                     default, launch_value,
+                                                     expected_extra_vars, status):
+    objects = job_template_factory('jt', organization='org1', project='prj',
+                                   inventory='inv', credential='cred')
+    job_template = objects.job_template
+    job_template.survey_enabled = True
+    job_template.save()
+    input_data = {
+        'description': 'A survey',
+        'spec': [{
+            'index': 0,
+            'question_name': 'What is your password?',
+            'required': False,
+            'variable': 'secret_value',
+            'type': 'password',
+            'max': 3
+        }],
+        'name': 'my survey'
+    }
+    if default is not None:
+        input_data['spec'][0]['default'] = default
+    post(url=reverse('api:job_template_survey_spec', kwargs={'pk': job_template.id}),
+         data=input_data, user=admin_user, expect=200)
+
+    resp = post(reverse('api:job_template_launch', kwargs={'pk': job_template.pk}),
+                data={'extra_vars': {'secret_value': launch_value}}, user=admin_user, expect=status)
+
+    if status == 201:
+        job = Job.objects.get(pk=resp.data['job'])
+        assert json.loads(job.decrypted_extra_vars()) == expected_extra_vars
+        if default:
+            assert default not in json.loads(job.extra_vars).values()
+        assert launch_value not in json.loads(job.extra_vars).values()
+
+
+@mock.patch('awx.api.views.feature_enabled', lambda feature: True)
+@pytest.mark.django_db
+@pytest.mark.parametrize('default, launch_value, expected_extra_vars, status', [
+    ['', '$encrypted$', {'secret_value': ''}, 201],
+    [None, '$encrypted$', {}, 400],
+    [None, 'y', {'secret_value': 'y'}, 201],
+])
+def test_survey_spec_passwords_with_default_required(job_template_factory, post, admin_user,
+                                                     default, launch_value,
+                                                     expected_extra_vars, status):
+    objects = job_template_factory('jt', organization='org1', project='prj',
+                                   inventory='inv', credential='cred')
+    job_template = objects.job_template
+    job_template.survey_enabled = True
+    job_template.save()
+    input_data = {
+        'description': 'A survey',
+        'spec': [{
+            'index': 0,
+            'question_name': 'What is your password?',
+            'required': True,
+            'variable': 'secret_value',
+            'type': 'password',
+            'max': 3
+        }],
+        'name': 'my survey'
+    }
+    if default is not None:
+        input_data['spec'][0]['default'] = default
+    post(url=reverse('api:job_template_survey_spec', kwargs={'pk': job_template.id}),
+         data=input_data, user=admin_user, expect=200)
+
+    resp = post(reverse('api:job_template_launch', kwargs={'pk': job_template.pk}),
+                data={'extra_vars': {'secret_value': launch_value}}, user=admin_user, expect=status)
+
+    if status == 201:
+        job = Job.objects.get(pk=resp.data['job'])
+        assert json.loads(job.decrypted_extra_vars()) == expected_extra_vars
+        if default:
+            assert default not in json.loads(job.extra_vars).values()
+        assert launch_value not in json.loads(job.extra_vars).values()
+
+
 @mock.patch('awx.api.views.feature_enabled', lambda feature: True)
 @pytest.mark.django_db
 @pytest.mark.parametrize('default, status', [

awx/main/tests/unit/models/test_survey_models.py

@@ -8,7 +8,6 @@ from awx.main.models import (
     JobTemplate,
     WorkflowJobTemplate
 )
-from awx.main.utils.encryption import encrypt_value
 
 
 @pytest.fixture
@@ -144,21 +143,6 @@ def test_optional_survey_question_defaults(
         assert 'c' not in defaulted_extra_vars['extra_vars']
 
 
-@pytest.mark.survey
-def test_encrypted_default_validation(survey_spec_factory):
-    element = {
-        "required": True,
-        "default": encrypt_value("test1234", pk=None),
-        "variable": "x",
-        "min": 0,
-        "max": 8,
-        "type": "password",
-    }
-    spec = survey_spec_factory([element])
-    jt = JobTemplate(name="test-jt", survey_spec=spec, survey_enabled=True)
-    assert not len(jt.survey_variable_validation({'x': '$encrypted$'}))
-
-
 @pytest.mark.survey
 class TestWorkflowSurveys:
     def test_update_kwargs_survey_defaults(self, survey_spec_factory):