From 5e15f9e04e043d82f38c9c11cdb3031629b80c0f Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Fri, 28 Jul 2017 15:05:27 -0400 Subject: [PATCH] add validation errors for certain dependent credential fields see: https://github.com/ansible/ansible-tower/issues/7323 see: https://github.com/ansible/ansible-tower/issues/7293 see: https://github.com/ansible/ansible-tower/issues/7289 see: https://github.com/ansible/ansible-tower/issues/7292 --- awx/main/fields.py | 43 ++++++++++++++++--- awx/main/models/credential.py | 16 +++++-- .../tests/functional/api/test_credential.py | 30 ++++++++++++- .../functional/api/test_credential_type.py | 33 +++++++++----- 4 files changed, 103 insertions(+), 19 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 2ba41b7841..0e956f7a96 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -4,6 +4,7 @@ # Python import copy import json +import re import six from jinja2 import Environment, StrictUndefined @@ -467,6 +468,7 @@ class CredentialInputField(JSONSchemaField): return { 'type': 'object', 'properties': properties, + 'dependencies': model_instance.credential_type.inputs.get('dependencies', {}), 'additionalProperties': False, } @@ -504,6 +506,28 @@ class CredentialInputField(JSONSchemaField): ).iter_errors(decrypted_values): if error.validator == 'pattern' and 'error' in error.schema: error.message = error.schema['error'] % error.instance + if error.validator == 'dependencies': + # replace the default error messaging w/ a better i18n string + # I wish there was a better way to determine the parameters of + # this validation failure, but the exception jsonschema raises + # doesn't include them as attributes (just a hard-coded error + # string) + match = re.search( + # 'foo' is a dependency of 'bar' + "'" # apostrophe + "([^']+)" # one or more non-apostrophes (first group) + "'[\w ]+'" # one or more words/spaces + "([^']+)", # second group + error.message, + ) + if match: + label, extraneous = match.groups() + if error.schema['properties'].get(label): + label = error.schema['properties'][label]['label'] + errors[extraneous] = [ + _('cannot be set unless "%s" is set') % label + ] + continue if 'id' not in error.schema: # If the error is not for a specific field, it's specific to # `inputs` in general @@ -542,11 +566,12 @@ class CredentialInputField(JSONSchemaField): if model_instance.has_encrypted_ssh_key_data and not value.get('ssh_key_unlock'): errors['ssh_key_unlock'] = [_('must be set when SSH key is encrypted.')] - if value.get('ssh_key_unlock'): - if not model_instance.ssh_key_data: - errors['ssh_key_unlock'] = [_('should not be set when SSH key is empty.')] - elif not model_instance.has_encrypted_ssh_key_data: - errors['ssh_key_unlock'] = [_('should not be set when SSH key is not encrypted.')] + if all([ + model_instance.ssh_key_data, + value.get('ssh_key_unlock'), + not model_instance.has_encrypted_ssh_key_data + ]): + errors['ssh_key_unlock'] = [_('should not be set when SSH key is not encrypted.')] if errors: raise serializers.ValidationError({ @@ -601,6 +626,14 @@ class CredentialTypeInputField(JSONSchemaField): } def validate(self, value, model_instance): + if isinstance(value, dict) and 'dependencies' in value and \ + not model_instance.managed_by_tower: + raise django_exceptions.ValidationError( + _("'dependencies' is not supported for custom credentials."), + code='invalid', + params={'value': value}, + ) + super(CredentialTypeInputField, self).validate( value, model_instance ) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 2c337bd5f0..720e3c2fa3 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -632,7 +632,10 @@ def ssh(cls): 'type': 'string', 'secret': True, 'ask_at_runtime': True - }] + }], + 'dependencies': { + 'ssh_key_unlock': ['ssh_key_data'], + } } ) @@ -665,7 +668,10 @@ def scm(cls): 'label': 'Private Key Passphrase', 'type': 'string', 'secret': True - }] + }], + 'dependencies': { + 'ssh_key_unlock': ['ssh_key_data'], + } } ) @@ -725,7 +731,11 @@ def net(cls): 'label': 'Authorize Password', 'type': 'string', 'secret': True, - }] + }], + 'dependencies': { + 'ssh_key_unlock': ['ssh_key_data'], + 'authorize_password': ['authorize'], + } } ) diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index 4a94777bb8..ac9c1be960 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -1,4 +1,5 @@ import itertools +import re import mock # noqa import pytest @@ -711,7 +712,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred @pytest.mark.django_db @pytest.mark.parametrize('field_name, field_value', itertools.product( - ['username', 'password', 'ssh_key_data', 'ssh_key_unlock', 'become_method', 'become_username', 'become_password'], # noqa + ['username', 'password', 'ssh_key_data', 'become_method', 'become_username', 'become_password'], # noqa ['', None] )) def test_nullish_field_data(get, post, organization, admin, field_name, field_value): @@ -762,6 +763,33 @@ def test_falsey_field_data(get, post, organization, admin, field_value): assert cred.authorize is False +@pytest.mark.django_db +@pytest.mark.parametrize('kind, extraneous', [ + ['ssh', 'ssh_key_unlock'], + ['scm', 'ssh_key_unlock'], + ['net', 'ssh_key_unlock'], + ['net', 'authorize_password'], +]) +def test_field_dependencies(get, post, organization, admin, kind, extraneous): + _type = CredentialType.defaults[kind]() + _type.save() + params = { + 'name': 'Best credential ever', + 'credential_type': _type.pk, + 'organization': organization.id, + 'inputs': {extraneous: 'not needed'} + } + response = post( + reverse('api:credential_list', kwargs={'version': 'v2'}), + params, + admin + ) + assert response.status_code == 400 + assert re.search('cannot be set unless .+ is set.', response.content) + + assert Credential.objects.count() == 0 + + # # SCM Credentials # diff --git a/awx/main/tests/functional/api/test_credential_type.py b/awx/main/tests/functional/api/test_credential_type.py index a8c9d2000f..38c8998f30 100644 --- a/awx/main/tests/functional/api/test_credential_type.py +++ b/awx/main/tests/functional/api/test_credential_type.py @@ -80,16 +80,15 @@ def test_update_managed_by_tower_xfail(patch, delete, admin): @pytest.mark.django_db def test_update_credential_type_in_use_xfail(patch, delete, admin): - ssh = CredentialType.defaults['ssh']() - ssh.managed_by_tower = False - ssh.save() - Credential(credential_type=ssh, name='My SSH Key').save() + _type = CredentialType(kind='cloud', inputs={'fields': []}) + _type.save() + Credential(credential_type=_type, name='My Custom Cred').save() - url = reverse('api:credential_type_detail', kwargs={'pk': ssh.pk}) + url = reverse('api:credential_type_detail', kwargs={'pk': _type.pk}) response = patch(url, {'name': 'Some Other Name'}, admin) assert response.status_code == 200 - url = reverse('api:credential_type_detail', kwargs={'pk': ssh.pk}) + url = reverse('api:credential_type_detail', kwargs={'pk': _type.pk}) response = patch(url, {'inputs': {}}, admin) assert response.status_code == 403 @@ -98,11 +97,10 @@ def test_update_credential_type_in_use_xfail(patch, delete, admin): @pytest.mark.django_db def test_update_credential_type_success(get, patch, delete, admin): - ssh = CredentialType.defaults['ssh']() - ssh.managed_by_tower = False - ssh.save() + _type = CredentialType(kind='cloud') + _type.save() - url = reverse('api:credential_type_detail', kwargs={'pk': ssh.pk}) + url = reverse('api:credential_type_detail', kwargs={'pk': _type.pk}) response = patch(url, {'name': 'Some Other Name'}, admin) assert response.status_code == 200 @@ -163,6 +161,21 @@ def test_create_managed_by_tower_readonly(get, post, admin): assert response.data['results'][0]['managed_by_tower'] is False +@pytest.mark.django_db +def test_create_dependencies_not_supported(get, post, admin): + response = post(reverse('api:credential_type_list'), { + 'kind': 'cloud', + 'name': 'Custom Credential Type', + 'inputs': {'dependencies': {'foo': ['bar']}}, + 'injectors': {}, + }, admin) + assert response.status_code == 400 + assert response.data['inputs'] == ["'dependencies' is not supported for custom credentials."] + + response = get(reverse('api:credential_type_list'), admin) + assert response.data['count'] == 0 + + @pytest.mark.django_db @pytest.mark.parametrize('kind', ['cloud', 'net']) def test_create_valid_kind(kind, get, post, admin):