Skip to content

Commit ff62aa9

Browse files
rylndrylnd
committed
[Security Solution][Detections] Validate file type of value lists (#72746)
* UI validates file type of uploaded value list * file picker itself is restricted to text/csv and text/plain * if they drag/drop an invalid file, we disable the upload button and display an error message * refactors form state to be a File instead of a FileList * Refactor validation and error message in terms of file type Instead of maintaining lists of both valid extensions and valid mime types, we simply use the latter.
1 parent e0f017b commit ff62aa9

3 files changed

Lines changed: 55 additions & 11 deletions

File tree

‎x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const mockUseImportList = useImportList as jest.Mock;
1616

1717
const mockFile = ({
1818
name: 'foo.csv',
19-
path: '/home/foo.csv',
19+
type: 'text/csv',
2020
} as unknown) as File;
2121

2222
const mockSelectFile: <P>(container: ReactWrapper<P>, file: File) => Promise<void> = async (
@@ -26,7 +26,7 @@ const mockSelectFile: <P>(container: ReactWrapper<P>, file: File) => Promise<voi
2626
const fileChange = container.find('EuiFilePicker').prop('onChange');
2727
act(() => {
2828
if (fileChange) {
29-
fileChange(([file] as unknown) as FormEvent);
29+
fileChange(({ item: () => file } as unknown) as FormEvent);
3030
}
3131
});
3232
};
@@ -83,6 +83,29 @@ describe('ValueListsForm', () => {
8383
expect(onError).toHaveBeenCalledWith('whoops');
8484
});
8585

86+
it('disables upload and displays an error if file has invalid extension', async () => {
87+
const badMockFile = ({
88+
name: 'foo.pdf',
89+
type: 'application/pdf',
90+
} as unknown) as File;
91+
92+
const container = mount(
93+
<TestProviders>
94+
<ValueListsForm onError={jest.fn()} onSuccess={jest.fn()} />
95+
</TestProviders>
96+
);
97+
98+
await mockSelectFile(container, badMockFile);
99+
100+
expect(
101+
container.find('button[data-test-subj="value-lists-form-import-action"]').prop('disabled')
102+
).toEqual(true);
103+
104+
expect(container.find('div[data-test-subj="value-list-file-picker-row"]').text()).toContain(
105+
'File must be one of the following types: [text/csv, text/plain]'
106+
);
107+
});
108+
86109
it('calls onSuccess if import succeeds', async () => {
87110
mockUseImportList.mockImplementation(() => ({
88111
start: jest.fn(),

‎x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.tsx‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const options: ListTypeOptions[] = [
4646
];
4747

4848
const defaultListType: Type = 'keyword';
49+
const validFileTypes = ['text/csv', 'text/plain'];
4950

5051
export interface ValueListsFormProps {
5152
onError: (error: Error) => void;
@@ -54,23 +55,29 @@ export interface ValueListsFormProps {
5455

5556
export const ValueListsFormComponent: React.FC<ValueListsFormProps> = ({ onError, onSuccess }) => {
5657
const ctrl = useRef(new AbortController());
57-
const [files, setFiles] = useState<FileList | null>(null);
58+
const [file, setFile] = useState<File | null>(null);
5859
const [type, setType] = useState<Type>(defaultListType);
5960
const filePickerRef = useRef<EuiFilePicker | null>(null);
6061
const { http } = useKibana().services;
6162
const { start: importList, ...importState } = useImportList();
6263

64+
const fileIsValid = !file || validFileTypes.some((fileType) => file.type === fileType);
65+
6366
// EuiRadioGroup's onChange only infers 'string' from our options
6467
const handleRadioChange = useCallback((t: string) => setType(t as Type), [setType]);
6568

69+
const handleFileChange = useCallback((files: FileList | null) => {
70+
setFile(files?.item(0) ?? null);
71+
}, []);
72+
6673
const resetForm = useCallback(() => {
6774
if (filePickerRef.current?.fileInput) {
6875
filePickerRef.current.fileInput.value = '';
6976
filePickerRef.current.handleChange();
7077
}
71-
setFiles(null);
78+
setFile(null);
7279
setType(defaultListType);
73-
}, [setType]);
80+
}, []);
7481

7582
const handleCancel = useCallback(() => {
7683
ctrl.current.abort();
@@ -91,17 +98,17 @@ export const ValueListsFormComponent: React.FC<ValueListsFormProps> = ({ onError
9198
);
9299

93100
const handleImport = useCallback(() => {
94-
if (!importState.loading && files && files.length) {
101+
if (!importState.loading && file) {
95102
ctrl.current = new AbortController();
96103
importList({
97-
file: files[0],
104+
file,
98105
listId: undefined,
99106
http,
100107
signal: ctrl.current.signal,
101108
type,
102109
});
103110
}
104-
}, [importState.loading, files, importList, http, type]);
111+
}, [importState.loading, file, importList, http, type]);
105112

106113
useEffect(() => {
107114
if (!importState.loading && importState.result) {
@@ -117,14 +124,22 @@ export const ValueListsFormComponent: React.FC<ValueListsFormProps> = ({ onError
117124

118125
return (
119126
<EuiForm>
120-
<EuiFormRow label={i18n.FILE_PICKER_LABEL} fullWidth>
127+
<EuiFormRow
128+
data-test-subj="value-list-file-picker-row"
129+
label={i18n.FILE_PICKER_LABEL}
130+
fullWidth
131+
isInvalid={!fileIsValid}
132+
error={[i18n.FILE_PICKER_INVALID_FILE_TYPE(validFileTypes.join(', '))]}
133+
>
121134
<EuiFilePicker
135+
accept={validFileTypes.join()}
122136
id="value-list-file-picker"
123137
initialPromptText={i18n.FILE_PICKER_PROMPT}
124138
ref={filePickerRef}
125-
onChange={setFiles}
139+
onChange={handleFileChange}
126140
fullWidth={true}
127141
isLoading={importState.loading}
142+
isInvalid={!fileIsValid}
128143
/>
129144
</EuiFormRow>
130145
<EuiFormRow fullWidth>
@@ -151,7 +166,7 @@ export const ValueListsFormComponent: React.FC<ValueListsFormProps> = ({ onError
151166
<EuiButton
152167
data-test-subj="value-lists-form-import-action"
153168
onClick={handleImport}
154-
disabled={!files?.length || importState.loading}
169+
disabled={file == null || !fileIsValid || importState.loading}
155170
>
156171
{i18n.UPLOAD_BUTTON}
157172
</EuiButton>

‎x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ export const FILE_PICKER_PROMPT = i18n.translate(
2424
}
2525
);
2626

27+
export const FILE_PICKER_INVALID_FILE_TYPE = (fileTypes: string): string =>
28+
i18n.translate('xpack.securitySolution.lists.uploadValueListExtensionValidationMessage', {
29+
values: { fileTypes },
30+
defaultMessage: 'File must be one of the following types: [{fileTypes}]',
31+
});
32+
2733
export const CLOSE_BUTTON = i18n.translate(
2834
'xpack.securitySolution.lists.closeValueListsModalTitle',
2935
{

0 commit comments

Comments
 (0)