Skip to content

Commit

Permalink
Removed step listener once the detector is being created used for dum…
Browse files Browse the repository at this point in the history 
…my index search. Updated tests

Signed-off-by: Stevan Buzejic <[email protected]>
  • Loading branch information
@stevanbz
stevanbz committed Jan 5, 2023
1 parent d659585 commit 142982a
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 57 deletions.
41 changes: 23 additions & 18 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,22 @@ private void checkIndicesAndExecute(
) {
String [] detectorIndices = request.getDetector().getInputs().stream().flatMap(detectorInput -> detectorInput.getIndices().stream()).toArray(String[]::new);
SearchRequest searchRequest = new SearchRequest(detectorIndices).source(SearchSourceBuilder.searchSource().size(1).query(QueryBuilders.matchAllQuery()));;
StepListener<SearchResponse> checkIndexAccessStep = new StepListener();
client.search(searchRequest, checkIndexAccessStep);
AsyncIndexDetectorsAction asyncAction = new AsyncIndexDetectorsAction(user, task, request, listener);
// Check and execute as a step if the check was successful
checkIndexAccessStep.whenComplete(searchResponse -> asyncAction.start(), e -> {
if(e instanceof OpenSearchStatusException) {
listener.onFailure(SecurityAnalyticsException.wrap(
new OpenSearchStatusException(String.format(Locale.getDefault(), "User doesn't have read permissions for one or more configured index %s", detectorIndices), RestStatus.FORBIDDEN)
));
} else {
listener.onFailure(e);
client.search(searchRequest, new ActionListener<>() {
@Override
public void onResponse(SearchResponse searchResponse) {
AsyncIndexDetectorsAction asyncAction = new AsyncIndexDetectorsAction(user, task, request, listener);
asyncAction.start();
}

@Override
public void onFailure(Exception e) {
if (e instanceof OpenSearchStatusException) {
listener.onFailure(SecurityAnalyticsException.wrap(
new OpenSearchStatusException(String.format(Locale.getDefault(), "User doesn't have read permissions for one or more configured index %s", detectorIndices), RestStatus.FORBIDDEN)
));
} else {
listener.onFailure(e);
}
}
});
}
Expand All @@ -214,7 +219,7 @@ private void createMonitorFromQueries(String index, List<Pair<String, Rule>> rul
monitorRequests.addAll(buildBucketLevelMonitorRequests(Pair.of(index, bucketLevelRules), detector, refreshPolicy, Monitor.NO_ID, Method.POST));
}
// Do nothing if detector doesn't have any monitor
if(monitorRequests.isEmpty()){
if (monitorRequests.isEmpty()){
listener.onResponse(Collections.emptyList());
return;
}
Expand All @@ -228,7 +233,7 @@ private void createMonitorFromQueries(String index, List<Pair<String, Rule>> rul
addFirstMonitorStep.whenComplete(addedFirstMonitorResponse -> {
monitorResponses.add(addedFirstMonitorResponse);
int numberOfUnprocessedResponses = monitorRequests.size() - 1;
if(numberOfUnprocessedResponses == 0){
if (numberOfUnprocessedResponses == 0){
listener.onResponse(monitorResponses);
} else {
GroupedActionListener<IndexMonitorResponse> monitorResponseListener = new GroupedActionListener(
Expand Down Expand Up @@ -273,7 +278,7 @@ private void updateMonitorFromQueries(String index, List<Pair<String, Rule>> rul

for (Pair<String, Rule> query: bucketLevelRules) {
Rule rule = query.getRight();
if(rule.getAggregationQueries() != null){
if (rule.getAggregationQueries() != null){
// Detect if the monitor should be added or updated
if (monitorPerRule.containsKey(rule.getId())) {
String monitorId = monitorPerRule.get(rule.getId());
Expand Down Expand Up @@ -343,15 +348,15 @@ private void updateAlertingMonitors(
executeMonitorActionRequest(monitorsToBeAdded, addNewMonitorsStep);
// 1. Add new alerting monitors (for the rules that didn't exist previously)
addNewMonitorsStep.whenComplete(addNewMonitorsResponse -> {
if(addNewMonitorsResponse != null && !addNewMonitorsResponse.isEmpty()) {
if (addNewMonitorsResponse != null && !addNewMonitorsResponse.isEmpty()) {
updatedMonitors.addAll(addNewMonitorsResponse);
}

StepListener<List<IndexMonitorResponse>> updateMonitorsStep = new StepListener<>();
executeMonitorActionRequest(monitorsToBeUpdated, updateMonitorsStep);
// 2. Update existing alerting monitors (based on the common rules)
updateMonitorsStep.whenComplete(updateMonitorResponse -> {
if(updateMonitorResponse!=null && !updateMonitorResponse.isEmpty()) {
if (updateMonitorResponse!=null && !updateMonitorResponse.isEmpty()) {
updatedMonitors.addAll(updateMonitorResponse);
}

Expand Down Expand Up @@ -435,7 +440,7 @@ private List<IndexMonitorRequest> buildBucketLevelMonitorRequests(Pair<String, L
Rule rule = query.getRight();

// Creating bucket level monitor per each aggregation rule
if(rule.getAggregationQueries() != null){
if (rule.getAggregationQueries() != null){
monitorRequests.add(createBucketLevelMonitorRequest(
query.getRight(),
logIndexToQueries.getLeft(),
Expand Down Expand Up @@ -512,7 +517,7 @@ public void executeMonitorActionRequest(
ActionListener<List<IndexMonitorResponse>> listener) {

// In the case of not provided monitors, just return empty list
if(indexMonitors == null || indexMonitors.isEmpty()) {
if (indexMonitors == null || indexMonitors.isEmpty()) {
listener.onResponse(new ArrayList<>());
return;
}
Expand Down
133 changes: 94 additions & 39 deletions src/test/java/org/opensearch/securityanalytics/resthandler/SecureDetectorRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,52 @@ public void testCreateDetectorWithNoBackendRoles() throws IOException {
}
}

/**
* 1. Tries to create a detector as a user without access to the given index pattern and verifies that it is forbidden
* 2. Creates a detector as a user with access to a given index
* @throws IOException
*/
public void testCreateDetectorIndexAccess() throws IOException {
public void testCreateDetector_userHasIndexAccess_success() throws IOException {
String[] backendRoles = { TEST_IT_BACKEND_ROLE };
String userWithAccess = "user1";
String roleNameWithIndexPatternAccess = "test-role-1";
String windowsIndexPattern = "windows*";
createUserWithDataAndCustomRole(userWithAccess, userWithAccess, roleNameWithIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(windowsIndexPattern));
RestClient clientWithAccess = null;

try {
clientWithAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithAccess, userWithAccess).setSocketTimeout(60000).build();
String index = createTestIndex(client(), randomIndex(), windowsIndexMapping(), Settings.EMPTY);

Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
createMappingRequest.setJsonEntity(
"{ \"index_name\":\"" + index + "\"," +
" \"rule_topic\":\"" + randomDetectorType() + "\", " +
" \"partial\":true" +
"}"
);
Response response = clientWithAccess.performRequest(createMappingRequest);
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());

Detector detector = randomDetector(getRandomPrePackagedRules());

Response createResponse = makeRequest(clientWithAccess, "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));

Map<String, Object> responseBody = asMap(createResponse);

String createdId = responseBody.get("_id").toString();
int createdVersion = Integer.parseInt(responseBody.get("_version").toString());

assertNotEquals("response is missing Id", Detector.NO_ID, createdId);
assertTrue("incorrect version", createdVersion > 0);
assertEquals("Incorrect Location header", String.format(Locale.getDefault(), "%s/%s", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, createdId), createResponse.getHeader("Location"));
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("rule_topic_index"));
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("findings_index"));
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("alert_index"));
} finally {
if (clientWithAccess != null) clientWithAccess.close();
deleteUser(userWithAccess);
tryDeletingRole(roleNameWithIndexPatternAccess);
}
}

public void testCreateDetector_userDoesntHaveIndexAccess_failure() throws IOException {
String[] backendRoles = { TEST_IT_BACKEND_ROLE };

String userWithoutAccess = "user";
Expand All @@ -223,15 +263,9 @@ public void testCreateDetectorIndexAccess() throws IOException {
createUserWithDataAndCustomRole(userWithoutAccess, userWithoutAccess, roleNameWithoutIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(testIndexPattern));
RestClient clientWithoutAccess = null;

String userWithAccess = "user1";
String roleNameWithIndexPatternAccess = "test-role-1";
String windowsIndexPattern = "windows*";
createUserWithDataAndCustomRole(userWithAccess, userWithAccess, roleNameWithIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(windowsIndexPattern));
RestClient clientWithAccess = null;
try {
clientWithoutAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithoutAccess, userWithoutAccess).setSocketTimeout(60000).build();
clientWithAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithAccess, userWithAccess).setSocketTimeout(60000).build();
//createUserRolesMapping("alerting_full_access", users);

String index = createTestIndex(client(), randomIndex(), windowsIndexMapping(), Settings.EMPTY);

// Execute CreateMappingsAction to add alias mapping for index
Expand All @@ -253,6 +287,39 @@ public void testCreateDetectorIndexAccess() throws IOException {
} catch (ResponseException e) {
assertEquals("Create detector error status", RestStatus.FORBIDDEN, restStatus(e.getResponse()));
}
} finally {
if (clientWithoutAccess!= null) clientWithoutAccess.close();
deleteUser(userWithoutAccess);
tryDeletingRole(roleNameWithoutIndexPatternAccess);
}
}

public void testUpdateDetector_userHasIndexAccess_success() throws IOException {
String[] backendRoles = { TEST_IT_BACKEND_ROLE };

String userWithAccess = "user1";
String roleNameWithIndexPatternAccess = "test-role-1";
String windowsIndexPattern = "windows*";
createUserWithDataAndCustomRole(userWithAccess, userWithAccess, roleNameWithIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(windowsIndexPattern));
RestClient clientWithAccess = null;
try {
clientWithAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithAccess, userWithAccess).setSocketTimeout(60000).build();
//createUserRolesMapping("alerting_full_access", users);
String index = createTestIndex(client(), randomIndex(), windowsIndexMapping(), Settings.EMPTY);

// 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" +
"}"
);
Response response = clientWithAccess.performRequest(createMappingRequest);
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());

Detector detector = randomDetector(getRandomPrePackagedRules());

Response createResponse = makeRequest(clientWithAccess, "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));
Expand All @@ -268,23 +335,18 @@ public void testCreateDetectorIndexAccess() throws IOException {
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("rule_topic_index"));
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("findings_index"));
assertFalse(((Map<String, Object>) responseBody.get("detector")).containsKey("alert_index"));
} finally {
if(clientWithoutAccess!= null) clientWithoutAccess.close();
deleteUser(userWithoutAccess);
tryDeletingRole(roleNameWithoutIndexPatternAccess);

String detectorId = responseBody.get("_id").toString();
Response updateResponse = makeRequest(clientWithAccess, "PUT", SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/" + detectorId, Collections.emptyMap(), toHttpEntity(detector));
assertEquals("Update detector failed", RestStatus.OK, restStatus(updateResponse));
} finally {
if (clientWithAccess != null) clientWithAccess.close();
deleteUser(userWithAccess);
tryDeletingRole(roleNameWithIndexPatternAccess);
}
}

/**
* 1. Tries to update a detector as a user without access to the given index pattern and verifies that it is forbidden
* 2. Updates a detector as a user with access to a given index
* @throws IOException
*/
public void testUpdateDetectorIndexAccess() throws IOException {
public void testUpdateDetector_userDoesntHaveIndexAccess_failure() throws IOException {
String[] backendRoles = { TEST_IT_BACKEND_ROLE };

String userWithoutAccess = "user";
Expand All @@ -293,17 +355,16 @@ public void testUpdateDetectorIndexAccess() throws IOException {
createUserWithDataAndCustomRole(userWithoutAccess, userWithoutAccess, roleNameWithoutIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(testIndexPattern));
RestClient clientWithoutAccess = null;


String userWithAccess = "user1";
String roleNameWithIndexPatternAccess = "test-role-1";
String windowsIndexPattern = "windows*";
createUserWithDataAndCustomRole(userWithAccess, userWithAccess, roleNameWithIndexPatternAccess, backendRoles, clusterPermissions, indexPermissions, List.of(windowsIndexPattern));
RestClient clientWithAccess = null;
try {
clientWithoutAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithoutAccess, userWithoutAccess).setSocketTimeout(60000).build();
clientWithAccess = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[]{}), isHttps(), userWithAccess, userWithAccess).setSocketTimeout(60000).build();

//createUserRolesMapping("alerting_full_access", users);
String index = createTestIndex(client(), randomIndex(), windowsIndexMapping(), Settings.EMPTY);
// Assign a role to the index
createIndexRole(TEST_HR_ROLE, Collections.emptyList(), indexPermissions, List.of(index));
String[] users = {user};
// Assign a role to existing user
createUserRolesMapping(TEST_HR_ROLE, users);

// Execute CreateMappingsAction to add alias mapping for index
Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
Expand All @@ -314,12 +375,12 @@ public void testUpdateDetectorIndexAccess() throws IOException {
" \"partial\":true" +
"}"
);
Response response = clientWithAccess.performRequest(createMappingRequest);
Response response = userClient.performRequest(createMappingRequest);
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());

Detector detector = randomDetector(getRandomPrePackagedRules());

Response createResponse = makeRequest(clientWithAccess, "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
Response createResponse = makeRequest(userClient, "POST", SecurityAnalyticsPlugin.DETECTOR_BASE_URI, Collections.emptyMap(), toHttpEntity(detector));
assertEquals("Create detector failed", RestStatus.CREATED, restStatus(createResponse));

Map<String, Object> responseBody = asMap(createResponse);
Expand All @@ -341,17 +402,11 @@ public void testUpdateDetectorIndexAccess() throws IOException {
} catch (ResponseException e) {
assertEquals("Update detector error status", RestStatus.FORBIDDEN, restStatus(e.getResponse()));
}

Response updateResponse = makeRequest(clientWithAccess, "PUT", SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/" + detectorId, Collections.emptyMap(), toHttpEntity(detector));
assertEquals("Update detector failed", RestStatus.OK, restStatus(updateResponse));
} finally {
if (clientWithoutAccess != null) clientWithoutAccess.close();
deleteUser(userWithoutAccess);
tryDeletingRole(roleNameWithoutIndexPatternAccess);

if (clientWithAccess != null) clientWithAccess.close();
deleteUser(userWithAccess);
tryDeletingRole(roleNameWithIndexPatternAccess);
tryDeletingRole(TEST_HR_ROLE);
}
}
}

0 comments on commit 142982a

Please sign in to comment.