Fix file upload validation service

build.gradle

@@ -85,6 +85,8 @@ dependencies {
     implementation 'com.mailgun:mailgun-java:1.1.3'
     implementation 'com.google.crypto.tink:tink:1.14.1'
     implementation 'org.springframework.session:spring-session-jdbc'
+    implementation 'org.apache.tika:tika-core:2.9.2'
+
 
     // For M1 issue?
     //runtimeOnly 'io.netty:netty-resolver-dns-native-macos:4.1.97.Final:osx-aarch_64'

src/main/java/formflow/library/file/FileValidationService.java

@@ -2,9 +2,11 @@
 
 import static java.util.Arrays.stream;
 
+import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.tika.Tika;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.MediaType;
 import org.springframework.stereotype.Service;
@@ -99,11 +101,12 @@ public List<String> getAcceptableFileExts() {
    * @param file the file to check the mime type of
    * @return Boolean True if the mimetype is one of the ones the system accepts, False otherwise.
    */
-  public Boolean isAcceptedMimeType(MultipartFile file) {
+  public Boolean isAcceptedMimeType(MultipartFile file) throws IOException {
+    Tika tikaFileValidator = new Tika();
     if (file.getContentType() == null || file.getContentType().isBlank()) {
       return false;
     }
-    return ACCEPTED_MIME_TYPES.contains(MimeType.valueOf(file.getContentType()));
+    return ACCEPTED_MIME_TYPES.contains(MimeType.valueOf(tikaFileValidator.detect(file.getInputStream())));
   }
 
   /**

src/test/java/formflow/library/controllers/FileControllerTest.java

@@ -21,6 +21,7 @@
 import formflow.library.file.ClammitVirusScanner;
 import formflow.library.file.CloudFile;
 import formflow.library.file.CloudFileRepository;
+import formflow.library.file.FileValidationService;
 import formflow.library.utilities.AbstractMockMvcTest;
 import formflow.library.utils.UserFileMap;
 
@@ -75,6 +76,8 @@ public class FileControllerTest extends AbstractMockMvcTest {
   private FileController fileController;
   @MockBean
   private ClammitVirusScanner clammitVirusScanner;
+  @SpyBean
+  private FileValidationService fileValidationService;
   @Captor
   private ArgumentCaptor<UserFile> userFileArgumentCaptor;
   private final UUID fileId = UUID.randomUUID();
@@ -122,7 +125,8 @@ public void fileUploadEndpointHitsCloudFileRepositoryAndAddsUserFileToSession()
     // the "name" param has to match what the endpoint expects: "file"
     MockMultipartFile testImage = new MockMultipartFile("file", "someImage.jpg",
         MediaType.IMAGE_JPEG_VALUE, "test".getBytes());
-
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
+
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
             .param("flow", "testFlow")
@@ -163,6 +167,7 @@ void shouldShowFileContainsVirusErrorIfClammitScanFindsVirus() throws Exception
         "X5O!P%@AP[4\\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*".getBytes());
     when(clammitVirusScanner.virusDetected(testVirusFile)).thenReturn(true);
     when(submissionRepositoryService.findById(any())).thenReturn(Optional.of(submission));
+    when(fileValidationService.isAcceptedMimeType(testVirusFile)).thenReturn(true);
 
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testVirusFile)
@@ -186,7 +191,8 @@ void shouldAllowUploadIfBlockIfUnreachableIsSetToFalse() throws Exception {
     when(clammitVirusScanner.virusDetected(testImage)).thenThrow(
         new WebClientResponseException(500, "Failed!", null, null, null));
     when(submissionRepositoryService.findById(any())).thenReturn(Optional.of(submission));
-
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
+
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
             .param("flow", "testFlow")
@@ -208,7 +214,8 @@ void shouldSetFalseIfVirusScannerDidNotRun() throws Exception {
     when(clammitVirusScanner.virusDetected(testImage)).thenThrow(
         new WebClientResponseException(500, "Failed!", null, null, null));
     doNothing().when(cloudFileRepository).upload(any(), any());
-
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
+
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
             .param("flow", "testFlow")
@@ -227,6 +234,7 @@ void shouldSetFalseIfVirusScannerDidNotRun() throws Exception {
   void shouldReturn413IfUploadedFileViolatesMaxFileSizeConstraint() throws Exception {
     MockMultipartFile testImage = new MockMultipartFile("file", "testFileSizeImage.jpg",
         MediaType.IMAGE_JPEG_VALUE, new byte[(int) (FileUtils.ONE_MB + 1)]);
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
 
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
@@ -242,11 +250,15 @@ void shouldReturn413IfUploadedFileViolatesMaxFileSizeConstraint() throws Excepti
 
   @Test
   void shouldReturn4xxIfUploadFileViolatesMaxFilesConstraint() throws Exception {
+    MockMultipartFile testImage = new MockMultipartFile("file", "testFileSizeImage.jpg",
+            MediaType.IMAGE_JPEG_VALUE, new byte[10]);
+
     when(userFileRepositoryService.countBySubmission(submission)).thenReturn(10L);
     when(submissionRepositoryService.findById(any())).thenReturn(Optional.of(submission));
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
+
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
-            .file(new MockMultipartFile("file", "testFileSizeImage.jpg",
-                MediaType.IMAGE_JPEG_VALUE, new byte[10]))
+            .file(testImage)
             .param("flow", "testFlow")
             .param("inputName", "dropZoneTestInstance")
             .param("thumbDataURL", "base64string")

src/test/java/formflow/library/controllers/NoOpVirusScannerTest.java

@@ -11,6 +11,7 @@
 import formflow.library.data.SubmissionRepositoryService;
 import formflow.library.data.UserFile;
 import formflow.library.data.UserFileRepositoryService;
+import formflow.library.file.FileValidationService;
 import formflow.library.utilities.AbstractMockMvcTest;
 import java.util.HashMap;
 import java.util.Map;
@@ -21,6 +22,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.boot.test.mock.mockito.SpyBean;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockMultipartFile;
@@ -42,7 +44,9 @@ public class NoOpVirusScannerTest extends AbstractMockMvcTest {
   private UserFileRepositoryService userFileRepositoryService;
   @Autowired
   private FileController fileController;
-
+  @SpyBean
+  FileValidationService fileValidationService;
+
   @Override
   @BeforeEach
   public void setUp() throws Exception {
@@ -72,6 +76,7 @@ public void setUp() throws Exception {
   void shouldBypassRealVirusScanningIfDisabled() throws Exception {
     MockMultipartFile testImage = new MockMultipartFile("file", "someImage.jpg",
         MediaType.IMAGE_JPEG_VALUE, "test".getBytes());
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
             .param("flow", "testFlow")

src/test/java/formflow/library/controllers/ScreenControllerJourneyTest.java

@@ -1,15 +1,19 @@
 package formflow.library.controllers;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
 import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
 
+import formflow.library.file.FileValidationService;
 import formflow.library.utilities.AbstractBasePageTest;
 
 import java.io.IOException;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.SpyBean;
 
 @SpringBootTest(properties = {"form-flow.path=flows-config/test-flow.yaml"},
     webEnvironment = RANDOM_PORT)

src/test/java/formflow/library/controllers/UploadBlockedIfVirusScanUnreachableTest.java

@@ -10,6 +10,7 @@
 import formflow.library.data.SubmissionRepositoryService;
 import formflow.library.data.UserFileRepositoryService;
 import formflow.library.file.ClammitVirusScanner;
+import formflow.library.file.FileValidationService;
 import formflow.library.utilities.AbstractMockMvcTest;
 import java.util.Locale;
 import java.util.UUID;
@@ -33,22 +34,19 @@
 public class UploadBlockedIfVirusScanUnreachableTest extends AbstractMockMvcTest {
 
   private MockMvc mockMvc;
-  @MockBean
-  private SubmissionRepositoryService submissionRepositoryService;
   @Autowired
   UserFileRepositoryService userFileRepositoryService;
   @Autowired
   private FileController fileController;
   @MockBean
   private ClammitVirusScanner clammitVirusScanner;
+  @MockBean
+  FileValidationService fileValidationService;
 
   @Override
   @BeforeEach
   public void setUp() throws Exception {
-    UUID submissionUUID = UUID.randomUUID();
     mockMvc = MockMvcBuilders.standaloneSetup(fileController).build();
-    Submission submission = Submission.builder().id(submissionUUID).build();
-    //when(submissionRepositoryService.findOrCreate(any())).thenReturn(submission);
     super.setUp();
   }
 
@@ -58,6 +56,7 @@ void shouldPreventUploadAndShowAnErrorIfBlockIfUnreachableIsSetToTrue() throws E
         MediaType.IMAGE_JPEG_VALUE, "test".getBytes());
     when(clammitVirusScanner.virusDetected(testImage)).thenThrow(
         new WebClientResponseException(500, "Failed!", null, null, null));
+    when(fileValidationService.isAcceptedMimeType(testImage)).thenReturn(true);
     mockMvc.perform(MockMvcRequestBuilders.multipart("/file-upload")
             .file(testImage)
             .param("flow", "testFlow")

src/test/java/formflow/library/file/FileValidationServiceTest.java

@@ -2,14 +2,19 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.springframework.core.io.ClassPathResource;
 import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockMultipartFile;
+import org.springframework.web.multipart.MultipartFile;
 
 class FileValidationServiceTest {
 
@@ -29,23 +34,31 @@ void acceptedFileTypesShouldReturnTheIntersectionOfDefaultTypesWithUserProvidedO
 
   private static Stream<Arguments> provideMultiPartFiles() {
     return Stream.of(
-        Arguments.of("image/jpeg", true),
-        Arguments.of("fake/nonsense", false),
-        Arguments.of(null, false),
-        Arguments.of("", false)
+        Arguments.of("test.jpeg", true),
+        Arguments.of("test-archive.zip", false),
+        Arguments.of("i-am-not-a-png.txt.png", false)
     );
   }
 
   @ParameterizedTest
   @MethodSource("provideMultiPartFiles")
-  void isAcceptedMimeTypeReturnsTrueIfAccepted(String contentType, boolean assertion) {
+  void isAcceptedMimeTypeReturnsTrueIfAccepted(String contentName, boolean assertion) throws IOException {
     FileValidationService fileValidationService = new FileValidationService(".jpeg,.bmp", 1);
-    MockMultipartFile testFile = new MockMultipartFile("test", "test", contentType, new byte[]{});
+    ClassPathResource resource = new ClassPathResource(contentName);
+    MultipartFile file = new MockMultipartFile("file", contentName, Files.probeContentType(Path.of(contentName)), resource.getInputStream());
 
-    assertThat(fileValidationService.isAcceptedMimeType(testFile)).isEqualTo(assertion);
+    assertThat(fileValidationService.isAcceptedMimeType(file)).isEqualTo(assertion);
   }
 
 
+  @Test
+  void detectsFalseMimeTypes() throws IOException {
+    ClassPathResource resource = new ClassPathResource("i-am-not-a-png.txt.png");
+    MultipartFile file = new MockMultipartFile("file", "i-am-not-a-png.txt.png", MediaType.IMAGE_PNG_VALUE, resource.getInputStream());
+    FileValidationService fileValidationService = new FileValidationService(".jpeg,.bmp", 1);
+    assertThat(fileValidationService.isAcceptedMimeType(file)).isFalse();
+  }
+
   @Test
   void isTooLargeReturnsTrueIfSizeExceedsMax() {
     FileValidationService fileValidationService = new FileValidationService(".jpeg,.bmp", 1);

src/test/java/formflow/library/utilities/Page.java

@@ -3,12 +3,14 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 
+import java.time.Duration;
 import java.util.List;
 import java.util.stream.Collectors;
 import org.openqa.selenium.By;
 import org.openqa.selenium.By.ByTagName;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.remote.RemoteWebDriver;
+import org.openqa.selenium.support.ui.WebDriverWait;
 
 public class Page {
 
@@ -20,6 +22,8 @@ public Page(RemoteWebDriver driver) {
   }
 
   public String getTitle() {
+    WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
+    wait.until(webDriver -> !driver.getTitle().isBlank());
     return driver.getTitle();
   }