diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java index c0efcfc1d..9c6774ac5 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java @@ -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 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); + } } }); } @@ -214,7 +219,7 @@ private void createMonitorFromQueries(String index, List> 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; } @@ -228,7 +233,7 @@ private void createMonitorFromQueries(String index, List> rul addFirstMonitorStep.whenComplete(addedFirstMonitorResponse -> { monitorResponses.add(addedFirstMonitorResponse); int numberOfUnprocessedResponses = monitorRequests.size() - 1; - if(numberOfUnprocessedResponses == 0){ + if (numberOfUnprocessedResponses == 0){ listener.onResponse(monitorResponses); } else { GroupedActionListener monitorResponseListener = new GroupedActionListener( @@ -273,7 +278,7 @@ private void updateMonitorFromQueries(String index, List> rul for (Pair 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()); @@ -343,7 +348,7 @@ 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); } @@ -351,7 +356,7 @@ private void updateAlertingMonitors( 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); } @@ -435,7 +440,7 @@ private List buildBucketLevelMonitorRequests(Pair> 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; } diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/SecureDetectorRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/SecureDetectorRestApiIT.java index db28888b4..e9ce3b9d9 100644 --- a/src/test/java/org/opensearch/securityanalytics/resthandler/SecureDetectorRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/SecureDetectorRestApiIT.java @@ -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 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) responseBody.get("detector")).containsKey("rule_topic_index")); + assertFalse(((Map) responseBody.get("detector")).containsKey("findings_index")); + assertFalse(((Map) 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"; @@ -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 @@ -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)); @@ -268,23 +335,18 @@ public void testCreateDetectorIndexAccess() throws IOException { assertFalse(((Map) responseBody.get("detector")).containsKey("rule_topic_index")); assertFalse(((Map) responseBody.get("detector")).containsKey("findings_index")); assertFalse(((Map) 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"; @@ -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); @@ -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 responseBody = asMap(createResponse); @@ -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); } } } \ No newline at end of file