This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 0ca51d9796a fix(Database Connection Form): Added validation to db
credential input (#37919)
0ca51d9796a is described below
commit 0ca51d9796ab5246f18853da96eab7dc86b7f3ec
Author: isaac-jaynes-imperva <[email protected]>
AuthorDate: Sun Jul 12 09:36:41 2026 -0700
fix(Database Connection Form): Added validation to db credential input
(#37919)
Co-authored-by: codeant-ai-for-open-source[bot]
<244253245+codeant-ai-for-open-source[bot]@users.noreply.github.com>
Co-authored-by: Evan Rusackas <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../src/components/Form/LabeledErrorBoundInput.tsx | 8 ++
.../superset-ui-core/src/components/Form/types.ts | 5 +
.../DatabaseConnectionForm/EncryptedField.test.tsx | 142 ++++++++++++++++++++-
.../DatabaseConnectionForm/EncryptedField.tsx | 73 ++++++++---
.../src/features/databases/DatabaseModal/styles.ts | 31 ++---
5 files changed, 224 insertions(+), 35 deletions(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx
b/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx
index 2095cca2f03..cfe59254f9f 100644
---
a/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx
+++
b/superset-frontend/packages/superset-ui-core/src/components/Form/LabeledErrorBoundInput.tsx
@@ -28,6 +28,10 @@ const StyledInput = styled(Input)`
margin: ${({ theme }) => `${theme.sizeUnit}px 0 ${theme.sizeUnit * 2}px`};
`;
+const StyledTextArea = styled(Input.TextArea)`
+ margin: ${({ theme }) => `${theme.sizeUnit}px 0 ${theme.sizeUnit * 2}px`};
+`;
+
const StyledInputPassword = styled(Input.Password)`
margin: ${({ theme }) => `${theme.sizeUnit}px 0 ${theme.sizeUnit * 2}px`};
`;
@@ -62,6 +66,8 @@ export const LabeledErrorBoundInput = ({
get_url,
description,
isValidating = false,
+ renderAsTextArea,
+ textAreaCss,
...props
}: LabeledErrorBoundInputProps) => {
const hasError = !!errorMessage;
@@ -98,6 +104,8 @@ export const LabeledErrorBoundInput = ({
}
role="textbox"
/>
+ ) : renderAsTextArea ? (
+ <StyledTextArea css={textAreaCss} {...props} {...validationMethods}
/>
) : (
<StyledInput {...props} {...validationMethods} />
)}
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/Form/types.ts
b/superset-frontend/packages/superset-ui-core/src/components/Form/types.ts
index ffff65dc35a..bcf5db252bb 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Form/types.ts
+++ b/superset-frontend/packages/superset-ui-core/src/components/Form/types.ts
@@ -16,7 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { SerializedStyles } from '@emotion/react';
+
export type { FormProps, FormInstance, FormItemProps } from 'antd/es/form';
+export type { SerializedStyles };
export interface LabeledErrorBoundInputProps {
label?: string;
@@ -31,5 +34,7 @@ export interface LabeledErrorBoundInputProps {
classname?: string;
visibilityToggle?: boolean;
isValidating?: boolean;
+ renderAsTextArea?: boolean;
+ textAreaCss?: SerializedStyles;
[x: string]: any;
}
diff --git
a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.test.tsx
b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.test.tsx
index 2215f0ef0e7..bf067bbbe17 100644
---
a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.test.tsx
+++
b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.test.tsx
@@ -17,7 +17,12 @@
* under the License.
*/
-import { render, fireEvent, screen } from 'spec/helpers/testing-library';
+import {
+ render,
+ fireEvent,
+ screen,
+ waitFor,
+} from 'spec/helpers/testing-library';
import { DatabaseObject, ConfigurationMethod } from '../../types';
import { EncryptedField, encryptedCredentialsMap } from './EncryptedField';
@@ -324,6 +329,110 @@ describe('EncryptedField', () => {
expect(screen.getByRole('textbox')).toBeInTheDocument();
expect(screen.queryByText('Upload credentials')).not.toBeInTheDocument();
});
+
+ test('uploading a file does not validate stale state, and validates once
the parent commits the new value', async () => {
+ const changeMethods = createMockChangeMethods();
+ const getValidation = jest.fn();
+ const fileContent = '{"type": "service_account"}';
+
+ const { rerender } = render(
+ <EncryptedField
+ {...defaultProps}
+ changeMethods={changeMethods}
+ getValidation={getValidation}
+ db={createMockDb('bigquery', { credentials_info: '' })}
+ />,
+ );
+
+ const file = new File([fileContent], 'creds.json', {
+ type: 'application/json',
+ });
+ const input = document.querySelector(
+ 'input[type="file"]',
+ ) as HTMLInputElement;
+
+ fireEvent.change(input, { target: { files: [file] } });
+
+ await waitFor(() =>
+ expect(changeMethods.onParametersChange).toHaveBeenCalledWith(
+ expect.objectContaining({
+ target: expect.objectContaining({ value: fileContent }),
+ }),
+ ),
+ );
+
+ // The parent hasn't re-rendered with the committed value yet (in the
+ // real app this is the reducer-dispatch/render gap) — validation must
+ // not fire against the stale `db` in the meantime.
+ expect(getValidation).not.toHaveBeenCalled();
+
+ // Once the parent commits the update and re-renders with the new
+ // `db.parameters.credentials_info`, validation should fire exactly
+ // once, against the up-to-date value.
+ rerender(
+ <EncryptedField
+ {...defaultProps}
+ changeMethods={changeMethods}
+ getValidation={getValidation}
+ db={createMockDb('bigquery', { credentials_info: fileContent })}
+ />,
+ );
+
+ await waitFor(() => expect(getValidation).toHaveBeenCalledTimes(1));
+ });
+
+ test('removing an upload before the parent commits clears the pending
validation', async () => {
+ const changeMethods = createMockChangeMethods();
+ const getValidation = jest.fn();
+ const fileContent = '{"type": "service_account"}';
+
+ const { container, rerender } = render(
+ <EncryptedField
+ {...defaultProps}
+ changeMethods={changeMethods}
+ getValidation={getValidation}
+ db={createMockDb('bigquery', { credentials_info: '' })}
+ />,
+ );
+
+ const file = new File([fileContent], 'creds.json', {
+ type: 'application/json',
+ });
+ const input = document.querySelector(
+ 'input[type="file"]',
+ ) as HTMLInputElement;
+ fireEvent.change(input, { target: { files: [file] } });
+
+ await waitFor(() =>
+ expect(changeMethods.onParametersChange).toHaveBeenCalledWith(
+ expect.objectContaining({
+ target: expect.objectContaining({ value: fileContent }),
+ }),
+ ),
+ );
+
+ // Remove the upload before the parent has committed/re-rendered with
+ // the uploaded content — this must clear the pending validation, not
+ // just leave it dangling.
+ const removeButton = container.querySelector(
+ '[aria-label="delete"]',
+ ) as HTMLElement;
+ fireEvent.click(removeButton);
+
+ // Even if some later, unrelated render happens to carry the same
+ // content the removed upload had (e.g. stale props echoed back), the
+ // cleared pending state must not fire a validation for it.
+ rerender(
+ <EncryptedField
+ {...defaultProps}
+ changeMethods={changeMethods}
+ getValidation={getValidation}
+ db={createMockDb('bigquery', { credentials_info: fileContent })}
+ />,
+ );
+
+ expect(getValidation).not.toHaveBeenCalled();
+ });
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
@@ -373,6 +482,37 @@ describe('EncryptedField', () => {
});
});
+ // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
+ test('renderAsTextArea branch: renders a textarea element (not a plain
input) in edit mode', () => {
+ const props = { ...defaultProps, isEditMode: true };
+ const { container } = render(<EncryptedField {...props} />);
+
+ // renderAsTextArea causes LabeledErrorBoundInput to render <textarea>
+ expect(container.querySelector('textarea')).toBeInTheDocument();
+ expect(container.querySelector('input')).not.toBeInTheDocument();
+ });
+
+ test('renderAsTextArea branch: renders a textarea element in copy-paste
mode', () => {
+ const props = { ...defaultProps, isEditMode: false };
+ render(<EncryptedField {...props} />);
+
+ // Switch to copy-paste option to trigger renderAsTextArea branch
+ const select = screen.getByRole('combobox');
+ fireEvent.mouseDown(select);
+ fireEvent.click(screen.getByText('Copy and Paste JSON credentials'));
+
+ expect(screen.getByRole('textbox').tagName).toBe('TEXTAREA');
+ });
+
+ test('renderAsTextArea branch: does not render a textarea when upload option
is selected', () => {
+ const props = { ...defaultProps, isEditMode: false, editNewDb: false };
+ render(<EncryptedField {...props} />);
+
+ // Default upload option — no textarea should be present
+ expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+ expect(screen.getByText('Upload credentials')).toBeInTheDocument();
+ });
+
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
describe('Error Boundaries', () => {
test('renders gracefully when database prop is missing', () => {
diff --git
a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
index 5c1eec0a302..f1dddf76223 100644
---
a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
+++
b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
@@ -18,19 +18,23 @@
*/
import { useState, useEffect } from 'react';
import { t } from '@apache-superset/core/translation';
-import { SupersetTheme } from '@apache-superset/core/theme';
+import { SupersetTheme, useTheme } from '@apache-superset/core/theme';
import {
- Input,
Button,
FormLabel,
Select,
Upload,
type UploadFile,
+ LabeledErrorBoundInput as ValidatedInput,
} from '@superset-ui/core/components';
import { Icons } from '@superset-ui/core/components/Icons';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { DatabaseParameters, Engines, FieldPropTypes } from '../../types';
-import { infoTooltip, CredentialInfoForm } from '../styles';
+import {
+ infoTooltip,
+ CredentialInfoForm,
+ CredentialInfoFormTextArea,
+} from '../styles';
enum CredentialInfoOptions {
JsonUpload,
@@ -53,11 +57,24 @@ export const EncryptedField = ({
editNewDb,
isPublic = true,
setIsPublic,
+ isValidating,
+ validationErrors,
+ getValidation,
}: FieldPropTypes) => {
+ const theme = useTheme() as SupersetTheme;
+ const credentialTextAreaCss = CredentialInfoFormTextArea(theme);
const [fileList, setFileList] = useState<UploadFile[]>([]);
const [uploadOption, setUploadOption] = useState<number>(
CredentialInfoOptions.JsonUpload.valueOf(),
);
+ // `getValidation` closes over the parent's `db` state as of the last
+ // render, so calling it synchronously right after `onParametersChange`
+ // (which just dispatches a state update) would validate the *previous*
+ // credential value. Instead, remember the uploaded value and validate
+ // once the parameter prop actually reflects it.
+ const [pendingValidationValue, setPendingValidationValue] = useState<
+ string | null
+ >(null);
const { addDangerToast } = useToasts();
const isGSheets = db?.engine === Engines.GSheet;
const showCredentialsInfo = !isEditMode && (!isGSheets || !isPublic);
@@ -123,6 +140,16 @@ export const EncryptedField = ({
});
}, []);
+ useEffect(() => {
+ if (
+ pendingValidationValue !== null &&
+ paramValue === pendingValidationValue
+ ) {
+ setPendingValidationValue(null);
+ getValidation();
+ }
+ }, [paramValue, pendingValidationValue, getValidation]);
+
return (
<CredentialInfoForm>
{isGSheets && (
@@ -168,22 +195,27 @@ export const EncryptedField = ({
(uploadOption === CredentialInfoOptions.CopyPaste ||
isEditMode ||
editNewDb) ? (
- <div className="input-container">
- <FormLabel>{t('Service Account')}</FormLabel>
- <Input.TextArea
- className="input-form"
- name={encryptedField}
- value={
- typeof encryptedValue === 'boolean'
- ? String(encryptedValue)
- : encryptedValue
- }
- onChange={changeMethods.onParametersChange}
- placeholder={t(
- 'Paste content of service credentials JSON file here',
- )}
- />
- </div>
+ <ValidatedInput
+ id={encryptedField}
+ name={encryptedField}
+ required={false}
+ isValidating={isValidating}
+ value={
+ typeof encryptedValue === 'boolean'
+ ? String(encryptedValue)
+ : encryptedValue
+ }
+ validationMethods={{ onBlur: getValidation }}
+ errorMessage={
+ encryptedField ? validationErrors?.[encryptedField] : null
+ }
+ placeholder={t('Paste content of service credentials JSON file
here')}
+ label={t('Service Account')}
+ onChange={changeMethods.onParametersChange}
+ helpText={t('Add service credentials')}
+ renderAsTextArea
+ textAreaCss={credentialTextAreaCss}
+ />
) : (
showCredentialsInfo && (
<div
@@ -198,6 +230,7 @@ export const EncryptedField = ({
beforeUpload={() => false}
onRemove={() => {
setFileList([]);
+ setPendingValidationValue(null);
changeMethods.onParametersChange({
target: {
name: encryptedField,
@@ -220,6 +253,7 @@ export const EncryptedField = ({
},
});
setFileList(info.fileList);
+ setPendingValidationValue(fileContent);
} catch {
setFileList([]);
addDangerToast(
@@ -229,6 +263,7 @@ export const EncryptedField = ({
);
}
} else {
+ setPendingValidationValue(null);
changeMethods.onParametersChange({
target: {
name: encryptedField,
diff --git a/superset-frontend/src/features/databases/DatabaseModal/styles.ts
b/superset-frontend/src/features/databases/DatabaseModal/styles.ts
index 6e065acebb6..59770c17810 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/styles.ts
+++ b/superset-frontend/src/features/databases/DatabaseModal/styles.ts
@@ -383,6 +383,18 @@ export const EditHeaderSubtitle = styled.div`
font-weight: ${({ theme }) => theme.fontWeightStrong};
`;
+export const CredentialInfoFormTextArea = (theme: SupersetTheme) => css`
+ height: 100px;
+ width: 100%;
+ border: 1px solid ${theme.colorBorder};
+ border-radius: ${theme.borderRadius}px;
+ resize: vertical;
+ padding: ${theme.sizeUnit * 1.5}px ${theme.sizeUnit * 2}px;
+ &::placeholder {
+ color: ${theme.colorTextPlaceholder};
+ }
+`;
+
export const CredentialInfoForm = styled.div`
margin-top: ${({ theme }) => theme.sizeUnit * 4}px;
@@ -413,19 +425,6 @@ export const CredentialInfoForm = styled.div`
margin: ${({ theme }) => theme.sizeUnit * 4}px 0;
display: flex;
flex-direction: column;
-}
- }
- .input-form {
- height: 100px;
- width: 100%;
- border: 1px solid ${({ theme }) => theme.colorBorder};
- border-radius: ${({ theme }) => theme.borderRadius}px;
- resize: vertical;
- padding: ${({ theme }) => theme.sizeUnit * 1.5}px
- ${({ theme }) => theme.sizeUnit * 2}px;
- &::placeholder {
- color: ${({ theme }) => theme.colorTextPlaceholder};
- }
}
.input-container {
@@ -442,7 +441,8 @@ export const CredentialInfoForm = styled.div`
width: fit-content;
}
- .credentials-uploaded-btn, .credentials-uploaded-remove {
+ .credentials-uploaded-btn,
+ .credentials-uploaded-remove {
flex: 0 0 auto;
}
@@ -450,7 +450,8 @@ export const CredentialInfoForm = styled.div`
.input-upload {
display: none !important;
}
- }`;
+ }
+`;
export const SelectDatabaseStyles = styled.div`
.preferred {
