[Security Solution][Detections] Validate file type of value lists (#7…

…2746) * 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.
elastic · Jul 21, 2020 · ba643bd · ba643bd
1 parent a4957e6
commit ba643bd
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 11 deletions.
diff --git a/...security_solution/public/detections/components/value_lists_management_modal/form.test.tsx b/...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: <P>(container: ReactWrapper<P>, file: File) => Promise<void> = async (
@@ -26,7 +26,7 @@ const mockSelectFile: <P>(container: ReactWrapper<P>, file: File) => Promise<voi
   const fileChange = container.find('EuiFilePicker').prop('onChange');
   act(() => {
     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(
+      <TestProviders>
+        <ValueListsForm onError={jest.fn()} onSuccess={jest.fn()} />
+      </TestProviders>
+    );
+
+    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/...gins/security_solution/public/detections/components/value_lists_management_modal/form.tsx b/...gins/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<ValueListsFormProps> = ({ onError, onSuccess }) => {
   const ctrl = useRef(new AbortController());
-  const [files, setFiles] = useState<FileList | null>(null);
+  const [file, setFile] = useState<File | null>(null);
   const [type, setType] = useState<Type>(defaultListType);
   const filePickerRef = useRef<EuiFilePicker | null>(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<ValueListsFormProps> = ({ 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<ValueListsFormProps> = ({ onError
 
   return (
     <EuiForm>
-      <EuiFormRow label={i18n.FILE_PICKER_LABEL} fullWidth>
+      <EuiFormRow
+        data-test-subj="value-list-file-picker-row"
+        label={i18n.FILE_PICKER_LABEL}
+        fullWidth
+        isInvalid={!fileIsValid}
+        error={[i18n.FILE_PICKER_INVALID_FILE_TYPE(validFileTypes.join(', '))]}
+      >
         <EuiFilePicker
+          accept={validFileTypes.join()}
           id="value-list-file-picker"
           initialPromptText={i18n.FILE_PICKER_PROMPT}
           ref={filePickerRef}
-          onChange={setFiles}
+          onChange={handleFileChange}
           fullWidth={true}
           isLoading={importState.loading}
+          isInvalid={!fileIsValid}
         />
       </EuiFormRow>
       <EuiFormRow fullWidth>
@@ -151,7 +166,7 @@ export const ValueListsFormComponent: React.FC<ValueListsFormProps> = ({ onError
                   <EuiButton
                     data-test-subj="value-lists-form-import-action"
                     onClick={handleImport}
-                    disabled={!files?.length || importState.loading}
+                    disabled={file == null || !fileIsValid || importState.loading}
                   >
                     {i18n.UPLOAD_BUTTON}
                   </EuiButton>

diff --git a/...curity_solution/public/detections/components/value_lists_management_modal/translations.ts b/...curity_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',
   {