From 25e6a7069a8b380d26988fb92bbcce8cdc76edb5 Mon Sep 17 00:00:00 2001 From: rutuja-amazon <110013621+rutuja-amazon@users.noreply.github.com> Date: Wed, 10 Aug 2022 07:19:26 +0530 Subject: [PATCH] Revert "backport cluster evaluation permissions #1885 to opendistro-1.11 (#1986)" This reverts commit ec8057846c41e3283b5ceafeaa06e3964e9c2842. Signed-off-by: Rutuja Surve --- .../privileges/PrivilegesEvaluator.java | 1 - .../resolver/IndexResolverReplacer.java | 3 +- ...exTemplateClusterPermissionsCheckTest.java | 64 --------------- .../security/test/helper/rest/RestHelper.java | 79 +------------------ src/test/resources/internal_users.yml | 7 -- src/test/resources/roles.yml | 71 ----------------- src/test/resources/roles_mapping.yml | 36 --------- 7 files changed, 3 insertions(+), 258 deletions(-) delete mode 100644 src/test/java/com/amazon/opendistroforelasticsearch/security/IndexTemplateClusterPermissionsCheckTest.java diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/com/amazon/opendistroforelasticsearch/security/privileges/PrivilegesEvaluator.java index 106ab01a42..cf40706570 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/security/privileges/PrivilegesEvaluator.java @@ -585,7 +585,6 @@ private Set evaluateAdditionalIndexPermissions(final ActionRequest reque private static boolean isClusterPerm(String action0) { return ( action0.startsWith("cluster:") || action0.startsWith("indices:admin/template/") - || action0.startsWith("indices:admin/index_template/") || action0.startsWith(SearchScrollAction.NAME) || (action0.equals(BulkAction.NAME)) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/security/resolver/IndexResolverReplacer.java b/src/main/java/com/amazon/opendistroforelasticsearch/security/resolver/IndexResolverReplacer.java index 137b318eaf..71a6ed02b5 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/security/resolver/IndexResolverReplacer.java @@ -46,7 +46,6 @@ import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; -import org.apache.commons.collections.keyvalue.MultiKey; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionRequest; @@ -758,4 +757,4 @@ else if (RestoreSnapshotRequest.class.isInstance(localRequest)) { public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { respectRequestIndicesOptions = dcm.isRespectRequestIndicesEnabled(); } -} +} \ No newline at end of file diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/security/IndexTemplateClusterPermissionsCheckTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/security/IndexTemplateClusterPermissionsCheckTest.java deleted file mode 100644 index 26aec2481f..0000000000 --- a/src/test/java/com/amazon/opendistroforelasticsearch/security/IndexTemplateClusterPermissionsCheckTest.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security; - -import org.apache.http.HttpStatus; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - -import org.opensearch.security.test.SingleClusterTest; -import org.opensearch.security.test.helper.rest.RestHelper; -import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; - -public class IndexTemplateClusterPermissionsCheckTest extends SingleClusterTest{ - private RestHelper rh; - - final static String indexTemplateBody = "{ \"index_patterns\": [\"sem1234*\"], \"template\": { \"settings\": { \"number_of_shards\": 2, \"number_of_replicas\": 1 }, \"mappings\": { \"properties\": { \"timestamp\": { \"type\": \"date\", \"format\": \"yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis\" }, \"value\": { \"type\": \"double\" } } } } }"; - - private String getFailureResponseReason(String user) { - return "no permissions for [indices:admin/index_template/put] and User [name=" + user + ", backend_roles=[], requestedTenant=null]"; - } - - @Before - public void setupRestHelper() throws Exception{ - setup(); - rh = nonSslRestHelper(); - } - @Test - public void testPutIndexTemplateByNonPrivilegedUser() throws Exception { - String expectedFailureResponse = getFailureResponseReason("ds3"); - - // should fail, as user `ds3` doesn't have correct permissions - HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds3", "nagilum")); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason")); - } - - @Test - public void testPutIndexTemplateByPrivilegedUser() throws Exception { - // should pass, as user `sem-user` has correct permissions - HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("sem-user", "nagilum")); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - } - - @Test - public void testPutIndexTemplateAsIndexLevelPermission() throws Exception { - String expectedFailureResponse = getFailureResponseReason("sem-user2"); - - // should fail, as user `sem-user2` is assigned `put-template` permission as index-level, not cluster-level - HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("sem-user2", "nagilum")); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason")); - } - - } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/security/test/helper/rest/RestHelper.java b/src/test/java/com/amazon/opendistroforelasticsearch/security/test/helper/rest/RestHelper.java index a837f332b4..287e591d09 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/security/test/helper/rest/RestHelper.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/security/test/helper/rest/RestHelper.java @@ -37,16 +37,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Scanner; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.net.ssl.SSLContext; -import com.fasterxml.jackson.databind.JsonNode; import org.apache.commons.io.IOUtils; import org.apache.http.Header; import org.apache.http.HttpEntity; @@ -75,11 +71,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.test.helper.cluster.ClusterInfo; -import org.opensearch.security.test.helper.file.FileHelper; - -import static org.junit.jupiter.api.Assertions.fail; +import com.amazon.opendistroforelasticsearch.security.test.helper.cluster.ClusterInfo; +import com.amazon.opendistroforelasticsearch.security.test.helper.file.FileHelper; public class RestHelper { @@ -349,74 +342,6 @@ public String toString() { + ", statusReason=" + statusReason + "]"; } - private static void findArrayAccessor(String input) { - final Pattern r = Pattern.compile("(.+?)\\[(\\d+)\\]"); - final Matcher m = r.matcher(input); - if(m.find()) { - System.out.println("'" + input + "'\t Name was: " + m.group(1) + ",\t index position: " + m.group(2)); - } else { - System.out.println("'" + input + "'\t No Match"); - } - } - - /** - * Given a json path with dots delimiated returns the object at the leaf - */ - public String findValueInJson(final String jsonDotPath) { - // Make sure its json / then parse it - if (!isJsonContentType()) { - fail("Response was expected to be JSON, body was: \n" + body); - } - JsonNode currentNode = null; - try { - currentNode = DefaultObjectMapper.readTree(body); - } catch (final Exception e) { - throw new RuntimeException(e); - } - - // Break the path into parts, and scan into the json object - try (final Scanner jsonPathScanner = new Scanner(jsonDotPath).useDelimiter("\\.")) { - if (!jsonPathScanner.hasNext()) { - fail("Invalid json dot path '" + jsonDotPath + "', rewrite with '.' characters between path elements."); - } - do { - String pathEntry = jsonPathScanner.next(); - // if pathEntry is an array lookup - int arrayEntryIdx = -1; - - // Looks for an array-lookup pattern in the path - // e.g. root_cause[1] -> will match - // e.g. root_cause[2aasd] -> won't match - final Pattern r = Pattern.compile("(.+?)\\[(\\d+)\\]"); - final Matcher m = r.matcher(pathEntry); - if(m.find()) { - pathEntry = m.group(1); - arrayEntryIdx = Integer.parseInt(m.group(2)); - } - - if (!currentNode.has(pathEntry)) { - fail("Unable to resolve '" + jsonDotPath + "', on path entry '" + pathEntry + "' from available fields " + currentNode.toPrettyString()); - } - currentNode = currentNode.get(pathEntry); - - // if it's an Array lookup we get the requested index item - if (arrayEntryIdx > -1) { - if(!currentNode.isArray()) { - fail("Unable to resolve '" + jsonDotPath + "', the '" + pathEntry + "' field is not an array " + currentNode.toPrettyString()); - } else if (!currentNode.has(arrayEntryIdx)) { - fail("Unable to resolve '" + jsonDotPath + "', index '" + arrayEntryIdx + "' is out of bounds for array '" + pathEntry + "' \n" + currentNode.toPrettyString()); - } - currentNode = currentNode.get(arrayEntryIdx); - } - } while (jsonPathScanner.hasNext()); - - if (!currentNode.isValueNode()) { - fail("Unexpected value note, index directly to the object to reference, object\n" + currentNode.toPrettyString()); - } - return currentNode.asText(); - } - } - private static HttpResponse from(Future future) { try { return future.get(); diff --git a/src/test/resources/internal_users.yml b/src/test/resources/internal_users.yml index 7ab1306899..31401a5370 100644 --- a/src/test/resources/internal_users.yml +++ b/src/test/resources/internal_users.yml @@ -338,10 +338,3 @@ hidden_test: hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m opendistro_security_roles: - hidden_test -sem-user: - hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m - #password is: nagilum -sem-user2: - hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m - #password is: nagilum - diff --git a/src/test/resources/roles.yml b/src/test/resources/roles.yml index e867055955..6e8c7f452e 100644 --- a/src/test/resources/roles.yml +++ b/src/test/resources/roles.yml @@ -1072,80 +1072,9 @@ xyz_sr_reserved: hidden_test: cluster_permissions: - SGS_CLUSTER_COMPOSITE_OPS - -index_template_perm: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [] - index_permissions: - - index_patterns: - - "*" - allowed_actions: - - "indices:admin/index_template/*" - -data_stream_0: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [] - index_permissions: - - index_patterns: - - "*my-data-stream2*" - allowed_actions: - - "indices:admin/get" - -data_stream_1: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [ "indices:admin/index_template/put" ] - index_permissions: - - index_patterns: - - "my-data-stream*" - allowed_actions: - - "indices:admin/data_stream/get" - - "indices:admin/data_stream/create" - -data_stream_2: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [] index_permissions: - index_patterns: - hidden_test_not_hidden - allowed_actions: - - "DATASTREAM_ALL" - -hidden_test: - cluster_permissions: - - SGS_CLUSTER_COMPOSITE_OPS - index_permissions: - - index_patterns: - - hidden_test_not_hidden - allowed_actions: - - "*" - -sem-role: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [ "cluster_monitor", "indices:admin/index_template/put" ] - index_permissions: - - index_patterns: - - "sem*" allowed_actions: - "*" -sem-role2: - reserved: true - hidden: false - description: "Migrated from v6 (all types mapped)" - cluster_permissions: [ "cluster_monitor" ] - index_permissions: - - index_patterns: - - "sem*" - allowed_actions: - - "indices:admin/index_template/put" - diff --git a/src/test/resources/roles_mapping.yml b/src/test/resources/roles_mapping.yml index 435cc8c71b..d62758a26c 100644 --- a/src/test/resources/roles_mapping.yml +++ b/src/test/resources/roles_mapping.yml @@ -388,39 +388,3 @@ xyz_sr_hidden: bulk_test_user_role: users: - "bulk_test_user" -index_template_perm: - reserved: false - hidden: false - users: - - "ds1" -data_stream_0: - reserved: false - hidden: false - users: - - "ds0" -data_stream_1: - reserved: false - hidden: false - users: - - "ds1" -data_stream_2: - reserved: false - hidden: false - users: - - "ds2" -data_stream_3: - reserved: false - hidden: false - users: - - "ds3" -sem-role: - reserved: false - hidden: false - users: - - "sem-user" -sem-role2: - reserved: false - hidden: false - users: - - "sem-user2" -