Merge pull request #9368 from mabashian/7256-cred-password-replace

Add support for replace/revert on secret credential fields

SUMMARY
link #7256
Note that this only applies to editing an existing credential.  You should not see this button on fields when adding a new credential.
When editing an existing credential the replace button should show up on fields where secret is true and the field has an existing value that is not an external credential.  Examples:



Fields with external credentials should look the same:

Initially the button tooltip should say Replace.  Clicking Replace will clear out the previously saved value and enable the form field:

The tooltip will change to Revert.  Clicking Revert will take the field back to it's original state.
I also noticed a race condition which would result in the input fields (subform) not being populated due to the form rendering before the request(s) were completed.  I fixed this.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

UI

Reviewed-by: Kersom <None>
Reviewed-by: Tiago Góes <tiago.goes2009@gmail.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-03-19 17:40:16 +00:00 committed by GitHub
commit 5c0850b279
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 206 additions and 15 deletions

View File

@ -127,7 +127,7 @@ function CredentialAdd({ me }) {
</PageSection>
);
}
if (isLoading && !result) {
if (isLoading) {
return (
<PageSection>
<Card>

View File

@ -178,7 +178,7 @@ function CredentialEdit({ credential }) {
return <ContentError error={error} />;
}
if (isLoading && !credentialTypes) {
if (isLoading) {
return <ContentLoading />;
}

View File

@ -1,5 +1,5 @@
import React, { useCallback, useState } from 'react';
import { func, shape } from 'prop-types';
import { shape } from 'prop-types';
import { Formik, useField, useFormikContext } from 'formik';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
@ -315,8 +315,6 @@ function CredentialForm({
}
CredentialForm.propTypes = {
handleSubmit: func.isRequired,
handleCancel: func.isRequired,
credentialTypes: shape({}).isRequired,
credential: shape({}),
inputSources: shape({}),

View File

@ -5,11 +5,15 @@ import styled from 'styled-components';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
import {
Button,
ButtonVariant,
FileUpload as PFFileUpload,
FormGroup,
InputGroup,
TextInput,
Tooltip,
} from '@patternfly/react-core';
import { PficonHistoryIcon } from '@patternfly/react-icons';
import { PasswordInput } from '../../../../components/FormField';
import AnsibleSelect from '../../../../components/AnsibleSelect';
import Popover from '../../../../components/Popover';
@ -22,20 +26,79 @@ const FileUpload = styled(PFFileUpload)`
flex-grow: 1;
`;
function CredentialInput({ fieldOptions, credentialKind, ...rest }) {
function CredentialInput({ i18n, fieldOptions, credentialKind, ...rest }) {
const [fileName, setFileName] = useState('');
const [fileIsUploading, setFileIsUploading] = useState(false);
const [subFormField, meta, helpers] = useField(`inputs.${fieldOptions.id}`);
const isValid = !(meta.touched && meta.error);
const RevertReplaceButton = (
<>
{meta.initialValue &&
meta.initialValue !== '' &&
!meta.initialValue.credential && (
<Tooltip
id={`credential-${fieldOptions.id}-replace-tooltip`}
content={
meta.value !== meta.initialValue
? i18n._(t`Revert`)
: i18n._(t`Replace`)
}
>
<Button
id={`credential-${fieldOptions.id}-replace-button`}
variant={ButtonVariant.control}
aria-label={
meta.touched
? i18n._(t`Revert field to previously saved value`)
: i18n._(t`Replace field with new value`)
}
onClick={() => {
if (meta.value !== meta.initialValue) {
helpers.setValue(meta.initialValue);
} else {
helpers.setValue('', false);
}
}}
>
<PficonHistoryIcon />
</Button>
</Tooltip>
)}
</>
);
if (fieldOptions.multiline) {
const handleFileChange = (value, filename) => {
helpers.setValue(value);
setFileName(filename);
};
if (fieldOptions.secret) {
return (
<>
{RevertReplaceButton}
<FileUpload
{...subFormField}
{...rest}
id={`credential-${fieldOptions.id}`}
type="text"
filename={fileName}
onChange={handleFileChange}
onReadStarted={() => setFileIsUploading(true)}
onReadFinished={() => setFileIsUploading(false)}
isLoading={fileIsUploading}
allowEditingUploadedText
validated={isValid ? 'default' : 'error'}
/>
</>
);
}
return (
<FileUpload
{...subFormField}
{...rest}
id={`credential-${fieldOptions.id}`}
type="text"
filename={fileName}
@ -48,13 +111,17 @@ function CredentialInput({ fieldOptions, credentialKind, ...rest }) {
/>
);
}
if (fieldOptions.secret) {
const passwordInput = () => (
<PasswordInput
{...subFormField}
id={`credential-${fieldOptions.id}`}
{...rest}
/>
<>
{RevertReplaceButton}
<PasswordInput
{...subFormField}
id={`credential-${fieldOptions.id}`}
{...rest}
/>
</>
);
return credentialKind === 'external' ? (
<InputGroup>{passwordInput()}</InputGroup>
@ -147,8 +214,16 @@ function CredentialField({ credentialType, fieldOptions, i18n }) {
validated={isValid ? 'default' : 'error'}
>
<CredentialInput
i18n={i18n}
credentialKind={credentialType.kind}
fieldOptions={fieldOptions}
isDisabled={
!!(
meta.initialValue &&
meta.initialValue !== '' &&
meta.value === meta.initialValue
)
}
/>
</FormGroup>
);
@ -164,7 +239,7 @@ function CredentialField({ credentialType, fieldOptions, i18n }) {
isRequired={isRequired}
validated={isValid ? 'default' : 'error'}
>
<CredentialInput fieldOptions={fieldOptions} />
<CredentialInput i18n={i18n} fieldOptions={fieldOptions} />
</CredentialPluginField>
);
}

View File

@ -0,0 +1,110 @@
import React from 'react';
import { Formik } from 'formik';
import { mountWithContexts } from '../../../../../testUtils/enzymeHelpers';
import credentialTypes from '../data.credentialTypes.json';
import CredentialField from './CredentialField';
const credentialType = credentialTypes.find(type => type.id === 5);
const fieldOptions = {
id: 'password',
label: 'Secret Key',
type: 'string',
secret: true,
};
describe('<CredentialField />', () => {
let wrapper;
afterEach(() => {
wrapper.unmount();
});
test('renders correctly without initial value', () => {
wrapper = mountWithContexts(
<Formik
initialValues={{
passwordPrompts: {},
inputs: {
password: '',
},
}}
>
{() => (
<CredentialField
fieldOptions={fieldOptions}
credentialType={credentialType}
/>
)}
</Formik>
);
expect(wrapper.find('CredentialField').length).toBe(1);
expect(wrapper.find('PasswordInput').length).toBe(1);
expect(wrapper.find('TextInput').props().isDisabled).toBe(false);
expect(wrapper.find('KeyIcon').length).toBe(1);
expect(wrapper.find('PficonHistoryIcon').length).toBe(0);
});
test('renders correctly with initial value', () => {
wrapper = mountWithContexts(
<Formik
initialValues={{
passwordPrompts: {},
inputs: {
password: '$encrypted$',
},
}}
>
{() => (
<CredentialField
fieldOptions={fieldOptions}
credentialType={credentialType}
/>
)}
</Formik>
);
expect(wrapper.find('CredentialField').length).toBe(1);
expect(wrapper.find('PasswordInput').length).toBe(1);
expect(wrapper.find('TextInput').props().isDisabled).toBe(true);
expect(wrapper.find('TextInput').props().value).toBe('');
expect(wrapper.find('TextInput').props().placeholder).toBe('ENCRYPTED');
expect(wrapper.find('KeyIcon').length).toBe(1);
expect(wrapper.find('PficonHistoryIcon').length).toBe(1);
});
test('replace/revert button behaves as expected', () => {
wrapper = mountWithContexts(
<Formik
initialValues={{
passwordPrompts: {},
inputs: {
password: '$encrypted$',
},
}}
>
{() => (
<CredentialField
fieldOptions={fieldOptions}
credentialType={credentialType}
/>
)}
</Formik>
);
expect(
wrapper.find('Tooltip#credential-password-replace-tooltip').props()
.content
).toBe('Replace');
expect(wrapper.find('TextInput').props().isDisabled).toBe(true);
expect(wrapper.find('PficonHistoryIcon').simulate('click'));
expect(
wrapper.find('Tooltip#credential-password-replace-tooltip').props()
.content
).toBe('Revert');
expect(wrapper.find('TextInput').props().isDisabled).toBe(false);
expect(wrapper.find('TextInput').props().value).toBe('');
expect(wrapper.find('TextInput').props().placeholder).toBe(undefined);
expect(wrapper.find('PficonHistoryIcon').simulate('click'));
expect(
wrapper.find('Tooltip#credential-password-replace-tooltip').props()
.content
).toBe('Replace');
expect(wrapper.find('TextInput').props().isDisabled).toBe(true);
expect(wrapper.find('TextInput').props().value).toBe('');
expect(wrapper.find('TextInput').props().placeholder).toBe('ENCRYPTED');
});
});

View File

@ -27,9 +27,17 @@ function CredentialPluginInput(props) {
} = props;
const [showPluginWizard, setShowPluginWizard] = useState(false);
const [inputField, , helpers] = useField(`inputs.${fieldOptions.id}`);
const [inputField, meta, helpers] = useField(`inputs.${fieldOptions.id}`);
const [passwordPromptField] = useField(`passwordPrompts.${fieldOptions.id}`);
const disableFieldAndButtons =
!!passwordPromptField.value ||
!!(
meta.initialValue &&
meta.initialValue !== '' &&
meta.value === meta.initialValue
);
return (
<>
{inputField?.value?.credential ? (
@ -44,7 +52,7 @@ function CredentialPluginInput(props) {
...inputField,
isRequired,
validated: isValid ? 'default' : 'error',
isDisabled: !!passwordPromptField.value,
isDisabled: disableFieldAndButtons,
onChange: (_, event) => {
inputField.onChange(event);
},
@ -61,7 +69,7 @@ function CredentialPluginInput(props) {
t`Populate field from an external secret management system`
)}
onClick={() => setShowPluginWizard(true)}
isDisabled={isDisabled || !!passwordPromptField.value}
isDisabled={isDisabled || disableFieldAndButtons}
>
<KeyIcon />
</Button>