diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 4e0d303efe..e5da287c1e 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3679,6 +3679,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 @@ -3687,39 +3691,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 + 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 + 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: - 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 + 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: @@ -3730,6 +3733,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') diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index f553fd7254..198595424f 100644 --- a/awx/main/models/workflow.py +++ b/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) diff --git a/awx/main/tests/functional/api/test_schedules.py b/awx/main/tests/functional/api/test_schedules.py index c03ecead1a..21e3b91065 100644 --- a/awx/main/tests/functional/api/test_schedules.py +++ b/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"),