diff --git a/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java b/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java index 5e1a43b3b..3851ed695 100644 --- a/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java +++ b/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java @@ -15,6 +15,7 @@ import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.util.ParseUtils.addUserBackendRolesFilter; import static org.opensearch.ad.util.ParseUtils.getUserContext; +import static org.opensearch.ad.util.ParseUtils.isAdmin; import static org.opensearch.ad.util.RestHandlerUtils.wrapRestActionListener; import org.apache.logging.log4j.LogManager; @@ -62,13 +63,14 @@ public void search(SearchRequest request, ActionListener actionL } private void validateRole(SearchRequest request, User user, ActionListener listener) { - if (user == null || !filterEnabled) { + if (user == null || !filterEnabled || isAdmin(user)) { // Case 1: user == null when 1. Security is disabled. 2. When user is super-admin // Case 2: If Security is enabled and filter is disabled, proceed with search as // user is already authenticated to hit this API. + // case 3: user is admin which means we don't have to check backend role filtering client.search(request, listener); } else { - // Security is enabled and filter is enabled + // Security is enabled, filter is enabled and user isn't admin try { addUserBackendRolesFilter(user, request.source()); logger.debug("Filtering result by " + user.getBackendRoles()); diff --git a/src/main/java/org/opensearch/ad/util/ParseUtils.java b/src/main/java/org/opensearch/ad/util/ParseUtils.java index 9f6fa9e87..b75430022 100644 --- a/src/main/java/org/opensearch/ad/util/ParseUtils.java +++ b/src/main/java/org/opensearch/ad/util/ParseUtils.java @@ -559,7 +559,7 @@ public static void onGetAdResponse( AnomalyDetector detector = AnomalyDetector.parse(parser); User resourceUser = detector.getUser(); - if (!filterByBackendRole || checkUserPermissions(requestUser, resourceUser, detectorId)) { + if (!filterByBackendRole || checkUserPermissions(requestUser, resourceUser, detectorId) || isAdmin(requestUser)) { function.accept(detector); } else { logger.debug("User: " + requestUser.getName() + " does not have permissions to access detector: " + detectorId); @@ -573,6 +573,16 @@ public static void onGetAdResponse( } } + /** + * 'all_access' role users are treated as admins. + */ + public static boolean isAdmin(User user) { + if (user == null) { + return false; + } + return user.getRoles().contains("all_access"); + } + private static boolean checkUserPermissions(User requestedUser, User resourceUser, String detectorId) throws Exception { if (resourceUser.getBackendRoles() == null || requestedUser.getBackendRoles() == null) { return false; diff --git a/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java b/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java index 357e0ce5a..0c26143bb 100644 --- a/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java +++ b/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java @@ -16,8 +16,11 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; +import java.util.Map; +import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.message.BasicHeader; import org.hamcrest.CoreMatchers; import org.hamcrest.MatcherAssert; import org.junit.After; @@ -186,6 +189,77 @@ public void testGetApiFilterByEnabled() throws IOException { ); } + private void confirmingClientIsAdmin() throws IOException { + Response resp = TestHelpers + .makeRequest( + client(), + "GET", + "_plugins/_security/api/account", + null, + "", + ImmutableList.of(new BasicHeader(HttpHeaders.USER_AGENT, "admin")) + ); + Map responseMap = entityAsMap(resp); + ArrayList roles = (ArrayList) responseMap.get("roles"); + assertTrue(roles.contains("all_access")); + } + + public void testGetApiFilterByEnabledForAdmin() throws IOException { + // User Alice has AD full access, should be able to create a detector and has backend role "odfe" + AnomalyDetector aliceDetector = createRandomAnomalyDetector(false, false, aliceClient); + enableFilterBy(); + confirmingClientIsAdmin(); + AnomalyDetector detector = getAnomalyDetector(aliceDetector.getDetectorId(), client()); + Assert + .assertArrayEquals( + "User backend role of detector doesn't change", + new String[] { "odfe" }, + detector.getUser().getBackendRoles().toArray(new String[0]) + ); + } + + public void testUpdateApiFilterByEnabledForAdmin() throws IOException { + // User Alice has AD full access, should be able to create a detector and has backend role "odfe" + AnomalyDetector aliceDetector = createRandomAnomalyDetector(false, false, aliceClient); + enableFilterBy(); + + AnomalyDetector newDetector = new AnomalyDetector( + aliceDetector.getDetectorId(), + aliceDetector.getVersion(), + aliceDetector.getName(), + randomAlphaOfLength(10), + aliceDetector.getTimeField(), + aliceDetector.getIndices(), + aliceDetector.getFeatureAttributes(), + aliceDetector.getFilterQuery(), + aliceDetector.getDetectionInterval(), + aliceDetector.getWindowDelay(), + aliceDetector.getShingleSize(), + aliceDetector.getUiMetadata(), + aliceDetector.getSchemaVersion(), + Instant.now(), + aliceDetector.getCategoryField(), + new User( + randomAlphaOfLength(5), + ImmutableList.of("odfe", randomAlphaOfLength(5)), + ImmutableList.of(randomAlphaOfLength(5)), + ImmutableList.of(randomAlphaOfLength(5)) + ), + null + ); + // User client has admin all access, and has "opensearch" backend role so client should be able to update detector + // But the detector's backend role should not be replaced as client's backend roles (all_access). + Response response = updateAnomalyDetector(aliceDetector.getDetectorId(), newDetector, client()); + Assert.assertEquals(response.getStatusLine().getStatusCode(), 200); + AnomalyDetector anomalyDetector = getAnomalyDetector(aliceDetector.getDetectorId(), aliceClient); + Assert + .assertArrayEquals( + "odfe is still the backendrole, not opensearch", + new String[] { "odfe" }, + anomalyDetector.getUser().getBackendRoles().toArray(new String[0]) + ); + } + public void testUpdateApiFilterByEnabled() throws IOException { // User Alice has AD full access, should be able to create a detector AnomalyDetector aliceDetector = createRandomAnomalyDetector(false, false, aliceClient); diff --git a/src/test/java/org/opensearch/ad/util/ParseUtilsTests.java b/src/test/java/org/opensearch/ad/util/ParseUtilsTests.java index da8d992f6..972dbb46c 100644 --- a/src/test/java/org/opensearch/ad/util/ParseUtilsTests.java +++ b/src/test/java/org/opensearch/ad/util/ParseUtilsTests.java @@ -12,6 +12,7 @@ package org.opensearch.ad.util; import static org.opensearch.ad.util.ParseUtils.addUserBackendRolesFilter; +import static org.opensearch.ad.util.ParseUtils.isAdmin; import java.io.IOException; import java.time.Instant; @@ -287,4 +288,29 @@ public void testGetFeatureFieldNames() throws IOException { assertTrue(fieldNames.contains("field-name1")); assertTrue(fieldNames.contains("field-name2")); } + + public void testIsAdmin() { + User user1 = new User( + randomAlphaOfLength(5), + ImmutableList.of(), + ImmutableList.of("all_access"), + ImmutableList.of(randomAlphaOfLength(5)) + ); + assertTrue(isAdmin(user1)); + } + + public void testIsAdminBackendRoleIsAllAccess() { + String backendRole1 = "all_access"; + User user1 = new User( + randomAlphaOfLength(5), + ImmutableList.of(backendRole1), + ImmutableList.of(randomAlphaOfLength(5)), + ImmutableList.of(randomAlphaOfLength(5)) + ); + assertFalse(isAdmin(user1)); + } + + public void testIsAdminNull() { + assertFalse(isAdmin(null)); + } }