[Backport 2.x] Fix duplicate ecs mappings which returns incorrect log index field in mapping view API (#786) #889

src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java

@@ -8,7 +8,6 @@
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.admin.indices.get.GetIndexRequest;
 import org.opensearch.action.admin.indices.get.GetIndexResponse;
 import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
@@ -485,13 +484,16 @@
                             String rawPath = requiredField.getRawField();
                             String ocsfPath = requiredField.getOcsf();
                             if (allFieldsFromIndex.contains(rawPath)) {
-                                if (alias != null) {
-                                    // Maintain list of found paths in index
-                                    applyableAliases.add(alias);
-                                } else {
-                                    applyableAliases.add(rawPath);
+                                // if the alias was already added into applyable aliases, then skip to avoid duplicates
+                                if (!applyableAliases.contains(alias) && !applyableAliases.contains(rawPath)) {
+                                    if (alias != null) {
+                                        // Maintain list of found paths in index
+                                        applyableAliases.add(alias);
+                                    } else {
+                                        applyableAliases.add(rawPath);
+                                    }
+                                    pathsOfApplyableAliases.add(rawPath);
                                 }
-                                pathsOfApplyableAliases.add(rawPath);
                             } else if (allFieldsFromIndex.contains(ocsfPath)) {
                                 applyableAliases.add(alias);
                                 pathsOfApplyableAliases.add(ocsfPath);
@@ -505,13 +507,21 @@
                             }
                         }
 
+                        // turn unmappedFieldAliases into a set to remove duplicates
+                        Set<String> setOfUnmappedFieldAliases = new HashSet<>(unmappedFieldAliases);
+
+                        // filter out aliases that were included in applyableAliases already
+                        List<String> filteredUnmappedFieldAliases = setOfUnmappedFieldAliases.stream()
+                                .filter(e -> false == applyableAliases.contains(e))
+                                .collect(Collectors.toList());
+
                         Map<String, Map<String, String>> aliasMappingFields = new HashMap<>();
                         XContentBuilder aliasMappingsObj = XContentFactory.jsonBuilder().startObject();
                         for (LogType.Mapping mapping : requiredFields) {
                             if (allFieldsFromIndex.contains(mapping.getOcsf())) {
                                 aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getOcsf()));
                             } else if (mapping.getEcs() != null) {
-                                aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+                                shouldUpdateEcsMappingAndMaybeUpdates(mapping, aliasMappingFields, pathsOfApplyableAliases);
                             } else if (mapping.getEcs() == null) {
                                 aliasMappingFields.put(mapping.getRawField(), Map.of("type", "alias", "path", mapping.getRawField()));
                             }
@@ -527,7 +537,7 @@
                                 .filter(e -> pathsOfApplyableAliases.contains(e) == false)
                                 .collect(Collectors.toList());
                         actionListener.onResponse(
-                                new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, unmappedFieldAliases, logTypeService.getIocFieldsList(logType))
+                                new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, filteredUnmappedFieldAliases, logTypeService.getIocFieldsList(logType))
                         );
                     } catch (Exception e) {
                         actionListener.onFailure(e);
@@ -542,6 +552,26 @@
         });
     }
 
+    /**
+     * Only updates the alias mapping fields if the ecs key has not been mapped yet
+     * or if pathOfApplyableAliases contains the raw field
+     *
+     * @param mapping
+     * @param aliasMappingFields
+     * @param pathsOfApplyableAliases
+     */
+    private static void shouldUpdateEcsMappingAndMaybeUpdates(LogType.Mapping mapping, Map<String, Map<String, String>> aliasMappingFields, List<String> pathsOfApplyableAliases) {
+        // check if aliasMappingFields already contains a key
+        if (aliasMappingFields.containsKey(mapping.getEcs())) {
+            // if the pathOfApplyableAliases contains the raw field, then override the existing map
+            if (pathsOfApplyableAliases.contains(mapping.getRawField())) {
+                aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+            }
+        } else {
+            aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+        }
+    }
+
     /**
      * Given index name, resolves it to single concrete index, depending on what initial <code>indexName</code> is.
      * In case of Datastream or Alias, WriteIndex would be returned. In case of index pattern, newest index by creation date would be returned.

src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java

@@ -394,6 +394,114 @@ public void testGetMappingsViewLinuxSuccess() throws IOException {
         assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
     }
 
+    // Tests mappings where multiple raw fields correspond to one ecs value
+    public void testGetMappingsViewWindowsSuccess() throws IOException {
+
+        String testIndexName = "get_mappings_view_index";
+
+        createSampleWindex(testIndexName);
+
+        // Execute GetMappingsViewAction to add alias mapping for index
+        Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
+        // both req params and req body are supported
+        request.addParameter("index_name", testIndexName);
+        request.addParameter("rule_topic", "windows");
+        Response response = client().performRequest(request);
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+        Map<String, Object> respMap = responseAsMap(response);
+
+        // Verify alias mappings
+        Map<String, Object> props = (Map<String, Object>) respMap.get("properties");
+        assertEquals(3, props.size());
+        assertTrue(props.containsKey("winlog.event_data.LogonType"));
+        assertTrue(props.containsKey("winlog.provider_name"));
+        assertTrue(props.containsKey("host.hostname"));
+
+        // Verify unmapped index fields
+        List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
+        assertEquals(3, unmappedIndexFields.size());
+        assert(unmappedIndexFields.contains("plain1"));
+        assert(unmappedIndexFields.contains("ParentUser.first"));
+        assert(unmappedIndexFields.contains("ParentUser.last"));
+
+        // Verify unmapped field aliases
+        List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
+        assertEquals(191, filteredUnmappedFieldAliases.size());
+        assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
+        assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
+        assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
+        List<HashMap<String, Object>> iocFieldsList = (List<HashMap<String, Object>>) respMap.get(GetMappingsViewResponse.THREAT_INTEL_FIELD_ALIASES);
+        assertEquals(iocFieldsList.size(), 1);
+
+        // Index a doc for a field with multiple raw fields corresponding to one ecs field
+        indexDoc(testIndexName, "1", "{ \"EventID\": 1 }");
+        // Execute GetMappingsViewAction to add alias mapping for index
+        request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
+        // both req params and req body are supported
+        request.addParameter("index_name", testIndexName);
+        request.addParameter("rule_topic", "windows");
+        response = client().performRequest(request);
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+        respMap = responseAsMap(response);
+
+        // Verify alias mappings
+        props = (Map<String, Object>) respMap.get("properties");
+        assertEquals(4, props.size());
+        assertTrue(props.containsKey("winlog.event_id"));
+
+        // verify unmapped index fields
+        unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
+        assertEquals(3, unmappedIndexFields.size());
+
+        // verify unmapped field aliases
+        filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
+        assertEquals(190, filteredUnmappedFieldAliases.size());
+        assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
+    }
+
+    // Tests mappings where multiple raw fields correspond to one ecs value and all fields are present in the index
+    public void testGetMappingsViewMulitpleRawFieldsSuccess() throws IOException {
+
+        String testIndexName = "get_mappings_view_index";
+
+        createSampleWindex(testIndexName);
+        String sampleDoc = "{" +
+                "  \"EventID\": 1," +
+                "  \"EventId\": 2," +
+                "  \"event_uid\": 3" +
+                "}";
+        indexDoc(testIndexName, "1", sampleDoc);
+
+        // Execute GetMappingsViewAction to add alias mapping for index
+        Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
+        // both req params and req body are supported
+        request.addParameter("index_name", testIndexName);
+        request.addParameter("rule_topic", "windows");
+        Response response = client().performRequest(request);
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+        Map<String, Object> respMap = responseAsMap(response);
+
+        // Verify alias mappings
+        Map<String, Object> props = (Map<String, Object>) respMap.get("properties");
+        assertEquals(4, props.size());
+        assertTrue(props.containsKey("winlog.event_data.LogonType"));
+        assertTrue(props.containsKey("winlog.provider_name"));
+        assertTrue(props.containsKey("host.hostname"));
+        assertTrue(props.containsKey("winlog.event_id"));
+
+        // Verify unmapped index fields
+        List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
+        assertEquals(5, unmappedIndexFields.size());
+
+        // Verify unmapped field aliases
+        List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
+        assertEquals(190, filteredUnmappedFieldAliases.size());
+        assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
+        assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
+        assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
+        assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
+    }
+
     public void testCreateMappings_withDatastream_success() throws IOException {
         String datastream = "test_datastream";
 
@@ -1277,6 +1385,69 @@ private void createSampleIndex(String indexName, Settings settings, String alias
         assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
     }
 
+    private void createSampleWindex(String indexName) throws IOException {
+        createSampleWindex(indexName, Settings.EMPTY, null);
+    }
+
+    private void createSampleWindex(String indexName, Settings settings, String aliases) throws IOException {
+        String indexMapping =
+                "    \"properties\": {" +
+                        "        \"LogonType\": {" +
+                        "          \"type\": \"integer\"" +
+                        "        }," +
+                        "        \"Provider\": {" +
+                        "          \"type\": \"text\"" +
+                        "        }," +
+                        "        \"hostname\": {" +
+                        "          \"type\": \"text\"" +
+                        "        }," +
+                        "        \"plain1\": {" +
+                        "          \"type\": \"integer\"" +
+                        "        }," +
+                        "        \"ParentUser\":{" +
+                        "          \"type\":\"nested\"," +
+                        "            \"properties\":{" +
+                        "              \"first\":{" +
+                        "                \"type\":\"text\"," +
+                        "                  \"fields\":{" +
+                        "                    \"keyword\":{" +
+                        "                      \"type\":\"keyword\"," +
+                        "                      \"ignore_above\":256" +
+                        "}" +
+                        "}" +
+                        "}," +
+                        "              \"last\":{" +
+                        "\"type\":\"text\"," +
+                        "\"fields\":{" +
+                        "                      \"keyword\":{" +
+                        "                           \"type\":\"keyword\"," +
+                        "                           \"ignore_above\":256" +
+                        "}" +
+                        "}" +
+                        "}" +
+                        "}" +
+                        "}" +
+                        "    }";
+
+        createIndex(indexName, settings, indexMapping, aliases);
+
+        // Insert sample doc with event_uid not explicitly mapped
+        String sampleDoc = "{" +
+                "  \"LogonType\":1," +
+                "  \"Provider\":\"Microsoft-Windows-Security-Auditing\"," +
+                "  \"hostname\":\"FLUXCAPACITOR\"" +
+                "}";
+
+        // Index doc
+        Request indexRequest = new Request("POST", indexName + "/_doc?refresh=wait_for");
+        indexRequest.setJsonEntity(sampleDoc);
+        Response response = client().performRequest(indexRequest);
+        assertEquals(HttpStatus.SC_CREATED, response.getStatusLine().getStatusCode());
+        // Refresh everything
+        response = client().performRequest(new Request("POST", "_refresh"));
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+    }
+
     private void createSampleDatastream(String datastreamName) throws IOException {
         String indexMapping =
                 "    \"properties\": {" +

src/test/java/org/opensearch/securityanalytics/resthandler/OCSFDetectorRestApiIT.java

@@ -436,7 +436,7 @@ public void testOCSFCloudtrailGetMappingsViewApi() throws IOException {
         assertEquals(20, unmappedIndexFields.size());
         // Verify unmapped field aliases
         List<String> unmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
-        assertEquals(25, unmappedFieldAliases.size());
+        assertEquals(24, unmappedFieldAliases.size());
     }
 
     @SuppressWarnings("unchecked")
@@ -502,7 +502,7 @@ public void testRawCloudtrailGetMappingsViewApi() throws IOException {
         assertEquals(17, unmappedIndexFields.size());
         // Verify unmapped field aliases
         List<String> unmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
-        assertEquals(26, unmappedFieldAliases.size());
+        assertEquals(25, unmappedFieldAliases.size());
     }
 
     @SuppressWarnings("unchecked")