Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Subhobrata Dey <[email protected]>
  • Loading branch information
@sbcd90
sbcd90 committed Sep 26, 2024
1 parent 1f3eb29 commit c514a78
Show file tree
Hide file tree
Showing 10 changed files with 455 additions and 211 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/securityanalytics/config/monitors/DetectorMonitorConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class DetectorMonitorConfig {
public static final String OPENSEARCH_SAP_RULE_INDEX_TEMPLATE = ".opensearch-sap-detectors-queries-index-template";

public static String getRuleIndex(String logType) {
return String.format(Locale.getDefault(), ".opensearch-sap-%s-detectors-queries*", logType);
return String.format(Locale.getDefault(), ".opensearch-sap-%s-detectors-queries", logType);
}

public static String getRuleIndexOptimized(String logType) {
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1258,10 +1258,10 @@ void createDetector() {
request.getDetector().setFindingsIndexPattern(DetectorMonitorConfig.getFindingsIndexPattern(ruleTopic));

if (enableDetectorWithDedicatedQueryIndices) {
// disabling the setting after enabling it will mean delete & re-create the detector
request.getDetector().setRuleIndex(DetectorMonitorConfig.getRuleIndexOptimized(ruleTopic));
} else {
String ruleTopicIndex = DetectorMonitorConfig.getRuleIndex(ruleTopic);
request.getDetector().setRuleIndex(ruleTopicIndex.substring(0, ruleTopicIndex.length()-1));
request.getDetector().setRuleIndex(DetectorMonitorConfig.getRuleIndex(ruleTopic));
}

User originalContextUser = this.user;
Expand Down Expand Up @@ -1383,10 +1383,10 @@ void onGetResponse(Detector currentDetector, User user) {
request.getDetector().setRuleIndex(currentDetector.getRuleIndex());
} else {
if (enableDetectorWithDedicatedQueryIndices) {
// disabling the setting after enabling it will mean delete & re-create the detector
request.getDetector().setRuleIndex(DetectorMonitorConfig.getRuleIndexOptimized(ruleTopic));
} else {
String ruleTopicIndex = DetectorMonitorConfig.getRuleIndex(ruleTopic);
request.getDetector().setRuleIndex(ruleTopicIndex.substring(0, ruleTopicIndex.length() - 1));
request.getDetector().setRuleIndex(DetectorMonitorConfig.getRuleIndex(ruleTopic));
}
}
request.getDetector().setUser(user);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/securityanalytics/util/RuleTopicIndices.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ private void getAllRuleIndices(ActionListener<List<String>> listener) {
listener.onResponse(
logTypes
.stream()
.map(logType -> DetectorMonitorConfig.getRuleIndex(logType))
// use index pattern here to define rule topic index template for all query indices which match the pattern
.map(logType -> DetectorMonitorConfig.getRuleIndex(logType) + "*")
.collect(Collectors.toList())
);
}, listener::onFailure));
Expand Down
93 changes: 85 additions & 8 deletions src/test/java/org/opensearch/securityanalytics/DetectorThreatIntelIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,22 @@ public void testCreateDetectorWithThreatIntelEnabled_updateDetectorWithThreatInt
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), true, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(2, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -258,11 +269,22 @@ public void testCreateDetectorWithThreatIntelDisabled_updateDetectorWithThreatIn
Detector detector = randomDetectorWithInputsAndThreatIntel(List.of(input), false);
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -344,11 +366,22 @@ public void testCreateDetectorWithThreatIntelEnabledAndNoRules_triggerDetectionT
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), true, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -427,11 +460,22 @@ public void testCreateDetectorWithThreatIntelEnabled_triggerDetectionTypeOnlyThr
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), true, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -511,11 +555,22 @@ public void testCreateDetectorWithThreatIntelEnabled_triggerWithBothDetectionTyp
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), true, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -592,11 +647,22 @@ public void testCreateDetectorWithThreatIntelDisabled_triggerWithThreatIntelDete
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), false, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -673,11 +739,22 @@ public void testCreateDetectorWithThreatIntelDisabled_triggerWithRulesDetectionT
Detector detector = randomDetectorWithInputsAndThreatIntelAndTriggers(List.of(input), false, List.of(trigger));
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);


assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -725,4 +802,4 @@ public void testCreateDetectorWithThreatIntelDisabled_triggerWithRulesDetectionT
/** findings are present but alerts are NOT generated as detection type mentioned in trigger is threat_intel only but finding is from rules*/
Assert.assertEquals(3, getAlertsBody.get("total_alerts"));
}
}
}
33 changes: 22 additions & 11 deletions src/test/java/org/opensearch/securityanalytics/alerts/AlertsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ public void testGetAlertsByStartTimeAndEndTimeSuccess() throws IOException, Inte
}

public void testGetAlerts_noDetector_failure() throws IOException {
// Call GetAlerts API
// Call GetAlerts API
Map<String, String> params = new HashMap<>();
params.put("detector_id", "nonexistent_detector_id");
try {
Expand Down Expand Up @@ -821,14 +821,25 @@ public void testMultipleAggregationAndDocRules_alertSuccess() throws IOException
Collections.emptyList());
Detector detector = randomDetectorWithInputsAndTriggers(List.of(input),
List.of(new DetectorTrigger("randomtrigegr", "test-trigger", "1", List.of(randomDetectorType()), List.of(), List.of(), List.of(), List.of(), List.of()))
);
);

Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));


String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);

assertEquals(1, response.getHits().getTotalHits().value); // 5 for rules, 1 for match_all query in chained findings monitor

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);
String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -859,8 +870,8 @@ public void testMultipleAggregationAndDocRules_alertSuccess() throws IOException
}
}

assertEquals(1, numberOfMonitorTypes.get(Monitor.MonitorType.BUCKET_LEVEL_MONITOR.getValue()).intValue());
assertEquals(1, numberOfMonitorTypes.get(Monitor.MonitorType.DOC_LEVEL_MONITOR.getValue()).intValue());
assertEquals(1, numberOfMonitorTypes.get(Monitor.MonitorType.BUCKET_LEVEL_MONITOR.getValue()).intValue());
assertEquals(1, numberOfMonitorTypes.get(Monitor.MonitorType.DOC_LEVEL_MONITOR.getValue()).intValue());

Map<String, String> params = new HashMap<>();
params.put("detector_id", detectorId);
Expand All @@ -884,13 +895,13 @@ public void testMultipleAggregationAndDocRules_alertSuccess() throws IOException
List<Map<String, Object>> queries = (List<Map<String, Object>>) finding.get("queries");
Set<String> findingRuleIds = queries.stream().map(it -> it.get("id").toString()).collect(Collectors.toSet());

// In the case of bucket level monitors, queries will always contain one value
String aggRuleId = findingRuleIds.iterator().next();
List<String> findingDocs = (List<String>) finding.get("related_doc_ids");
// In the case of bucket level monitors, queries will always contain one value
String aggRuleId = findingRuleIds.iterator().next();
List<String> findingDocs = (List<String>) finding.get("related_doc_ids");

if (aggRuleId.equals(sumRuleId)) {
assertTrue(List.of("1", "2", "3", "4", "5", "6", "7").containsAll(findingDocs));
}
if (aggRuleId.equals(sumRuleId)) {
assertTrue(List.of("1", "2", "3", "4", "5", "6", "7").containsAll(findingDocs));
}
}

assertTrue(Arrays.asList("1", "2", "3", "4", "5", "6", "7", "8").containsAll(docLevelFinding));
Expand Down
14 changes: 12 additions & 2 deletions src/test/java/org/opensearch/securityanalytics/findings/FindingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1209,11 +1209,21 @@ public void testCreateDetectorWithNotCondition_verifyFindingsAndNoFindings_succe

Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));

String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);

assertEquals(1, response.getHits().getTotalHits().value);

assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Map<String, Object> responseBody = asMap(createResponse);

String detectorId = responseBody.get("_id").toString();
String request = "{\n" +
request = "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId + "\"\n" +
Expand Down Expand Up @@ -1251,7 +1261,7 @@ public void testCreateDetectorWithNotCondition_verifyFindingsAndNoFindings_succe
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getFindingsIndex(randomDetectorType()), request, true);
response = executeSearchAndGetResponse(DetectorMonitorConfig.getFindingsIndex(randomDetectorType()), request, true);

assertEquals(2, response.getHits().getTotalHits().value);

Expand Down
Loading

0 comments on commit c514a78

Please sign in to comment.