From f6fb46d99ed9db6470e2c08036bd78451e03c654 Mon Sep 17 00:00:00 2001 From: Lila Date: Thu, 12 May 2022 13:35:17 -0400 Subject: [PATCH 1/3] Prevent edit of vault ID once credential is created and added check to ensure user is actually trying to change vault id. --- awx/api/serializers.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index f70e582c89..29a90c08c3 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2664,6 +2664,13 @@ class CredentialSerializer(BaseSerializer): return credential_type + def validate_inputs(self, inputs): + if self.instance and self.instance.credential_type.kind == "vault": + if 'vault_id' in inputs and inputs['vault_id'] != self.instance.inputs['vault_id']: + raise ValidationError(_('We do not permit Vault IDs to be changed after they have been created.')) + + return inputs + class CredentialSerializerCreate(CredentialSerializer): From 40279bc6c0ca26c39a50529fbbf809b7ac2add74 Mon Sep 17 00:00:00 2001 From: Lila Date: Thu, 12 May 2022 15:49:09 -0400 Subject: [PATCH 2/3] Wrote corresponding tests. Updated verbiage to be more in line with existing messages. --- awx/api/serializers.py | 2 +- .../tests/functional/api/test_credential.py | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 29a90c08c3..08503db45e 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2667,7 +2667,7 @@ class CredentialSerializer(BaseSerializer): def validate_inputs(self, inputs): if self.instance and self.instance.credential_type.kind == "vault": if 'vault_id' in inputs and inputs['vault_id'] != self.instance.inputs['vault_id']: - raise ValidationError(_('We do not permit Vault IDs to be changed after they have been created.')) + raise ValidationError(_('Vault IDs cannot be changed once they have been created.')) return inputs diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index 3d277049ab..52814f3655 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -532,6 +532,49 @@ def test_vault_password_required(post, organization, admin): assert 'required fields (vault_password)' in j.job_explanation +@pytest.mark.django_db +def test_vault_id_immutable(post, patch, organization, admin): + vault = CredentialType.defaults['vault']() + vault.save() + response = post( + reverse('api:credential_list'), + { + 'credential_type': vault.pk, + 'organization': organization.id, + 'name': 'Best credential ever', + 'inputs': {'vault_id': 'password', 'vault_password': 'password'}, + }, + admin, + ) + assert response.status_code == 201 + assert Credential.objects.count() == 1 + response = patch( + reverse('api:credential_detail', kwargs={'pk': response.data['id']}), {'inputs': {'vault_id': 'password2', 'vault_password': 'password'}}, admin + ) + assert response.status_code == 400 + assert response.data['inputs'][0] == 'Vault IDs cannot be changed once they have been created.' + + +@pytest.mark.django_db +def test_patch_without_vault_id_valid(post, patch, organization, admin): + vault = CredentialType.defaults['vault']() + vault.save() + response = post( + reverse('api:credential_list'), + { + 'credential_type': vault.pk, + 'organization': organization.id, + 'name': 'Best credential ever', + 'inputs': {'vault_id': 'password', 'vault_password': 'password'}, + }, + admin, + ) + assert response.status_code == 201 + assert Credential.objects.count() == 1 + response = patch(reverse('api:credential_detail', kwargs={'pk': response.data['id']}), {'name': 'worst_credential_ever'}, admin) + assert response.status_code == 200 + + # # Net Credentials # From 78220cad82ae83d5ea1c0b2707a639f25e7751f4 Mon Sep 17 00:00:00 2001 From: Lila Date: Mon, 16 May 2022 16:36:40 -0400 Subject: [PATCH 3/3] Disables ability to edit vault ID on the UI side. --- .../CredentialFormFields/CredentialField.js | 14 ++++++++ .../CredentialField.test.js | 35 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.js b/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.js index fe8b31731b..0d4b1ed840 100644 --- a/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.js +++ b/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.js @@ -1,5 +1,6 @@ /* eslint-disable react/jsx-no-useless-fragment */ import React, { useState } from 'react'; +import { useLocation } from 'react-router-dom'; import { useField, useFormikContext } from 'formik'; import { shape, string } from 'prop-types'; import styled from 'styled-components'; @@ -31,6 +32,7 @@ function CredentialInput({ fieldOptions, isFieldGroupValid, credentialKind, + isVaultIdDisabled, ...rest }) { const [fileName, setFileName] = useState(''); @@ -148,6 +150,7 @@ function CredentialInput({ onChange={(value, event) => { subFormField.onChange(event); }} + isDisabled={isVaultIdDisabled} validated={isValid ? 'default' : 'error'} /> ); @@ -167,6 +170,7 @@ CredentialInput.defaultProps = { function CredentialField({ credentialType, fieldOptions }) { const { values: formikValues } = useFormikContext(); + const location = useLocation(); const requiredFields = credentialType?.inputs?.required || []; const isRequired = requiredFields.includes(fieldOptions.id); const validateField = () => { @@ -242,6 +246,15 @@ function CredentialField({ credentialType, fieldOptions }) { ); } + + let disabled = false; + if ( + credentialType.kind === 'vault' && + location.pathname.endsWith('edit') && + fieldOptions.id === 'vault_id' + ) { + disabled = true; + } return ( ); diff --git a/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.test.js b/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.test.js index 78b1cd8dcd..3902fb1d58 100644 --- a/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.test.js +++ b/awx/ui/src/screens/Credential/shared/CredentialFormFields/CredentialField.test.js @@ -13,6 +13,12 @@ const fieldOptions = { secret: true, }; +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useLocation: () => ({ + pathname: '/credentials/3/edit', + }), +})); describe('', () => { let wrapper; test('renders correctly without initial value', () => { @@ -113,4 +119,33 @@ describe('', () => { expect(wrapper.find('TextInput').props().value).toBe(''); expect(wrapper.find('TextInput').props().placeholder).toBe('ENCRYPTED'); }); + test('Should check to see if the ability to edit vault ID is disabled after creation.', () => { + const vaultCredential = credentialTypes.find((type) => type.id === 3); + const vaultFieldOptions = { + id: 'vault_id', + label: 'Vault Identifier', + type: 'string', + secret: true, + }; + wrapper = mountWithContexts( + + {() => ( + + )} + + ); + expect(wrapper.find('CredentialInput').props().isDisabled).toBe(true); + expect(wrapper.find('KeyIcon').length).toBe(1); + }); });