From 74bf058d620797e2bd93185a760502a9a01800cf Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 14 Dec 2017 16:26:17 -0500 Subject: [PATCH 1/4] encrypt password answers on config save --- awx/api/serializers.py | 17 +++++++++++++-- awx/main/models/unified_jobs.py | 8 ++----- .../serializers/test_workflow_serializers.py | 21 +++++++++++++++++++ awx/main/utils/encryption.py | 16 ++++++++++++-- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 4588a5ae80..ad05837d08 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -44,7 +44,7 @@ from awx.main.fields import ImplicitRoleField from awx.main.utils import ( get_type_for_model, get_model_for_type, timestamp_apiformat, camelcase_to_underscore, getattrd, parse_yaml_or_json, - has_model_field_prefetched, extract_ansible_vars) + has_model_field_prefetched, extract_ansible_vars, encrypt_dict) from awx.main.utils.filters import SmartFilter from awx.main.redact import REPLACE_STR @@ -3140,7 +3140,6 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): attrs['char_prompts'] = mock_obj.char_prompts # Insert survey_passwords to track redacted variables - # TODO: perform encryption on save if 'extra_data' in attrs: extra_data = parse_yaml_or_json(attrs.get('extra_data', {})) if hasattr(ujt, 'survey_password_variables'): @@ -3150,6 +3149,20 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): password_dict[key] = REPLACE_STR if not self.instance or password_dict != self.instance.survey_passwords: attrs['survey_passwords'] = password_dict + if not isinstance(attrs['extra_data'], dict): + attrs['extra_data'] = parse_yaml_or_json(attrs['extra_data']) + encrypt_dict(attrs['extra_data'], password_dict.keys()) + if self.instance: + db_extra_data = parse_yaml_or_json(self.instance.extra_data) + else: + db_extra_data = {} + for key in password_dict.keys(): + if attrs['extra_data'].get(key, None) == REPLACE_STR: + if key not in db_extra_data: + raise serializers.ValidationError( + _('Provided variable {} has no database value to replace with.').format(key)) + else: + attrs['extra_data'][key] = db_extra_data[key] return attrs diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 1d2a4eb221..f1c934b9a3 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -34,7 +34,7 @@ from django_celery_results.models import TaskResult from awx.main.models.base import * # noqa from awx.main.models.mixins import ResourceMixin, TaskManagerUnifiedJobMixin from awx.main.utils import ( - encrypt_value, decrypt_field, _inventory_updates, + encrypt_dict, decrypt_field, _inventory_updates, copy_model_by_class, copy_m2m_relationships, get_type_for_model, parse_yaml_or_json ) @@ -349,11 +349,7 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio # automatically encrypt survey fields if hasattr(self, 'survey_spec') and getattr(self, 'survey_enabled', False): password_list = self.survey_password_variables() - for key in kwargs.get('extra_vars', {}): - if key in password_list: - kwargs['extra_vars'][key] = encrypt_value( - kwargs['extra_vars'][key] - ) + encrypt_dict(kwargs.get('extra_vars', {}), password_list) unified_job_class = self._get_unified_job_class() fields = self._get_unified_job_field_names() diff --git a/awx/main/tests/unit/api/serializers/test_workflow_serializers.py b/awx/main/tests/unit/api/serializers/test_workflow_serializers.py index 7e19f66420..712345a1b6 100644 --- a/awx/main/tests/unit/api/serializers/test_workflow_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_workflow_serializers.py @@ -177,6 +177,8 @@ class TestWorkflowJobTemplateNodeSerializerSurveyPasswords(): }) assert 'survey_passwords' in attrs assert 'var1' in attrs['survey_passwords'] + assert attrs['extra_data']['var1'].startswith('$encrypted$') + assert len(attrs['extra_data']['var1']) > len('$encrypted$') def test_set_survey_passwords_modify(self, jt): serializer = WorkflowJobTemplateNodeSerializer() @@ -192,6 +194,25 @@ class TestWorkflowJobTemplateNodeSerializerSurveyPasswords(): }) assert 'survey_passwords' in attrs assert 'var1' in attrs['survey_passwords'] + assert attrs['extra_data']['var1'].startswith('$encrypted$') + assert len(attrs['extra_data']['var1']) > len('$encrypted$') + + def test_use_db_answer(self, jt): + serializer = WorkflowJobTemplateNodeSerializer() + wfjt = WorkflowJobTemplate(name='fake-wfjt') + serializer.instance = WorkflowJobTemplateNode( + workflow_job_template=wfjt, + unified_job_template=jt, + extra_data={'var1': '$encrypted$foooooo'} + ) + attrs = serializer.validate({ + 'unified_job_template': jt, + 'workflow_job_template': wfjt, + 'extra_data': {'var1': '$encrypted$'} + }) + assert 'survey_passwords' in attrs + assert 'var1' in attrs['survey_passwords'] + assert attrs['extra_data']['var1'] == '$encrypted$foooooo' @mock.patch('awx.api.serializers.WorkflowJobTemplateNodeSerializer.get_related', lambda x,y: {}) diff --git a/awx/main/utils/encryption.py b/awx/main/utils/encryption.py index c8c5b72afd..29aed4721e 100644 --- a/awx/main/utils/encryption.py +++ b/awx/main/utils/encryption.py @@ -9,8 +9,10 @@ from cryptography.hazmat.backends import default_backend from django.utils.encoding import smart_str -__all__ = ['get_encryption_key', 'encrypt_value', 'encrypt_field', - 'decrypt_field', 'decrypt_value'] +__all__ = ['get_encryption_key', + 'encrypt_field', 'decrypt_field', + 'encrypt_value', 'decrypt_value', + 'encrypt_dict'] logger = logging.getLogger('awx.main.utils.encryption') @@ -125,3 +127,13 @@ def decrypt_field(instance, field_name, subfield=None): exc_info=True ) raise + + +def encrypt_dict(data, fields): + ''' + Encrypts all of the dictionary values in `data` under the keys in `fields` + in-place operation on `data` + ''' + encrypt_fields = set(data.keys()).intersection(fields) + for key in encrypt_fields: + data[key] = encrypt_value(data[key]) From 1e1839915d469c7f70aa3f68beec0a019576f271 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 15 Dec 2017 10:47:23 -0500 Subject: [PATCH 2/4] validate against unencrypted values at spawn point --- awx/api/serializers.py | 3 ++- awx/main/models/jobs.py | 3 ++- awx/main/models/mixins.py | 9 +++++++-- awx/main/models/unified_jobs.py | 8 ++++++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index ad05837d08..30ea61de50 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3137,7 +3137,8 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): raise serializers.ValidationError(errors) # Model `.save` needs the container dict, not the psuedo fields - attrs['char_prompts'] = mock_obj.char_prompts + if mock_obj.char_prompts: + attrs['char_prompts'] = mock_obj.char_prompts # Insert survey_passwords to track redacted variables if 'extra_data' in attrs: diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index beaceab0a8..4ddeda81a5 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -355,7 +355,8 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour rejected_data = {} accepted_vars, rejected_vars, errors_dict = self.accept_or_ignore_variables( kwargs.get('extra_vars', {}), - _exclude_errors=exclude_errors) + _exclude_errors=exclude_errors, + extra_passwords=kwargs.get('survey_passwords', {})) if accepted_vars: prompted_data['extra_vars'] = accepted_vars if rejected_vars: diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 488ea3d609..5e30b29e05 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -253,7 +253,7 @@ class SurveyJobTemplateMixin(models.Model): choice_list)) return errors - def _accept_or_ignore_variables(self, data, errors=None, _exclude_errors=()): + def _accept_or_ignore_variables(self, data, errors=None, _exclude_errors=(), extra_passwords=None): survey_is_enabled = (self.survey_enabled and self.survey_spec) extra_vars = data.copy() if errors is None: @@ -265,8 +265,13 @@ class SurveyJobTemplateMixin(models.Model): # Check for data violation of survey rules survey_errors = [] for survey_element in self.survey_spec.get("spec", []): - element_errors = self._survey_element_validation(survey_element, data) key = survey_element.get('variable', None) + if extra_passwords and key in extra_passwords and data.get(key, None): + element_errors = self._survey_element_validation(survey_element, { + key: decrypt_value(get_encryption_key('value', pk=None), data[key]) + }) + else: + element_errors = self._survey_element_validation(survey_element, data) if element_errors: survey_errors += element_errors diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index f1c934b9a3..75e5e8dd05 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -441,7 +441,7 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio errors[field_name] = [_("Field is not allowed on launch.")] return ({}, kwargs, errors) - def accept_or_ignore_variables(self, data, errors=None, _exclude_errors=()): + def accept_or_ignore_variables(self, data, errors=None, _exclude_errors=(), extra_passwords=None): ''' If subclasses accept any `variables` or `extra_vars`, they should define _accept_or_ignore_variables to place those variables in the accepted dict, @@ -459,7 +459,11 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio # SurveyJobTemplateMixin cannot override any methods because of # resolution order, forced by how metaclass processes fields, # thus the need for hasattr check - return self._accept_or_ignore_variables(data, errors, _exclude_errors=_exclude_errors) + if extra_passwords: + return self._accept_or_ignore_variables( + data, errors, _exclude_errors=_exclude_errors, extra_passwords=extra_passwords) + else: + return self._accept_or_ignore_variables(data, errors, _exclude_errors=_exclude_errors) elif data: errors['extra_vars'] = [ _('Variables {list_of_keys} provided, but this template cannot accept variables.'.format( From c8e10adc96e1219117bfb877ab1ddd902ceb3666 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 18 Dec 2017 09:40:04 -0500 Subject: [PATCH 3/4] fix bug saving extra_data and follow prompts rules display_extra_vars was not taking a copy of the data before acting on it - this causes a bug where the activity stream will modify the existing object on the model. That leads to new data not being accepted. Also moved the processing of extra_data to prior to the accept or ignore kwargs logic so that we pass the right (post-encryption) form of the variables. --- awx/api/serializers.py | 30 ++++++++++--------- awx/main/models/jobs.py | 2 +- awx/main/models/mixins.py | 7 +++-- .../serializers/test_workflow_serializers.py | 13 ++++---- awx/main/utils/encryption.py | 6 ++++ 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 30ea61de50..709ad1d769 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3120,25 +3120,11 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): def validate(self, attrs): attrs = super(LaunchConfigurationBaseSerializer, self).validate(attrs) - # Build unsaved version of this config, use it to detect prompts errors ujt = None if 'unified_job_template' in attrs: ujt = attrs['unified_job_template'] elif self.instance: ujt = self.instance.unified_job_template - mock_obj = self._build_mock_obj(attrs) - accepted, rejected, errors = ujt._accept_or_ignore_job_kwargs( - _exclude_errors=self.exclude_errors, **mock_obj.prompts_dict()) - - # Launch configs call extra_vars extra_data for historical reasons - if 'extra_vars' in errors: - errors['extra_data'] = errors.pop('extra_vars') - if errors: - raise serializers.ValidationError(errors) - - # Model `.save` needs the container dict, not the psuedo fields - if mock_obj.char_prompts: - attrs['char_prompts'] = mock_obj.char_prompts # Insert survey_passwords to track redacted variables if 'extra_data' in attrs: @@ -3164,6 +3150,22 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): _('Provided variable {} has no database value to replace with.').format(key)) else: attrs['extra_data'][key] = db_extra_data[key] + + # Build unsaved version of this config, use it to detect prompts errors + mock_obj = self._build_mock_obj(attrs) + accepted, rejected, errors = ujt._accept_or_ignore_job_kwargs( + _exclude_errors=self.exclude_errors, **mock_obj.prompts_dict()) + + # Launch configs call extra_vars extra_data for historical reasons + if 'extra_vars' in errors: + errors['extra_data'] = errors.pop('extra_vars') + if errors: + raise serializers.ValidationError(errors) + + # Model `.save` needs the container dict, not the psuedo fields + if mock_obj.char_prompts: + attrs['char_prompts'] = mock_obj.char_prompts + return attrs diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 4ddeda81a5..2bb2f53ca2 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -893,7 +893,7 @@ class LaunchTimeConfig(BaseModel): Hides fields marked as passwords in survey. ''' if self.survey_passwords: - extra_data = parse_yaml_or_json(self.extra_data) + extra_data = parse_yaml_or_json(self.extra_data).copy() for key, value in self.survey_passwords.items(): if key in extra_data: extra_data[key] = value diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 5e30b29e05..9bf4d9292f 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -14,7 +14,7 @@ from awx.main.models.rbac import ( Role, RoleAncestorEntry, get_roles_on_resource ) from awx.main.utils import parse_yaml_or_json -from awx.main.utils.encryption import decrypt_value, get_encryption_key +from awx.main.utils.encryption import decrypt_value, get_encryption_key, is_encrypted from awx.main.fields import JSONField, AskForField @@ -266,9 +266,10 @@ class SurveyJobTemplateMixin(models.Model): survey_errors = [] for survey_element in self.survey_spec.get("spec", []): key = survey_element.get('variable', None) - if extra_passwords and key in extra_passwords and data.get(key, None): + value = data.get(key, None) + if extra_passwords and key in extra_passwords and is_encrypted(value): element_errors = self._survey_element_validation(survey_element, { - key: decrypt_value(get_encryption_key('value', pk=None), data[key]) + key: decrypt_value(get_encryption_key('value', pk=None), value) }) else: element_errors = self._survey_element_validation(survey_element, data) diff --git a/awx/main/tests/unit/api/serializers/test_workflow_serializers.py b/awx/main/tests/unit/api/serializers/test_workflow_serializers.py index 712345a1b6..49fb2da4e7 100644 --- a/awx/main/tests/unit/api/serializers/test_workflow_serializers.py +++ b/awx/main/tests/unit/api/serializers/test_workflow_serializers.py @@ -197,7 +197,7 @@ class TestWorkflowJobTemplateNodeSerializerSurveyPasswords(): assert attrs['extra_data']['var1'].startswith('$encrypted$') assert len(attrs['extra_data']['var1']) > len('$encrypted$') - def test_use_db_answer(self, jt): + def test_use_db_answer(self, jt, mocker): serializer = WorkflowJobTemplateNodeSerializer() wfjt = WorkflowJobTemplate(name='fake-wfjt') serializer.instance = WorkflowJobTemplateNode( @@ -205,11 +205,12 @@ class TestWorkflowJobTemplateNodeSerializerSurveyPasswords(): unified_job_template=jt, extra_data={'var1': '$encrypted$foooooo'} ) - attrs = serializer.validate({ - 'unified_job_template': jt, - 'workflow_job_template': wfjt, - 'extra_data': {'var1': '$encrypted$'} - }) + with mocker.patch('awx.main.models.mixins.decrypt_value', return_value='foo'): + attrs = serializer.validate({ + 'unified_job_template': jt, + 'workflow_job_template': wfjt, + 'extra_data': {'var1': '$encrypted$'} + }) assert 'survey_passwords' in attrs assert 'var1' in attrs['survey_passwords'] assert attrs['extra_data']['var1'] == '$encrypted$foooooo' diff --git a/awx/main/utils/encryption.py b/awx/main/utils/encryption.py index 29aed4721e..b8c0cc45a0 100644 --- a/awx/main/utils/encryption.py +++ b/awx/main/utils/encryption.py @@ -137,3 +137,9 @@ def encrypt_dict(data, fields): encrypt_fields = set(data.keys()).intersection(fields) for key in encrypt_fields: data[key] = encrypt_value(data[key]) + + +def is_encrypted(value): + if not isinstance(value, six.string_types): + return False + return value.startswith('$encrypted$') and len(value) > len('$encrypted$') From 3439ba5f3b0857a6497ea5ae154faa02301ae065 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 18 Dec 2017 11:34:32 -0500 Subject: [PATCH 4/4] allow WFJT nodes without required variables --- awx/api/serializers.py | 4 ++-- awx/main/models/mixins.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 709ad1d769..3923e5c9ef 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3176,7 +3176,7 @@ class WorkflowJobTemplateNodeSerializer(LaunchConfigurationBaseSerializer): success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) - exclude_errors = ('required') # required variables may be provided by WFJT or on launch + exclude_errors = ('required',) # required variables may be provided by WFJT or on launch class Meta: model = WorkflowJobTemplateNode @@ -3574,7 +3574,7 @@ class JobLaunchSerializer(BaseSerializer): template = self.context.get('template') accepted, rejected, errors = template._accept_or_ignore_job_kwargs( - _exclude_errors=['prompts', 'required'], # make several error types non-blocking + _exclude_errors=['prompts'], # make several error types non-blocking **attrs) self._ignored_fields = rejected diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 9bf4d9292f..2f3b20f8f6 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -173,7 +173,7 @@ class SurveyJobTemplateMixin(models.Model): create_kwargs['extra_vars'] = json.dumps(extra_vars) return create_kwargs - def _survey_element_validation(self, survey_element, data): + def _survey_element_validation(self, survey_element, data, validate_required=True): # Don't apply validation to the `$encrypted$` placeholder; the decrypted # default (if any) will be validated against instead errors = [] @@ -185,11 +185,13 @@ class SurveyJobTemplateMixin(models.Model): password_value == '$encrypted$' ): if survey_element.get('default') is None and survey_element['required']: - errors.append("'%s' value missing" % survey_element['variable']) + if validate_required: + errors.append("'%s' value missing" % survey_element['variable']) return errors if survey_element['variable'] not in data and survey_element['required']: - errors.append("'%s' value missing" % survey_element['variable']) + if validate_required: + errors.append("'%s' value missing" % survey_element['variable']) elif survey_element['type'] in ["textarea", "text", "password"]: if survey_element['variable'] in data: if type(data[survey_element['variable']]) not in (str, unicode): @@ -267,12 +269,14 @@ class SurveyJobTemplateMixin(models.Model): for survey_element in self.survey_spec.get("spec", []): key = survey_element.get('variable', None) value = data.get(key, None) + validate_required = 'required' not in _exclude_errors if extra_passwords and key in extra_passwords and is_encrypted(value): element_errors = self._survey_element_validation(survey_element, { key: decrypt_value(get_encryption_key('value', pk=None), value) - }) + }, validate_required=validate_required) else: - element_errors = self._survey_element_validation(survey_element, data) + element_errors = self._survey_element_validation( + survey_element, data, validate_required=validate_required) if element_errors: survey_errors += element_errors