From ff62aa908cdf488b2c7debc98b2081f1d47bf80c Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 21 Jul 2020 18:31:54 -0500 Subject: [PATCH] [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. --- .../form.test.tsx | 27 +++++++++++++-- .../value_lists_management_modal/form.tsx | 33 ++++++++++++++----- .../translations.ts | 6 ++++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx index e2e793b34eaf9..591e1c81cd2ad 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.test.tsx @@ -16,7 +16,7 @@ const mockUseImportList = useImportList as jest.Mock; const mockFile = ({ name: 'foo.csv', - path: '/home/foo.csv', + type: 'text/csv', } as unknown) as File; const mockSelectFile:

(container: ReactWrapper

, file: File) => Promise = async ( @@ -26,7 +26,7 @@ const mockSelectFile:

(container: ReactWrapper

, file: File) => Promise { if (fileChange) { - fileChange(([file] as unknown) as FormEvent); + fileChange(({ item: () => file } as unknown) as FormEvent); } }); }; @@ -83,6 +83,29 @@ describe('ValueListsForm', () => { expect(onError).toHaveBeenCalledWith('whoops'); }); + it('disables upload and displays an error if file has invalid extension', async () => { + const badMockFile = ({ + name: 'foo.pdf', + type: 'application/pdf', + } as unknown) as File; + + const container = mount( + + + + ); + + await mockSelectFile(container, badMockFile); + + expect( + container.find('button[data-test-subj="value-lists-form-import-action"]').prop('disabled') + ).toEqual(true); + + expect(container.find('div[data-test-subj="value-list-file-picker-row"]').text()).toContain( + 'File must be one of the following types: [text/csv, text/plain]' + ); + }); + it('calls onSuccess if import succeeds', async () => { mockUseImportList.mockImplementation(() => ({ start: jest.fn(), diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.tsx index b8416c3242e4a..aab665289e80d 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/form.tsx @@ -46,6 +46,7 @@ const options: ListTypeOptions[] = [ ]; const defaultListType: Type = 'keyword'; +const validFileTypes = ['text/csv', 'text/plain']; export interface ValueListsFormProps { onError: (error: Error) => void; @@ -54,23 +55,29 @@ export interface ValueListsFormProps { export const ValueListsFormComponent: React.FC = ({ onError, onSuccess }) => { const ctrl = useRef(new AbortController()); - const [files, setFiles] = useState(null); + const [file, setFile] = useState(null); const [type, setType] = useState(defaultListType); const filePickerRef = useRef(null); const { http } = useKibana().services; const { start: importList, ...importState } = useImportList(); + const fileIsValid = !file || validFileTypes.some((fileType) => file.type === fileType); + // EuiRadioGroup's onChange only infers 'string' from our options const handleRadioChange = useCallback((t: string) => setType(t as Type), [setType]); + const handleFileChange = useCallback((files: FileList | null) => { + setFile(files?.item(0) ?? null); + }, []); + const resetForm = useCallback(() => { if (filePickerRef.current?.fileInput) { filePickerRef.current.fileInput.value = ''; filePickerRef.current.handleChange(); } - setFiles(null); + setFile(null); setType(defaultListType); - }, [setType]); + }, []); const handleCancel = useCallback(() => { ctrl.current.abort(); @@ -91,17 +98,17 @@ export const ValueListsFormComponent: React.FC = ({ onError ); const handleImport = useCallback(() => { - if (!importState.loading && files && files.length) { + if (!importState.loading && file) { ctrl.current = new AbortController(); importList({ - file: files[0], + file, listId: undefined, http, signal: ctrl.current.signal, type, }); } - }, [importState.loading, files, importList, http, type]); + }, [importState.loading, file, importList, http, type]); useEffect(() => { if (!importState.loading && importState.result) { @@ -117,14 +124,22 @@ export const ValueListsFormComponent: React.FC = ({ onError return ( - + @@ -151,7 +166,7 @@ export const ValueListsFormComponent: React.FC = ({ onError {i18n.UPLOAD_BUTTON} diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts index dca6e43a98143..91f3f3797f422 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts @@ -24,6 +24,12 @@ export const FILE_PICKER_PROMPT = i18n.translate( } ); +export const FILE_PICKER_INVALID_FILE_TYPE = (fileTypes: string): string => + i18n.translate('xpack.securitySolution.lists.uploadValueListExtensionValidationMessage', { + values: { fileTypes }, + defaultMessage: 'File must be one of the following types: [{fileTypes}]', + }); + export const CLOSE_BUTTON = i18n.translate( 'xpack.securitySolution.lists.closeValueListsModalTitle', {