CSV File adapter: Use AllowedAuxiliaryPathChecker in File path UI val…

…idation (#10391) (#10425)

* Add path check

* Return after aux file check

* Return after aux file check

* Add tests for config validation

* Provide more appropriate error message

graylog2-server/src/main/java/org/graylog2/lookup/adapters/CSVFileDataAdapter.java

@@ -62,7 +62,16 @@ public class CSVFileDataAdapter extends LookupDataAdapter {
     private static final Logger LOG = LoggerFactory.getLogger(CSVFileDataAdapter.class);
 
     public static final String NAME = "csvfile";
-    public static final String ALLOWED_PATH_ERROR = "The specified CSV file is not in an allowed path.";
+
+    /**
+     * If the AllowedAuxiliaryPathChecker is enabled (one or more paths provided to the allowed_auxiliary_paths server
+     * configuration property), then this error path will also be triggered for cases where the file does not exist.
+     * This is unavoidable, since the AllowedAuxiliaryPathChecker tries to resolve symbolic links and relative paths,
+     * which cannot be done if the file does not exist. Therefore this error message also indicates the possibility
+     * that the file does not exist.
+     */
+    public static final String ALLOWED_PATH_ERROR =
+            "The specified CSV file either does not exist or is not in an allowed path.";
 
     private final Config config;
     private final AllowedAuxiliaryPathChecker pathChecker;
@@ -297,10 +306,18 @@ public static Builder builder() {
         }
 
         @Override
-        public Optional<Multimap<String, String>> validate() {
+        public Optional<Multimap<String, String>> validate(LookupDataAdapterValidationContext context) {
             final ArrayListMultimap<String, String> errors = ArrayListMultimap.create();
 
             final Path path = Paths.get(path());
+            if (!context.getPathChecker().fileIsInAllowedPath(path)) {
+                errors.put("path", ALLOWED_PATH_ERROR);
+
+                // Intentionally return here, because in the Cloud context, we should not perform the following checks
+                // to report to the user whether or not a file exists.
+                return Optional.of(errors);
+            }
+
             if (!Files.exists(path)) {
                 errors.put("path", "The file does not exist.");
             } else if (!Files.isReadable(path)) {

graylog2-server/src/main/java/org/graylog2/lookup/adapters/LookupDataAdapterValidationContext.java

@@ -17,20 +17,28 @@
 package org.graylog2.lookup.adapters;
 
 import com.google.inject.Inject;
+import org.graylog2.lookup.AllowedAuxiliaryPathChecker;
 import org.graylog2.system.urlwhitelist.UrlWhitelistService;
 
 /**
  * Context object for configurations which require access to services to perform validation.
  */
 public class LookupDataAdapterValidationContext {
     private final UrlWhitelistService urlWhitelistService;
+    private final AllowedAuxiliaryPathChecker pathChecker;
 
     @Inject
-    public LookupDataAdapterValidationContext(UrlWhitelistService urlWhitelistService) {
+    public LookupDataAdapterValidationContext(UrlWhitelistService urlWhitelistService,
+                                              AllowedAuxiliaryPathChecker pathChecker) {
         this.urlWhitelistService = urlWhitelistService;
+        this.pathChecker = pathChecker;
     }
 
     public UrlWhitelistService getUrlWhitelistService() {
         return urlWhitelistService;
     }
+
+    public AllowedAuxiliaryPathChecker getPathChecker() {
+        return pathChecker;
+    }
 }

graylog2-server/src/test/java/org/graylog2/lookup/adapters/CSVFileDataAdapterTest.java

@@ -17,6 +17,7 @@
 package org.graylog2.lookup.adapters;
 
 import com.codahale.metrics.MetricRegistry;
+import com.google.common.collect.Multimap;
 import com.google.common.io.Resources;
 import org.graylog2.lookup.AllowedAuxiliaryPathChecker;
 import org.graylog2.plugin.lookup.LookupCachePurge;
@@ -30,13 +31,16 @@
 import java.net.URL;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Optional;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.graylog2.lookup.adapters.CSVFileDataAdapter.Config;
 import static org.graylog2.lookup.adapters.CSVFileDataAdapter.NAME;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
@@ -52,7 +56,10 @@ public class CSVFileDataAdapterTest {
     AllowedAuxiliaryPathChecker pathChecker;
 
     @Mock
-    LookupCachePurge cachePurge;
+    private LookupCachePurge cachePurge;
+
+    @Mock
+    private LookupDataAdapterValidationContext validationContext;
 
     public CSVFileDataAdapterTest() throws Exception {
         final URL resource = Resources.getResource("org/graylog2/lookup/adapters/CSVFileDataAdapterTest.csv");
@@ -100,7 +107,7 @@ public void doGet_failure_filePathInvalid() throws Exception {
         csvFileDataAdapter = new CSVFileDataAdapter("id", "name", config, new MetricRegistry(), pathChecker);
         assertThatThrownBy(() -> csvFileDataAdapter.doStart())
                 .isExactlyInstanceOf(IllegalStateException.class)
-                .hasMessageStartingWith("The specified CSV file is not in an allowed path.");
+                .hasMessageStartingWith(CSVFileDataAdapter.ALLOWED_PATH_ERROR);
     }
 
     @Test
@@ -123,6 +130,63 @@ public void refresh_failure_disallowedFileLocation() throws Exception {
         assertTrue(csvFileDataAdapter.getError().isPresent());
     }
 
+    @Test
+    public void testConfigValidationSuccess() {
+        final Config config = Config.builder()
+                                    .type(NAME)
+                                    .path(csvFile.toString())
+                                    .separator(",")
+                                    .quotechar("\"")
+                                    .keyColumn("key")
+                                    .valueColumn("value")
+                                    .checkInterval(60)
+                                    .caseInsensitiveLookup(false)
+                                    .build();
+        when(validationContext.getPathChecker()).thenReturn(pathChecker);
+        when(pathChecker.fileIsInAllowedPath(any(Path.class))).thenReturn(true);
+        final Optional<Multimap<String, String>> result = config.validate(validationContext);
+        assertFalse(result.isPresent());
+    }
+
+    @Test
+    public void testConfigValidationFileDoesNotExist() {
+        final Config config = Config.builder()
+                                    .type(NAME)
+                                    .path("fake-path")
+                                    .separator(",")
+                                    .quotechar("\"")
+                                    .keyColumn("key")
+                                    .valueColumn("value")
+                                    .checkInterval(60)
+                                    .caseInsensitiveLookup(false)
+                                    .build();
+        when(validationContext.getPathChecker()).thenReturn(pathChecker);
+        when(pathChecker.fileIsInAllowedPath(any(Path.class))).thenReturn(true);
+        final Optional<Multimap<String, String>> result = config.validate(validationContext);
+        assertTrue(result.isPresent());
+        assertEquals("The file does not exist.", String.join("", result.get().asMap().get("path")));
+    }
+
+    @Test
+    public void testConfigValidationFileNotInPermittedLocation() {
+        final Config config = Config.builder()
+                                    .type(NAME)
+                                    .path("fake-path")
+                                    .separator(",")
+                                    .quotechar("\"")
+                                    .keyColumn("key")
+                                    .valueColumn("value")
+                                    .checkInterval(60)
+                                    .caseInsensitiveLookup(false)
+                                    .build();
+        when(validationContext.getPathChecker()).thenReturn(pathChecker);
+        when(pathChecker.fileIsInAllowedPath(any(Path.class))).thenReturn(false);
+        final Optional<Multimap<String, String>> result = config.validate(validationContext);
+        assertTrue(result.isPresent());
+        assertEquals(CSVFileDataAdapter.ALLOWED_PATH_ERROR,
+                     String.join("", result.get().asMap().get("path")));
+    }
+
     private Config baseConfig() {
         return Config.builder()
                      .type(NAME)