Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes detectorType incompatibility with detector rules #524

Merged
merged 8 commits into from
Sep 1, 2023
Merged

Fixes detectorType incompatibility with detector rules #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
89683e6
add error response
Aug 29, 2023
c8d0f76
add integration test
Aug 29, 2023
3123a5c
broader error msg and fixed integ test
jowg-amazon Aug 30, 2023
5b9bdbe
updated error msg
jowg-amazon Aug 30, 2023
e923df9
added as codeowner
jowg-amazon Aug 31, 2023
2e1e6e7
integ test checks error message
jowg-amazon Aug 31, 2023
09e6887
cfails if request goes through
jowg-amazon Aug 31, 2023
d95ed61
Remove from codeowners
jowg-amazon Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@
);
}, listener::onFailure);
} else {
// Do nothing if detector doesn't have any monitor
// Failure if detector doesn't have any monitor
if (monitorRequests.isEmpty()) {
listener.onResponse(Collections.emptyList());
listener.onFailure(new OpenSearchStatusException("Detector cannot be created as no compatible rules were provided", RestStatus.BAD_REQUEST));

Check warning on line 291 in src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java#L291 

Added line #L291 was not covered by tests
return;
}

Expand Down
51 changes: 31 additions & 20 deletions src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,29 +321,40 @@ public void testCreateDetectorWithoutRules() throws IOException {

Detector detector = randomDetector(Collections.emptyList());

Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
Assert.assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
try {
makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
fail("create detector call should have failed");
} catch (ResponseException ex) {
Assert.assertEquals(400, ex.getResponse().getStatusLine().getStatusCode());
assertTrue(ex.getMessage().contains("Detector cannot be created as no compatible rules were provided"));
}
}

Map<String, Object> responseBody = asMap(createResponse);
public void testCreateDetectorWithIncompatibleDetectorType() throws IOException {
String index = createTestIndex(randomIndex(), windowsIndexMapping());

// Verify rules
String request = "{\n" +
" \"query\" : {\n" +
" \"match_all\":{\n" +
" }\n" +
" }\n" +
"}";
SearchResponse response = executeSearchAndGetResponse(DetectorMonitorConfig.getRuleIndex(randomDetectorType()) + "*", request, true);
Assert.assertEquals(0, response.getHits().getTotalHits().value);
// Execute CreateMappingsAction to add alias mapping for index
Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
// both req params and req body are supported
createMappingRequest.setJsonEntity(
"{ \"index_name\":\"" + index + "\"," +
" \"rule_topic\":\"" + randomDetectorType() + "\", " +
" \"partial\":true" +
"}"
);

String createdId = responseBody.get("_id").toString();
int createdVersion = Integer.parseInt(responseBody.get("_version").toString());
Assert.assertNotEquals("response is missing Id", Detector.NO_ID, createdId);
Assert.assertTrue("incorrect version", createdVersion > 0);
Assert.assertEquals("Incorrect Location header", String.format(Locale.getDefault(), "%s/%s", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, createdId), createResponse.getHeader("Location"));
Assert.assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("rule_topic_index"));
Assert.assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("findings_index"));
Assert.assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("alert_index"));
Response createMappingResponse = client().performRequest(createMappingRequest);
assertEquals(HttpStatus.SC_OK, createMappingResponse.getStatusLine().getStatusCode());

Detector detector = randomDetector(getPrePackagedRules("ad_ldap"));

try {
makeRequest(client(), "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
fail("create detector call should have failed");
} catch (ResponseException ex) {
Assert.assertEquals(400, ex.getResponse().getStatusLine().getStatusCode());
assertTrue(ex.getMessage().contains("Detector cannot be created as no compatible rules were provided"));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: if the request doesn't fail the test will not recognize that.
can you ådd a fail() inside the try block to validate that behaviour.

}

public void testGettingADetector() throws IOException {
Expand Down
Loading