Test-based fixes to launch config password handling

Fix bug creating WFJT schedule with passwords:
discard survey_passwords field if given in WFJT
prompts processing method.
Fixed by porting prior JT fix to WFJT method of
same name.

Fix bug where API browser will show encrypted form
of variables in the POST submission box after
failed attempt:
copy extra_data so encrypted data values are not
added in still-linked request.data

Fix a bug where submitted extra_data $encrypted$ string
literal was saved because survey_passwords was empty
when there was no diff from prior.

Allow not answering required password questions with a
non-empty default value when saving a launch config.
The literal $encrypted$ string now gets passed into the
prompts / survey validator.

awx/api/serializers.py

@@ -3632,6 +3632,10 @@ class LaunchConfigurationBaseSerializer(BaseSerializer):
         return summary_fields
 
     def validate(self, attrs):
+        db_extra_data = {}
+        if self.instance:
+            db_extra_data = parse_yaml_or_json(self.instance.extra_data)
+
         attrs = super(LaunchConfigurationBaseSerializer, self).validate(attrs)
 
         ujt = None
@@ -3640,39 +3644,38 @@ class LaunchConfigurationBaseSerializer(BaseSerializer):
         elif self.instance:
             ujt = self.instance.unified_job_template
 
-        # Replace $encrypted$ submissions with db value if exists
         # build additional field survey_passwords to track redacted variables
-        if 'extra_data' in attrs:
+        password_dict = {}
         extra_data = parse_yaml_or_json(attrs.get('extra_data', {}))
         if hasattr(ujt, 'survey_password_variables'):
             # Prepare additional field survey_passwords for save
-                password_dict = {}
             for key in ujt.survey_password_variables():
                 if key in extra_data:
                     password_dict[key] = REPLACE_STR
+
+        # Replace $encrypted$ submissions with db value if exists
+        if 'extra_data' in attrs:
+            if password_dict:
                 if not self.instance or password_dict != self.instance.survey_passwords:
                     attrs['survey_passwords'] = password_dict.copy()
                 # Force dict type (cannot preserve YAML formatting if passwords are involved)
-                if not isinstance(attrs['extra_data'], dict):
-                    attrs['extra_data'] = parse_yaml_or_json(attrs['extra_data'])
                 # Encrypt the extra_data for save, only current password vars in JT survey
+                # but first, make a copy or else this is referenced by request.data, and
+                # user could get encrypted string in form data in API browser
+                attrs['extra_data'] = extra_data.copy()
                 encrypt_dict(attrs['extra_data'], password_dict.keys())
                 # For any raw $encrypted$ string, either
                 # - replace with existing DB value
                 # - raise a validation error
-                # - remove key from extra_data if survey default is present
-                if self.instance:
-                    db_extra_data = parse_yaml_or_json(self.instance.extra_data)
-                else:
-                    db_extra_data = {}
+                # - ignore, if default present
                 for key in password_dict.keys():
                     if attrs['extra_data'].get(key, None) == REPLACE_STR:
                         if key not in db_extra_data:
                             element = ujt.pivot_spec(ujt.survey_spec)[key]
-                            if 'default' in element and element['default']:
-                                attrs['survey_passwords'].pop(key, None)
-                                attrs['extra_data'].pop(key, None)
-                            else:
+                            # NOTE: validation _of_ the default values of password type
+                            # questions not done here or on launch, but doing so could
+                            # leak info about values, so it should not be added
+                            if not ('default' in element and element['default']):
                                 raise serializers.ValidationError(
                                     {"extra_data": _('Provided variable {} has no database value to replace with.').format(key)})
                         else:
@@ -3683,6 +3686,18 @@ class LaunchConfigurationBaseSerializer(BaseSerializer):
         accepted, rejected, errors = ujt._accept_or_ignore_job_kwargs(
             _exclude_errors=self.exclude_errors, **mock_obj.prompts_dict())
 
+        # Remove all unprocessed $encrypted$ strings, indicating default usage
+        if 'extra_data' in attrs and password_dict:
+            for key, value in attrs['extra_data'].copy().items():
+                if value == REPLACE_STR:
+                    if key in password_dict:
+                        attrs['extra_data'].pop(key)
+                        attrs.get('survey_passwords', {}).pop(key, None)
+                    else:
+                        errors.setdefault('extra_vars', []).append(
+                            _('"$encrypted$ is a reserved keyword, may not be used for {var_name}."'.format(key))
+                        )
+
         # Launch configs call extra_vars extra_data for historical reasons
         if 'extra_vars' in errors:
             errors['extra_data'] = errors.pop('extra_vars')

awx/main/models/workflow.py

@@ -371,23 +371,28 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl
         return workflow_job
 
     def _accept_or_ignore_job_kwargs(self, _exclude_errors=(), **kwargs):
-        prompted_fields = {}
-        rejected_fields = {}
-        accepted_vars, rejected_vars, errors_dict = self.accept_or_ignore_variables(kwargs.get('extra_vars', {}))
+        exclude_errors = kwargs.pop('_exclude_errors', [])
+        prompted_data = {}
+        rejected_data = {}
+        accepted_vars, rejected_vars, errors_dict = self.accept_or_ignore_variables(
+            kwargs.get('extra_vars', {}),
+            _exclude_errors=exclude_errors,
+            extra_passwords=kwargs.get('survey_passwords', {}))
         if accepted_vars:
-            prompted_fields['extra_vars'] = accepted_vars
+            prompted_data['extra_vars'] = accepted_vars
         if rejected_vars:
-            rejected_fields['extra_vars'] = rejected_vars
+            rejected_data['extra_vars'] = rejected_vars
 
         # WFJTs do not behave like JTs, it can not accept inventory, credential, etc.
         bad_kwargs = kwargs.copy()
         bad_kwargs.pop('extra_vars', None)
+        bad_kwargs.pop('survey_passwords', None)
         if bad_kwargs:
-            rejected_fields.update(bad_kwargs)
+            rejected_data.update(bad_kwargs)
             for field in bad_kwargs:
                 errors_dict[field] = _('Field is not allowed for use in workflows.')
 
-        return prompted_fields, rejected_fields, errors_dict
+        return prompted_data, rejected_data, errors_dict
 
     def can_start_without_user_input(self):
         return not bool(self.variables_needed_to_start)

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

@@ -2,7 +2,8 @@ import pytest
 
 from awx.api.versioning import reverse
 
-from awx.main.models import JobTemplate
+from awx.main.models import JobTemplate, Schedule
+from awx.main.utils.encryption import decrypt_value, get_encryption_key
 
 
 RRULE_EXAMPLE = 'DTSTART:20151117T050000Z RRULE:FREQ=DAILY;INTERVAL=1;COUNT=1'
@@ -51,6 +52,50 @@ def test_valid_survey_answer(post, admin_user, project, inventory, survey_spec_f
          admin_user, expect=201)
 
 
+@pytest.mark.django_db
+def test_encrypted_survey_answer(post, patch, admin_user, project, inventory, survey_spec_factory):
+    job_template = JobTemplate.objects.create(
+        name='test-jt',
+        project=project,
+        playbook='helloworld.yml',
+        inventory=inventory,
+        ask_variables_on_launch=False,
+        survey_enabled=True,
+        survey_spec=survey_spec_factory([{'variable': 'var1', 'type': 'password'}])
+    )
+
+    # test encrypted-on-create
+    url = reverse('api:job_template_schedules_list', kwargs={'pk': job_template.id})
+    r = post(url, {'name': 'test sch', 'rrule': RRULE_EXAMPLE, 'extra_data': '{"var1": "foo"}'},
+             admin_user, expect=201)
+    assert r.data['extra_data']['var1'] == "$encrypted$"
+    schedule = Schedule.objects.get(pk=r.data['id'])
+    assert schedule.extra_data['var1'].startswith('$encrypted$')
+    assert decrypt_value(get_encryption_key('value', pk=None), schedule.extra_data['var1']) == 'foo'
+
+    # test a no-op change
+    r = patch(
+        schedule.get_absolute_url(),
+        data={'extra_data': {'var1': '$encrypted$'}},
+        user=admin_user,
+        expect=200
+    )
+    assert r.data['extra_data']['var1'] == '$encrypted$'
+    schedule.refresh_from_db()
+    assert decrypt_value(get_encryption_key('value', pk=None), schedule.extra_data['var1']) == 'foo'
+
+    # change to a different value
+    r = patch(
+        schedule.get_absolute_url(),
+        data={'extra_data': {'var1': 'bar'}},
+        user=admin_user,
+        expect=200
+    )
+    assert r.data['extra_data']['var1'] == '$encrypted$'
+    schedule.refresh_from_db()
+    assert decrypt_value(get_encryption_key('value', pk=None), schedule.extra_data['var1']) == 'bar'
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize('rrule, error', [
     ("", "This field may not be blank"),