From 2bc0e9925a2d421007e31b82c5ec8cf616197e13 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 19 Mar 2025 06:49:51 +0100 Subject: [PATCH 01/23] Prevent usage of :: selectors for remote cluster requests --- .../metadata/IndexNameExpressionResolver.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index e221cb5c08e5b..64216cfb364a3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2356,6 +2356,7 @@ private static V splitSelectorExpression(String expression, BiFunction Date: Wed, 19 Mar 2025 19:36:22 +0100 Subject: [PATCH 02/23] fail only if ::failures selector is used --- .../metadata/IndexNameExpressionResolver.java | 28 ++++++++++--------- .../metadata/SelectorResolverTests.java | 21 ++++++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 64216cfb364a3..74251deb33afc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2356,7 +2356,7 @@ private static V splitSelectorExpression(String expression, BiFunction resolve(selectorsAllowed, "::::data")); + expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::failures")); + expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, ":::::failures")); // Suffix case is not supported because there is no component named with the empty string expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::")); + + // remote cluster syntax is not allowed with ::failures selector + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:index::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(noSelectors, "cluster:index::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index-*::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:*::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:index-*::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:*::failures"); + // even with an empty index name + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:::failures"); + } + + public void assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(Context context, String expression) { + var e = expectThrows(IllegalArgumentException.class, () -> resolve(context, expression)); + assertThat(e.getMessage(), containsString("failures selector is not supported with remote cluster expressions")); } public void testResolveMatchAllToSelectors() { From ba976a9742bd14b81209a3384812afa62c8ac2f6 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 19 Mar 2025 22:51:18 +0100 Subject: [PATCH 03/23] cleanup and extend existing rest IT --- .../metadata/IndexNameExpressionResolver.java | 14 +++++++------- .../cluster/metadata/SelectorResolverTests.java | 2 +- .../remotecluster/RemoteClusterSecurityRestIT.java | 8 ++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 74251deb33afc..0f92b294dfa57 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2356,7 +2356,7 @@ private static V splitSelectorExpression(String expression, BiFunction resolve(context, expression)); - assertThat(e.getMessage(), containsString("failures selector is not supported with remote cluster expressions")); + assertThat(e.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); } public void testResolveMatchAllToSelectors() { diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java index 307f59859c75a..afa8e9365fe93 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java @@ -609,6 +609,14 @@ public void testCrossClusterSearch() throws Exception { assertThat(exception6.getResponse().getStatusLine().getStatusCode(), equalTo(401)); assertThat(exception6.getMessage(), containsString("invalid cross-cluster API key value")); } + + // check that ::failures selector is not supported with cross cluster search + final ResponseException exception7 = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser(new Request("GET", "/my_remote_cluster:index1::failures/_search")) + ); + assertThat(exception7.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat(exception7.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); } assertNoRcs1DeprecationWarnings(); } From 5bc46b552e001bc5bd53792d74ae13cbcc0819da Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 20 Mar 2025 10:41:12 +0100 Subject: [PATCH 04/23] test ccs with rcs1 --- ...moteClusterSecurityFailureStoreRestIT.java | 134 +++++++++++++ ...ClusterSecurityRCS1FailureStoreRestIT.java | 182 ++++++++++++++++++ .../RemoteClusterSecurityRestIT.java | 8 - 3 files changed, 316 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java new file mode 100644 index 0000000000000..d1cd64b4cd992 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -0,0 +1,134 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.SearchResponseUtils; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; + +abstract class AbstractRemoteClusterSecurityFailureStoreRestIT extends AbstractRemoteClusterSecurityTestCase { + + protected void assertSearchResponseContainsIndices(Response response, String... expectedIndices) throws IOException { + assertOK(response); + final SearchResponse dataSearchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + try { + final List actualIndices = Arrays.stream(dataSearchResponse.getHits().getHits()) + .map(SearchHit::getIndex) + .collect(Collectors.toList()); + assertThat(actualIndices, containsInAnyOrder(expectedIndices)); + } finally { + dataSearchResponse.decRef(); + } + } + + protected void setupTestDataStreamOnFulfillingCluster() throws IOException { + // Create data stream and index some documents + final Request createComponentTemplate = new Request("PUT", "/_component_template/component1"); + createComponentTemplate.setJsonEntity(""" + { + "template": { + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "age": { + "type": "integer" + }, + "email": { + "type": "keyword" + }, + "name": { + "type": "text" + } + } + }, + "data_stream_options": { + "failure_store": { + "enabled": true + } + } + } + }"""); + assertOK(performRequestAgainstFulfillingCluster(createComponentTemplate)); + + final Request createTemplate = new Request("PUT", "/_index_template/template1"); + createTemplate.setJsonEntity(""" + { + "index_patterns": ["test*"], + "data_stream": {}, + "priority": 500, + "composed_of": ["component1"] + }"""); + assertOK(performRequestAgainstFulfillingCluster(createTemplate)); + + final Request createDoc1 = new Request("PUT", "/test1/_doc/1?refresh=true&op_type=create"); + createDoc1.setJsonEntity(""" + { + "@timestamp": 1, + "age" : 1, + "name" : "jack", + "email" : "jack@example.com" + }"""); + assertOK(performRequestAgainstFulfillingCluster(createDoc1)); + + final Request createDoc2 = new Request("PUT", "/test1/_doc/2?refresh=true&op_type=create"); + createDoc2.setJsonEntity(""" + { + "@timestamp": 2, + "age" : "this should be an int", + "name" : "jack", + "email" : "jack@example.com" + }"""); + assertOK(performRequestAgainstFulfillingCluster(createDoc2)); + } + + protected Response performRequestWithRemoteSearchUser(final Request request) throws IOException { + request.setOptions( + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(REMOTE_SEARCH_USER, PASS)) + ); + return client().performRequest(request); + } + + @SuppressWarnings("unchecked") + protected Tuple, List> getDataAndFailureIndices(String dataStreamName) throws IOException { + Request dataStream = new Request("GET", "/_data_stream/" + dataStreamName); + Response response = performRequestAgainstFulfillingCluster(dataStream); + Map dataStreams = entityAsMap(response); + assertEquals(Collections.singletonList("test1"), XContentMapValues.extractValue("data_streams.name", dataStreams)); + List dataIndexNames = (List) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams); + List failureIndexNames = (List) XContentMapValues.extractValue( + "data_streams.failure_store.indices.index_name", + dataStreams + ); + return new Tuple<>(dataIndexNames, failureIndexNames); + } + + protected Tuple getSingleDataAndFailureIndices(String dataStreamName) throws IOException { + Tuple, List> indices = getDataAndFailureIndices(dataStreamName); + assertThat(indices.v1().size(), equalTo(1)); + assertThat(indices.v2().size(), equalTo(1)); + return new Tuple<>(indices.v1().get(0), indices.v2().get(0)); + } + +} diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java new file mode 100644 index 0000000000000..33ce15adabe79 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -0,0 +1,182 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.FeatureFlag; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.Locale; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class RemoteClusterSecurityRCS1FailureStoreRestIT extends AbstractRemoteClusterSecurityFailureStoreRestIT { + + static { + fulfillingCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("fulfilling-cluster") + .nodes(3) + .apply(commonClusterConfig) + .feature(FeatureFlag.FAILURE_STORE_ENABLED) + .rolesFile(Resource.fromClasspath("roles.yml")) + .build(); + + queryCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("query-cluster") + .apply(commonClusterConfig) + .feature(FeatureFlag.FAILURE_STORE_ENABLED) + .rolesFile(Resource.fromClasspath("roles.yml")) + .build(); + } + + @ClassRule + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + public void testCrossClusterSearch() throws Exception { + // configure remote cluster using certificate-based authentication + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), randomBoolean()); + + // fulfilling cluster setup + setupRoleAndUserOnFulfillingCluster(); + setupTestDataStreamOnFulfillingCluster(); + + // query cluster setup + setupLocalDataOnQueryCluster(); + setupUserAndRoleOnQueryCluster(); + + final Tuple backingIndices = getSingleDataAndFailureIndices("test1"); + final String backingDataIndexName = backingIndices.v1(); + final String backingFailureIndexName = backingIndices.v2(); + { + // query remote cluster using ::data selector should succeed + final boolean alsoSearchLocally = randomBoolean(); + final Request dataSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false", + alsoSearchLocally ? "local_index," : "", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), + randomBoolean() + ) + ); + final String[] expectedIndices = alsoSearchLocally + ? new String[] { "local_index", backingDataIndexName } + : new String[] { backingDataIndexName }; + assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(dataSearchRequest), expectedIndices); + } + { + // query remote cluster using ::failures selector should fail + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser( + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + randomFrom("test1::failures", "test*::failures", "*::failures"), + randomBoolean() + ) + ) + ) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + } + { + // direct access to backing failure index is subject to the user's permissions and is allowed + assertSearchResponseContainsIndices( + performRequestWithRemoteSearchUser( + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + backingFailureIndexName, + randomBoolean() + ) + ) + ), + backingFailureIndexName + ); + } + } + + private static void setupLocalDataOnQueryCluster() throws IOException { + // Index some documents, to use them in a mixed-cluster search + final var indexDocRequest = new Request("POST", "/local_index/_doc?refresh=true"); + indexDocRequest.setJsonEntity("{\"local_foo\": \"local_bar\"}"); + assertOK(client().performRequest(indexDocRequest)); + } + + private static void setupUserAndRoleOnQueryCluster() throws IOException { + // Create user role with privileges for remote and local indices + final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequest.setJsonEntity(""" + { + "description": "Role with privileges for remote and local indices.", + "indices": [ + { + "names": ["local_index"], + "privileges": ["read"] + } + ], + "remote_indices": [ + { + "names": ["test*"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["my_remote_cluster"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleRequest)); + final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + } + + private static void setupRoleAndUserOnFulfillingCluster() throws IOException { + var putRoleOnFulfillingClusterRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleOnFulfillingClusterRequest.setJsonEntity(""" + { + "indices": [ + { + "names": ["test*"], + "privileges": ["read", "read_cross_cluster", "read_failure_store"] + } + ] + }"""); + assertOK(performRequestAgainstFulfillingCluster(putRoleOnFulfillingClusterRequest)); + + var putUserOnFulfillingClusterRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserOnFulfillingClusterRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(performRequestAgainstFulfillingCluster(putUserOnFulfillingClusterRequest)); + } + +} diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java index afa8e9365fe93..307f59859c75a 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestIT.java @@ -609,14 +609,6 @@ public void testCrossClusterSearch() throws Exception { assertThat(exception6.getResponse().getStatusLine().getStatusCode(), equalTo(401)); assertThat(exception6.getMessage(), containsString("invalid cross-cluster API key value")); } - - // check that ::failures selector is not supported with cross cluster search - final ResponseException exception7 = expectThrows( - ResponseException.class, - () -> performRequestWithRemoteSearchUser(new Request("GET", "/my_remote_cluster:index1::failures/_search")) - ); - assertThat(exception7.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat(exception7.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); } assertNoRcs1DeprecationWarnings(); } From e92ffde72b3bdf36d5cdc986abebcf0b27b4630d Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 20 Mar 2025 11:27:18 +0100 Subject: [PATCH 05/23] nit --- ...ClusterSecurityRCS1FailureStoreRestIT.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 33ce15adabe79..008d83a7af57d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -103,20 +103,16 @@ public void testCrossClusterSearch() throws Exception { } { // direct access to backing failure index is subject to the user's permissions and is allowed - assertSearchResponseContainsIndices( - performRequestWithRemoteSearchUser( - new Request( - "GET", - String.format( - Locale.ROOT, - "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", - backingFailureIndexName, - randomBoolean() - ) - ) - ), - backingFailureIndexName + Request failureIndexSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + backingFailureIndexName, + randomBoolean() + ) ); + assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); } } From 29de66983c7695f2fa127473ceb0bf25a55fb37f Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 20 Mar 2025 11:47:01 +0100 Subject: [PATCH 06/23] remove remote_indices - not relevant for RCS1 test case --- .../RemoteClusterSecurityRCS1FailureStoreRestIT.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 008d83a7af57d..37dc421e97c82 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -134,13 +134,6 @@ private static void setupUserAndRoleOnQueryCluster() throws IOException { "names": ["local_index"], "privileges": ["read"] } - ], - "remote_indices": [ - { - "names": ["test*"], - "privileges": ["read", "read_cross_cluster"], - "clusters": ["my_remote_cluster"] - } ] }"""); assertOK(adminClient().performRequest(putRoleRequest)); From eaf9a01b3a07b1126ec04e6967c04b20012546f5 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 20 Mar 2025 12:18:18 +0100 Subject: [PATCH 07/23] test CCS with RCS2 --- ...ClusterSecurityRCS2FailureStoreRestIT.java | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java new file mode 100644 index 0000000000000..fb8f47d23c32c --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -0,0 +1,179 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.FeatureFlag; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class RemoteClusterSecurityRCS2FailureStoreRestIT extends AbstractRemoteClusterSecurityFailureStoreRestIT { + + private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); + + static { + fulfillingCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("fulfilling-cluster") + .apply(commonClusterConfig) + .feature(FeatureFlag.FAILURE_STORE_ENABLED) + .setting("remote_cluster_server.enabled", "true") + .setting("remote_cluster.port", "0") + .setting("xpack.security.remote_cluster_server.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + .build(); + + queryCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("query-cluster") + .apply(commonClusterConfig) + .feature(FeatureFlag.FAILURE_STORE_ENABLED) + .setting("xpack.security.remote_cluster_client.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("cluster.remote.my_remote_cluster.credentials", () -> { + API_KEY_MAP_REF.compareAndSet(null, createCrossClusterAccessApiKey(""" + { + "search": [ + { + "names": ["test*"] + } + ] + }""")); + return (String) API_KEY_MAP_REF.get().get("encoded"); + }) + .rolesFile(Resource.fromClasspath("roles.yml")) + .build(); + } + + @ClassRule + // Use a RuleChain to ensure that fulfilling cluster is started before query cluster + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + public void testCrossClusterSearch() throws Exception { + // configure remote cluster using API Key-based authentication + configureRemoteCluster(); + + // fulfilling cluster setup + setupTestDataStreamOnFulfillingCluster(); + + // query cluster setup + setupLocalDataOnQueryCluster(); + setupUserAndRoleOnQueryCluster(); + + final Tuple backingIndices = getSingleDataAndFailureIndices("test1"); + final String backingDataIndexName = backingIndices.v1(); + final String backingFailureIndexName = backingIndices.v2(); + { + // query remote cluster using ::data selector should succeed + final boolean alsoSearchLocally = randomBoolean(); + final Request dataSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false", + alsoSearchLocally ? "local_index," : "", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), + randomBoolean() + ) + ); + final String[] expectedIndices = alsoSearchLocally + ? new String[] { "local_index", backingDataIndexName } + : new String[] { backingDataIndexName }; + assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(dataSearchRequest), expectedIndices); + } + { + // query remote cluster using ::failures selector should fail + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser( + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + randomFrom("test1::failures", "test*::failures", "*::failures"), + randomBoolean() + ) + ) + ) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + } + { + // direct access to backing failure index is subject to the user's permissions and is allowed + Request failureIndexSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + backingFailureIndexName, + randomBoolean() + ) + ); + assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); + } + } + + private static void setupLocalDataOnQueryCluster() throws IOException { + // Index some documents, to use them in a mixed-cluster search + final var indexDocRequest = new Request("POST", "/local_index/_doc?refresh=true"); + indexDocRequest.setJsonEntity("{\"local_foo\": \"local_bar\"}"); + assertOK(client().performRequest(indexDocRequest)); + } + + private static void setupUserAndRoleOnQueryCluster() throws IOException { + // Create user role with privileges for remote and local indices + final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequest.setJsonEntity(""" + { + "description": "Role with privileges for remote and local indices.", + "indices": [ + { + "names": ["local_index"], + "privileges": ["read"] + } + ], + "remote_indices": [ + { + "names": ["test*"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["my_remote_cluster"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleRequest)); + final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + } + +} From 4e366eda595be1b85cf5189064c63f9c8cb2a45a Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 21:21:34 +0100 Subject: [PATCH 08/23] cleanup tests, handle edge cases with ccs_minimize_roundtrips --- ...moteClusterSecurityFailureStoreRestIT.java | 6 +-- ...ClusterSecurityRCS1FailureStoreRestIT.java | 54 ++++++++++++++----- ...ClusterSecurityRCS2FailureStoreRestIT.java | 22 ++++++-- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index d1cd64b4cd992..2d169098fff6f 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -30,14 +30,14 @@ abstract class AbstractRemoteClusterSecurityFailureStoreRestIT extends AbstractR protected void assertSearchResponseContainsIndices(Response response, String... expectedIndices) throws IOException { assertOK(response); - final SearchResponse dataSearchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); try { - final List actualIndices = Arrays.stream(dataSearchResponse.getHits().getHits()) + final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) .map(SearchHit::getIndex) .collect(Collectors.toList()); assertThat(actualIndices, containsInAnyOrder(expectedIndices)); } finally { - dataSearchResponse.decRef(); + searchResponse.decRef(); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 37dc421e97c82..fa0c4b0b192c3 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -48,9 +48,13 @@ public class RemoteClusterSecurityRCS1FailureStoreRestIT extends AbstractRemoteC @ClassRule public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); - public void testCrossClusterSearch() throws Exception { - // configure remote cluster using certificate-based authentication - configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), randomBoolean()); + public void testRCS1CrossClusterSearch() throws Exception { + final boolean rcs1Security = true; + final boolean isProxyMode = randomBoolean(); + final boolean skipUnavailable = false; // we want to get actual failures and not skip and get empty results + final boolean ccsMinimizeRoundtrips = randomBoolean(); + + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, rcs1Security, isProxyMode, skipUnavailable); // fulfilling cluster setup setupRoleAndUserOnFulfillingCluster(); @@ -70,11 +74,11 @@ public void testCrossClusterSearch() throws Exception { "GET", String.format( Locale.ROOT, - "/%s%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false", + "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", alsoSearchLocally ? "local_index," : "", randomFrom("my_remote_cluster", "*", "my_remote_*"), randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), - randomBoolean() + ccsMinimizeRoundtrips ) ); final String[] expectedIndices = alsoSearchLocally @@ -91,9 +95,9 @@ public void testCrossClusterSearch() throws Exception { "GET", String.format( Locale.ROOT, - "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=true", randomFrom("test1::failures", "test*::failures", "*::failures"), - randomBoolean() + ccsMinimizeRoundtrips ) ) ) @@ -102,17 +106,45 @@ public void testCrossClusterSearch() throws Exception { assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); } { - // direct access to backing failure index is subject to the user's permissions and is allowed + // direct access to backing failure index is subject to the user's permissions + // it might fail in some cases and work in others Request failureIndexSearchRequest = new Request( "GET", String.format( Locale.ROOT, "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", backingFailureIndexName, - randomBoolean() + ccsMinimizeRoundtrips ) ); - assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); + if (ccsMinimizeRoundtrips) { + // this is a special case where indices:data/read/search will be sent to a remote cluster + // and the request to backing failure store index will be authorized based on the datastream + // which grants access to backing failure store indices (granted by read_failure_store privilege) + // from a security perspective, this is a valid use case and there is no way to prevent this with RCS1 security model + // since from the fulfilling cluster perspective this request is no different from any other local search request + assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); + } else { + // in this case, the user does not have the necessary permissions to search the backing failure index + // the request to failure store backing index is authorized based on the datastream + // which does not grant access to the indices:admin/search/search_shards action + // this action is granted by read_cross_cluster privilege which is currently + // not supporting the failure backing indices (only data backing indices) + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser(failureIndexSearchRequest) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat( + exception.getMessage(), + containsString( + "action [indices:admin/search/search_shards] is unauthorized for user [remote_search_user] " + + "with effective roles [remote_search] on indices [" + + backingFailureIndexName + + "], this action is granted by the index privileges [view_index_metadata,manage,read_cross_cluster,all]" + ) + ); + } } } @@ -124,11 +156,9 @@ private static void setupLocalDataOnQueryCluster() throws IOException { } private static void setupUserAndRoleOnQueryCluster() throws IOException { - // Create user role with privileges for remote and local indices final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); putRoleRequest.setJsonEntity(""" { - "description": "Role with privileges for remote and local indices.", "indices": [ { "names": ["local_index"], diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index fb8f47d23c32c..bc2866bc63b65 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -72,9 +72,10 @@ public class RemoteClusterSecurityRCS2FailureStoreRestIT extends AbstractRemoteC // Use a RuleChain to ensure that fulfilling cluster is started before query cluster public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); - public void testCrossClusterSearch() throws Exception { + public void testRCS2CrossClusterSearch() throws Exception { // configure remote cluster using API Key-based authentication configureRemoteCluster(); + final String crossClusterAccessApiKeyId = (String) API_KEY_MAP_REF.get().get("id"); // fulfilling cluster setup setupTestDataStreamOnFulfillingCluster(); @@ -125,7 +126,7 @@ public void testCrossClusterSearch() throws Exception { assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); } { - // direct access to backing failure index is subject to the user's permissions and is allowed + // direct access to backing failure index is not allowed - no explicit read privileges over .fs-* indices Request failureIndexSearchRequest = new Request( "GET", String.format( @@ -135,7 +136,22 @@ public void testCrossClusterSearch() throws Exception { randomBoolean() ) ); - assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser(failureIndexSearchRequest) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat( + exception.getMessage(), + containsString( + "action [indices:data/read/search] towards remote cluster is unauthorized for user [remote_search_user] " + + "with assigned roles [remote_search] authenticated by API key id [" + + crossClusterAccessApiKeyId + + "] of user [test_user] on indices [" + + backingFailureIndexName + + "], this action is granted by the index privileges [read,all]" + ) + ); } } From 816d919983076dd1bbaac37c9fce7bada918b607 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 22:18:43 +0100 Subject: [PATCH 09/23] more test users --- ...moteClusterSecurityFailureStoreRestIT.java | 5 + ...ClusterSecurityRCS1FailureStoreRestIT.java | 118 ++++++++++++++---- ...ClusterSecurityRCS2FailureStoreRestIT.java | 11 +- 3 files changed, 107 insertions(+), 27 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index 2d169098fff6f..b4265d2b9e3ad 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -110,6 +110,11 @@ protected Response performRequestWithRemoteSearchUser(final Request request) thr return client().performRequest(request); } + protected Response performRequestWithUser(final String user, final Request request) throws IOException { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(user, PASS))); + return client().performRequest(request); + } + @SuppressWarnings("unchecked") protected Tuple, List> getDataAndFailureIndices(String dataStreamName) throws IOException { Request dataStream = new Request("GET", "/_data_stream/" + dataStreamName); diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index fa0c4b0b192c3..c121c2ee0f8b9 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -9,6 +9,9 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.core.Strings; import org.elasticsearch.core.Tuple; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.FeatureFlag; @@ -48,6 +51,10 @@ public class RemoteClusterSecurityRCS1FailureStoreRestIT extends AbstractRemoteC @ClassRule public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + private static final String ALL_ACCESS = "all_access"; + private static final String DATA_ACCESS = "data_access"; + private static final String FAILURE_STORE_ACCESS = "failure_store_access"; + public void testRCS1CrossClusterSearch() throws Exception { final boolean rcs1Security = true; final boolean isProxyMode = randomBoolean(); @@ -67,7 +74,9 @@ public void testRCS1CrossClusterSearch() throws Exception { final Tuple backingIndices = getSingleDataAndFailureIndices("test1"); final String backingDataIndexName = backingIndices.v1(); final String backingFailureIndexName = backingIndices.v2(); - { + + final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS, ALL_ACCESS }; + for (String user : users) { // query remote cluster using ::data selector should succeed final boolean alsoSearchLocally = randomBoolean(); final Request dataSearchRequest = new Request( @@ -84,13 +93,14 @@ public void testRCS1CrossClusterSearch() throws Exception { final String[] expectedIndices = alsoSearchLocally ? new String[] { "local_index", backingDataIndexName } : new String[] { backingDataIndexName }; - assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(dataSearchRequest), expectedIndices); + assertSearchResponseContainsIndices(performRequestWithUser(user, dataSearchRequest), expectedIndices); } - { - // query remote cluster using ::failures selector should fail + for (String user : users) { + // query remote cluster using ::failures selector should fail (regardless of the user's permissions) final ResponseException exception = expectThrows( ResponseException.class, - () -> performRequestWithRemoteSearchUser( + () -> performRequestWithUser( + user, new Request( "GET", String.format( @@ -123,7 +133,10 @@ public void testRCS1CrossClusterSearch() throws Exception { // which grants access to backing failure store indices (granted by read_failure_store privilege) // from a security perspective, this is a valid use case and there is no way to prevent this with RCS1 security model // since from the fulfilling cluster perspective this request is no different from any other local search request - assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(failureIndexSearchRequest), backingFailureIndexName); + assertSearchResponseContainsIndices( + performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest), + backingFailureIndexName + ); } else { // in this case, the user does not have the necessary permissions to search the backing failure index // the request to failure store backing index is authorized based on the datastream @@ -132,7 +145,7 @@ public void testRCS1CrossClusterSearch() throws Exception { // not supporting the failure backing indices (only data backing indices) final ResponseException exception = expectThrows( ResponseException.class, - () -> performRequestWithRemoteSearchUser(failureIndexSearchRequest) + () -> performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest) ); assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); assertThat( @@ -156,8 +169,19 @@ private static void setupLocalDataOnQueryCluster() throws IOException { } private static void setupUserAndRoleOnQueryCluster() throws IOException { - final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" + createRole(adminClient(), ALL_ACCESS, """ + { + "indices": [ + { + "names": ["*"], + "privileges": ["all"] + } + ] + }"""); + createUser(adminClient(), ALL_ACCESS, PASS, ALL_ACCESS); + // the role must simply exist on query cluster, the access is irrelevant, + // but we here grant the access to local_index only to test mixed search + createRole(adminClient(), FAILURE_STORE_ACCESS, """ { "indices": [ { @@ -166,19 +190,48 @@ private static void setupUserAndRoleOnQueryCluster() throws IOException { } ] }"""); - assertOK(adminClient().performRequest(putRoleRequest)); - final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); - putUserRequest.setJsonEntity(""" + createUser(adminClient(), FAILURE_STORE_ACCESS, PASS, FAILURE_STORE_ACCESS); + createRole(adminClient(), DATA_ACCESS, """ { - "password": "x-pack-test-password", - "roles" : ["remote_search"] + "indices": [ + { + "names": ["local_index"], + "privileges": ["read"] + } + ] }"""); - assertOK(adminClient().performRequest(putUserRequest)); + createUser(adminClient(), DATA_ACCESS, PASS, DATA_ACCESS); + } + + private static void createRole(RestClient client, String role, String roleDescriptor) throws IOException { + final Request putRoleRequest = new Request("PUT", "/_security/role/" + role); + putRoleRequest.setJsonEntity(roleDescriptor); + assertOK(client.performRequest(putRoleRequest)); + } + + private static void createUser(RestClient client, String user, SecureString password, String role) throws IOException { + final Request putUserRequest = new Request("PUT", "/_security/user/" + user); + putUserRequest.setJsonEntity(Strings.format(""" + { + "password": "%s", + "roles" : ["%s"] + }""", password.toString(), role)); + assertOK(client.performRequest(putUserRequest)); } private static void setupRoleAndUserOnFulfillingCluster() throws IOException { - var putRoleOnFulfillingClusterRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleOnFulfillingClusterRequest.setJsonEntity(""" + putRoleOnFulfillingCluster(DATA_ACCESS, """ + { + "indices": [ + { + "names": ["test*"], + "privileges": ["read", "read_cross_cluster"] + } + ] + }"""); + putUserOnFulfillingCluster(DATA_ACCESS, DATA_ACCESS); + + putRoleOnFulfillingCluster(FAILURE_STORE_ACCESS, """ { "indices": [ { @@ -187,15 +240,34 @@ private static void setupRoleAndUserOnFulfillingCluster() throws IOException { } ] }"""); - assertOK(performRequestAgainstFulfillingCluster(putRoleOnFulfillingClusterRequest)); + putUserOnFulfillingCluster(FAILURE_STORE_ACCESS, FAILURE_STORE_ACCESS); - var putUserOnFulfillingClusterRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); - putUserOnFulfillingClusterRequest.setJsonEntity(""" + putRoleOnFulfillingCluster(ALL_ACCESS, """ { - "password": "x-pack-test-password", - "roles" : ["remote_search"] + "indices": [ + { + "names": ["*"], + "privileges": ["all"] + } + ] }"""); - assertOK(performRequestAgainstFulfillingCluster(putUserOnFulfillingClusterRequest)); + putUserOnFulfillingCluster(ALL_ACCESS, ALL_ACCESS); + } + + private static void putRoleOnFulfillingCluster(String roleName, String roleDescriptor) throws IOException { + Request request = new Request("PUT", "/_security/role/" + roleName); + request.setJsonEntity(roleDescriptor); + assertOK(performRequestAgainstFulfillingCluster(request)); + } + + private static void putUserOnFulfillingCluster(String user, String role) throws IOException { + Request request = new Request("PUT", "/_security/user/" + user); + request.setJsonEntity(Strings.format(""" + { + "password": "%s", + "roles" : ["%s"] + }""", PASS.toString(), role)); + assertOK(performRequestAgainstFulfillingCluster(request)); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index bc2866bc63b65..9204c13b388b7 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -76,6 +76,7 @@ public void testRCS2CrossClusterSearch() throws Exception { // configure remote cluster using API Key-based authentication configureRemoteCluster(); final String crossClusterAccessApiKeyId = (String) API_KEY_MAP_REF.get().get("id"); + final boolean ccsMinimizeRoundtrips = randomBoolean(); // fulfilling cluster setup setupTestDataStreamOnFulfillingCluster(); @@ -98,7 +99,7 @@ public void testRCS2CrossClusterSearch() throws Exception { alsoSearchLocally ? "local_index," : "", randomFrom("my_remote_cluster", "*", "my_remote_*"), randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), - randomBoolean() + ccsMinimizeRoundtrips ) ); final String[] expectedIndices = alsoSearchLocally @@ -117,7 +118,7 @@ public void testRCS2CrossClusterSearch() throws Exception { Locale.ROOT, "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", randomFrom("test1::failures", "test*::failures", "*::failures"), - randomBoolean() + ccsMinimizeRoundtrips ) ) ) @@ -133,7 +134,7 @@ public void testRCS2CrossClusterSearch() throws Exception { Locale.ROOT, "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", backingFailureIndexName, - randomBoolean() + ccsMinimizeRoundtrips ) ); final ResponseException exception = expectThrows( @@ -144,7 +145,9 @@ public void testRCS2CrossClusterSearch() throws Exception { assertThat( exception.getMessage(), containsString( - "action [indices:data/read/search] towards remote cluster is unauthorized for user [remote_search_user] " + "action [" + + (ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards") + + "] towards remote cluster is unauthorized for user [remote_search_user] " + "with assigned roles [remote_search] authenticated by API key id [" + crossClusterAccessApiKeyId + "] of user [test_user] on indices [" From 7dc89d4e47fd9ea17c0e2f72847a5e6034f54cec Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 22:20:09 +0100 Subject: [PATCH 10/23] fix assertion --- .../RemoteClusterSecurityRCS1FailureStoreRestIT.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index c121c2ee0f8b9..ae20283fc343d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -151,8 +151,12 @@ public void testRCS1CrossClusterSearch() throws Exception { assertThat( exception.getMessage(), containsString( - "action [indices:admin/search/search_shards] is unauthorized for user [remote_search_user] " - + "with effective roles [remote_search] on indices [" + "action [indices:admin/search/search_shards] is unauthorized for user [" + + FAILURE_STORE_ACCESS + + "] " + + "with effective roles [" + + FAILURE_STORE_ACCESS + + "] on indices [" + backingFailureIndexName + "], this action is granted by the index privileges [view_index_metadata,manage,read_cross_cluster,all]" ) From 26e8ae36385645544d8bddff1a5d74106f736a66 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 22:35:40 +0100 Subject: [PATCH 11/23] test direct access to backing failure index for other users --- ...ClusterSecurityRCS1FailureStoreRestIT.java | 30 +++++++++++++++++++ ...ClusterSecurityRCS2FailureStoreRestIT.java | 1 - 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index ae20283fc343d..f1ef686bafd08 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -127,6 +127,36 @@ public void testRCS1CrossClusterSearch() throws Exception { ccsMinimizeRoundtrips ) ); + + // user with access to all should be able to search the backing failure index + assertSearchResponseContainsIndices(performRequestWithUser(ALL_ACCESS, failureIndexSearchRequest), backingFailureIndexName); + + // user with data only access should not be able to search the backing failure index + { + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser(DATA_ACCESS, failureIndexSearchRequest) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat( + exception.getMessage(), + containsString( + "action [" + + (ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards") + + "] is unauthorized for user [" + + DATA_ACCESS + + "] " + + "with effective roles [" + + DATA_ACCESS + + "] on indices [" + + backingFailureIndexName + + "], this action is granted by the index privileges [view_index_metadata,manage,read_cross_cluster,all]" + + ) + ); + } + + // for user with access to failure store, it depends on the underlying action that is being sent to the remote cluster if (ccsMinimizeRoundtrips) { // this is a special case where indices:data/read/search will be sent to a remote cluster // and the request to backing failure store index will be authorized based on the datastream diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index 9204c13b388b7..dcae5fd97301d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -166,7 +166,6 @@ private static void setupLocalDataOnQueryCluster() throws IOException { } private static void setupUserAndRoleOnQueryCluster() throws IOException { - // Create user role with privileges for remote and local indices final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); putRoleRequest.setJsonEntity(""" { From 9c3d0d7adb1ead842b461ee02d95ebfdad89f050 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 22:44:23 +0100 Subject: [PATCH 12/23] fix assertion --- .../RemoteClusterSecurityRCS2FailureStoreRestIT.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index dcae5fd97301d..9b6176292ca4b 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -152,7 +152,9 @@ public void testRCS2CrossClusterSearch() throws Exception { + crossClusterAccessApiKeyId + "] of user [test_user] on indices [" + backingFailureIndexName - + "], this action is granted by the index privileges [read,all]" + + "], this action is granted by the index privileges [" + + (ccsMinimizeRoundtrips ? "read,all" : "view_index_metadata,manage,read_cross_cluster,all") + + "]" ) ); } From e8a4b960d701c663ae2ecb8d2cd2dea4eaa12fc5 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Fri, 21 Mar 2025 23:42:20 +0100 Subject: [PATCH 13/23] fix another assertion --- .../RemoteClusterSecurityRCS1FailureStoreRestIT.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index f1ef686bafd08..267958be91fb7 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -150,7 +150,9 @@ public void testRCS1CrossClusterSearch() throws Exception { + DATA_ACCESS + "] on indices [" + backingFailureIndexName - + "], this action is granted by the index privileges [view_index_metadata,manage,read_cross_cluster,all]" + + "], this action is granted by the index privileges [" + + (ccsMinimizeRoundtrips ? "read,all" : "view_index_metadata,manage,read_cross_cluster,all") + + "]" ) ); From 6d5293fdc3e6e0f71544e41f2aa3f2f49bfbcb6e Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 25 Mar 2025 12:57:59 +0100 Subject: [PATCH 14/23] bring back selector validation inline --- .../cluster/metadata/IndexNameExpressionResolver.java | 8 +++++++- .../cluster/metadata/SelectorResolverTests.java | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 35664ba9e5ef3..c82d401b3c3df 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2365,7 +2365,13 @@ private static V splitSelectorExpression(String expression, BiFunction= 0) { String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length()); - doValidateSelectorString(() -> expression, suffix); + IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix); + if (selector == null) { + throw new InvalidIndexNameException( + expression, + "invalid usage of :: separator, [" + suffix + "] is not a recognized selector" + ); + } String expressionBase = expression.substring(0, lastDoubleColon); ensureNoMoreSelectorSeparators(expressionBase, expression); ensureNoCrossClusterExpressionWithFailuresSelector(expressionBase, selector, expression); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java index 99f67d3ff57a7..5c64d181f95e7 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java @@ -98,6 +98,10 @@ public void testResolveExpression() { assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:*::failures"); // even with an empty index name assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:index::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:index::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:failures::failures"); + assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:data::failures"); } public void assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(Context context, String expression) { From fd1a0e51da4a29dde55aed96d55e7ac9d89f2a59 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 25 Mar 2025 13:23:33 +0100 Subject: [PATCH 15/23] refactor to reuse selector validation --- .../metadata/IndexNameExpressionResolver.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index c82d401b3c3df..8c89f00884894 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2365,13 +2365,7 @@ private static V splitSelectorExpression(String expression, BiFunction= 0) { String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length()); - IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix); - if (selector == null) { - throw new InvalidIndexNameException( - expression, - "invalid usage of :: separator, [" + suffix + "] is not a recognized selector" - ); - } + IndexComponentSelector selector = resolveAndValidateSelectorString(() -> expression, suffix); String expressionBase = expression.substring(0, lastDoubleColon); ensureNoMoreSelectorSeparators(expressionBase, expression); ensureNoCrossClusterExpressionWithFailuresSelector(expressionBase, selector, expression); @@ -2382,10 +2376,10 @@ private static V splitSelectorExpression(String expression, BiFunction indexName + SELECTOR_SEPARATOR + suffix, suffix); + resolveAndValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix); } - private static void doValidateSelectorString(Supplier expression, String suffix) { + private static IndexComponentSelector resolveAndValidateSelectorString(Supplier expression, String suffix) { IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix); if (selector == null) { throw new InvalidIndexNameException( @@ -2393,6 +2387,7 @@ private static void doValidateSelectorString(Supplier expression, String "invalid usage of :: separator, [" + suffix + "] is not a recognized selector" ); } + return selector; } /** From 1bd03a6e02ab5a95ab9d61b529bfe5f50ac91e22 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 25 Mar 2025 15:22:27 +0100 Subject: [PATCH 16/23] more tests --- ...moteClusterSecurityFailureStoreRestIT.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index b4265d2b9e3ad..732b7899a0609 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -11,6 +11,7 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.Tuple; import org.elasticsearch.search.SearchHit; @@ -24,6 +25,7 @@ import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; abstract class AbstractRemoteClusterSecurityFailureStoreRestIT extends AbstractRemoteClusterSecurityTestCase { @@ -101,6 +103,39 @@ protected void setupTestDataStreamOnFulfillingCluster() throws IOException { "email" : "jack@example.com" }"""); assertOK(performRequestAgainstFulfillingCluster(createDoc2)); + { + final Request otherTemplate = new Request("PUT", "/_index_template/other_template"); + otherTemplate.setJsonEntity(""" + { + "index_patterns": ["other*"], + "data_stream": {}, + "priority": 500, + "composed_of": ["component1"] + }"""); + assertOK(performRequestAgainstFulfillingCluster(otherTemplate)); + } + { + final Request createOtherDoc3 = new Request("PUT", "/other1/_doc/3?refresh=true&op_type=create"); + createOtherDoc3.setJsonEntity(""" + { + "@timestamp": 3, + "age" : 3, + "name" : "jane", + "email" : "jane@example.com" + }"""); + assertOK(performRequestAgainstFulfillingCluster(createOtherDoc3)); + } + { + final Request createOtherDoc4 = new Request("PUT", "/other1/_doc/4?refresh=true&op_type=create"); + createOtherDoc4.setJsonEntity(""" + { + "@timestamp": 4, + "age" : "this should be an int", + "name" : "jane", + "email" : "jane@example.com" + }"""); + assertOK(performRequestAgainstFulfillingCluster(createOtherDoc4)); + } } protected Response performRequestWithRemoteSearchUser(final Request request) throws IOException { @@ -136,4 +171,9 @@ protected Tuple getSingleDataAndFailureIndices(String dataStream return new Tuple<>(indices.v1().get(0), indices.v2().get(0)); } + protected static void assertFailuresSelectorNotSupported(ResponseException exception) { + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + } + } From 0dd4053c3d07fb6c7d5f71013a7787e6c7d4a438 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 25 Mar 2025 15:22:38 +0100 Subject: [PATCH 17/23] more tests 2 --- ...ClusterSecurityRCS1FailureStoreRestIT.java | 123 +++++++++++++----- ...ClusterSecurityRCS2FailureStoreRestIT.java | 3 +- 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 267958be91fb7..b752e41b2f22d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -54,6 +54,8 @@ public class RemoteClusterSecurityRCS1FailureStoreRestIT extends AbstractRemoteC private static final String ALL_ACCESS = "all_access"; private static final String DATA_ACCESS = "data_access"; private static final String FAILURE_STORE_ACCESS = "failure_store_access"; + private static final String MANAGE_FAILURE_STORE_ACCESS = "manage_failure_store_access"; + private static final String READ_FAILURE_STORE_ACCESS = "read_failure_store_access"; public void testRCS1CrossClusterSearch() throws Exception { final boolean rcs1Security = true; @@ -106,14 +108,13 @@ public void testRCS1CrossClusterSearch() throws Exception { String.format( Locale.ROOT, "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=true", - randomFrom("test1::failures", "test*::failures", "*::failures"), + randomFrom("test1::failures", "test*::failures", "*::failures", "other1::failures", "non-existing::failures"), ccsMinimizeRoundtrips ) ) ) ); - assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + assertFailuresSelectorNotSupported(exception); } { // direct access to backing failure index is subject to the user's permissions @@ -137,25 +138,8 @@ public void testRCS1CrossClusterSearch() throws Exception { ResponseException.class, () -> performRequestWithUser(DATA_ACCESS, failureIndexSearchRequest) ); - assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat( - exception.getMessage(), - containsString( - "action [" - + (ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards") - + "] is unauthorized for user [" - + DATA_ACCESS - + "] " - + "with effective roles [" - + DATA_ACCESS - + "] on indices [" - + backingFailureIndexName - + "], this action is granted by the index privileges [" - + (ccsMinimizeRoundtrips ? "read,all" : "view_index_metadata,manage,read_cross_cluster,all") - + "]" - - ) - ); + final String action = ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards"; + assertActionUnauthorized(exception, DATA_ACCESS, action, backingFailureIndexName); } // for user with access to failure store, it depends on the underlying action that is being sent to the remote cluster @@ -179,21 +163,37 @@ public void testRCS1CrossClusterSearch() throws Exception { ResponseException.class, () -> performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest) ); - assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertActionUnauthorized(exception, FAILURE_STORE_ACCESS, "indices:admin/search/search_shards", backingFailureIndexName); + } + + // user with manage failure store access should be able to search the backing failure index + assertSearchResponseContainsIndices( + performRequestWithUser(MANAGE_FAILURE_STORE_ACCESS, failureIndexSearchRequest), + backingFailureIndexName + ); + + { + // user with only read_failure_store only access should not be able to resolve remote clusters + var exc = expectThrows( + ResponseException.class, + () -> performRequestWithUser(READ_FAILURE_STORE_ACCESS, new Request("GET", "/_resolve/cluster/" + REMOTE_CLUSTER_ALIAS)) + ); + assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(403)); assertThat( - exception.getMessage(), + exc.getMessage(), containsString( - "action [indices:admin/search/search_shards] is unauthorized for user [" - + FAILURE_STORE_ACCESS + "action [" + + "indices:admin/resolve/cluster" + + "] is unauthorized for user [" + + READ_FAILURE_STORE_ACCESS + "] " + "with effective roles [" - + FAILURE_STORE_ACCESS - + "] on indices [" - + backingFailureIndexName - + "], this action is granted by the index privileges [view_index_metadata,manage,read_cross_cluster,all]" + + READ_FAILURE_STORE_ACCESS + + "]" ) ); } + } } @@ -237,6 +237,22 @@ private static void setupUserAndRoleOnQueryCluster() throws IOException { ] }"""); createUser(adminClient(), DATA_ACCESS, PASS, DATA_ACCESS); + createRole(adminClient(), MANAGE_FAILURE_STORE_ACCESS, """ + { + "indices": [ + { + "names": ["local_index"], + "privileges": ["read"] + } + ] + }"""); + createUser(adminClient(), MANAGE_FAILURE_STORE_ACCESS, PASS, MANAGE_FAILURE_STORE_ACCESS); + + createRole(adminClient(), READ_FAILURE_STORE_ACCESS, """ + { + + }"""); + createUser(adminClient(), READ_FAILURE_STORE_ACCESS, PASS, READ_FAILURE_STORE_ACCESS); } private static void createRole(RestClient client, String role, String roleDescriptor) throws IOException { @@ -271,13 +287,24 @@ private static void setupRoleAndUserOnFulfillingCluster() throws IOException { { "indices": [ { - "names": ["test*"], + "names": ["test*", "non-existing-index"], "privileges": ["read", "read_cross_cluster", "read_failure_store"] } ] }"""); putUserOnFulfillingCluster(FAILURE_STORE_ACCESS, FAILURE_STORE_ACCESS); + putRoleOnFulfillingCluster(MANAGE_FAILURE_STORE_ACCESS, """ + { + "indices": [ + { + "names": ["test*", "non-existing-index"], + "privileges": ["manage_failure_store", "read_cross_cluster", "read_failure_store"] + } + ] + }"""); + putUserOnFulfillingCluster(MANAGE_FAILURE_STORE_ACCESS, MANAGE_FAILURE_STORE_ACCESS); + putRoleOnFulfillingCluster(ALL_ACCESS, """ { "indices": [ @@ -288,6 +315,17 @@ private static void setupRoleAndUserOnFulfillingCluster() throws IOException { ] }"""); putUserOnFulfillingCluster(ALL_ACCESS, ALL_ACCESS); + + putRoleOnFulfillingCluster(READ_FAILURE_STORE_ACCESS, """ + { + "indices": [ + { + "names": ["test*", "non-existing-index"], + "privileges": ["read_failure_store"] + } + ] + }"""); + putUserOnFulfillingCluster(READ_FAILURE_STORE_ACCESS, READ_FAILURE_STORE_ACCESS); } private static void putRoleOnFulfillingCluster(String roleName, String roleDescriptor) throws IOException { @@ -306,4 +344,27 @@ private static void putUserOnFulfillingCluster(String user, String role) throws assertOK(performRequestAgainstFulfillingCluster(request)); } + private static void assertActionUnauthorized( + ResponseException exception, + String userAndRole, + String action, + String backingFailureIndexName + ) { + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat( + exception.getMessage(), + containsString( + "action [" + + action + + "] is unauthorized for user [" + + userAndRole + + "] " + + "with effective roles [" + + userAndRole + + "] on indices [" + + backingFailureIndexName + + "]" + ) + ); + } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index 9b6176292ca4b..95c07bf93ef7b 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -123,8 +123,7 @@ public void testRCS2CrossClusterSearch() throws Exception { ) ) ); - assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + assertFailuresSelectorNotSupported(exception); } { // direct access to backing failure index is not allowed - no explicit read privileges over .fs-* indices From f085835f94fa7c401e325234d50f7124fc54cf8e Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Tue, 25 Mar 2025 19:44:52 +0100 Subject: [PATCH 18/23] more test coverage --- ...moteClusterSecurityFailureStoreRestIT.java | 1 - ...ClusterSecurityRCS1FailureStoreRestIT.java | 342 +++++++++++++----- 2 files changed, 257 insertions(+), 86 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index 732b7899a0609..007b8e091de9d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -155,7 +155,6 @@ protected Tuple, List> getDataAndFailureIndices(String data Request dataStream = new Request("GET", "/_data_stream/" + dataStreamName); Response response = performRequestAgainstFulfillingCluster(dataStream); Map dataStreams = entityAsMap(response); - assertEquals(Collections.singletonList("test1"), XContentMapValues.extractValue("data_streams.name", dataStreams)); List dataIndexNames = (List) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams); List failureIndexNames = (List) XContentMapValues.extractValue( "data_streams.failure_store.indices.index_name", diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index b752e41b2f22d..bbd90a7d58177 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -55,7 +55,9 @@ public class RemoteClusterSecurityRCS1FailureStoreRestIT extends AbstractRemoteC private static final String DATA_ACCESS = "data_access"; private static final String FAILURE_STORE_ACCESS = "failure_store_access"; private static final String MANAGE_FAILURE_STORE_ACCESS = "manage_failure_store_access"; - private static final String READ_FAILURE_STORE_ACCESS = "read_failure_store_access"; + private static final String ONLY_READ_FAILURE_STORE_ACCESS = "only_read_failure_store_access"; + private static final String BACKING_FAILURE_STORE_INDEX_ACCESS = "backing_failure_store_index_access"; + private static final String BACKING_DATA_INDEX_ACCESS = "backing_data_index_access"; public void testRCS1CrossClusterSearch() throws Exception { final boolean rcs1Security = true; @@ -77,7 +79,193 @@ public void testRCS1CrossClusterSearch() throws Exception { final String backingDataIndexName = backingIndices.v1(); final String backingFailureIndexName = backingIndices.v2(); - final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS, ALL_ACCESS }; + final Tuple otherBackingIndices = getSingleDataAndFailureIndices("other1"); + final String otherBackingDataIndexName = otherBackingIndices.v1(); + final String otherBackingFailureIndexName = otherBackingIndices.v2(); + + testCcsWithDataSelectorSupported(backingDataIndexName, ccsMinimizeRoundtrips); + testCcsWithFailuresSelectorNotSupported(ccsMinimizeRoundtrips); + + testSearchingUnauthorizedIndices(otherBackingFailureIndexName, otherBackingDataIndexName, ccsMinimizeRoundtrips); + testSearchingWithAccessToAllIndices(ccsMinimizeRoundtrips, backingDataIndexName, otherBackingDataIndexName); + testBackingFailureIndexAccess(ccsMinimizeRoundtrips, backingFailureIndexName); + testBackingDataIndexAccess(ccsMinimizeRoundtrips, backingDataIndexName); + testSearchingNonExistingIndices(ccsMinimizeRoundtrips); + testResolveRemoteClustersIsUnauthorized(); + } + + private void testBackingDataIndexAccess(boolean ccsMinimizeRoundtrips, String backingDataIndexName) throws IOException { + Request dataIndexSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + backingDataIndexName, + ccsMinimizeRoundtrips + ) + ); + assertSearchResponseContainsIndices( + performRequestWithUser(BACKING_DATA_INDEX_ACCESS, dataIndexSearchRequest), + backingDataIndexName + ); + } + + private void testSearchingWithAccessToAllIndices( + boolean ccsMinimizeRoundtrips, + String backingDataIndexName, + String otherBackingDataIndexName + ) throws IOException { + final boolean alsoSearchLocally = randomBoolean(); + final Request dataSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", + alsoSearchLocally ? "local_index," : "", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("*", "*::data"), + ccsMinimizeRoundtrips + ) + ); + final String[] expectedIndices = alsoSearchLocally + ? new String[] { "local_index", backingDataIndexName, otherBackingDataIndexName } + : new String[] { backingDataIndexName, otherBackingDataIndexName }; + assertSearchResponseContainsIndices(performRequestWithUser(ALL_ACCESS, dataSearchRequest), expectedIndices); + } + + private void testSearchingNonExistingIndices(boolean ccsMinimizeRoundtrips) { + // searching non-existing index without permissions should result in 403 + { + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser( + FAILURE_STORE_ACCESS, + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + "non-existing-no-privileges", + ccsMinimizeRoundtrips + ) + ) + ) + ); + final String action = ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards"; + assertActionUnauthorized(exception, FAILURE_STORE_ACCESS, action, "non-existing-no-privileges"); + } + // searching non-existing index with permissions should result in 404 + { + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser( + FAILURE_STORE_ACCESS, + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + "non-existing-index", + ccsMinimizeRoundtrips + ) + ) + ) + ); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + } + } + + private void testSearchingUnauthorizedIndices( + String otherBackingFailureIndexName, + String otherBackingDataIndexName, + boolean ccsMinimizeRoundtrips + ) { + // try searching remote index for which user has no access + final String indexToSearch = randomFrom("other1", "other1::data", otherBackingFailureIndexName, otherBackingDataIndexName); + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser( + FAILURE_STORE_ACCESS, + new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + indexToSearch, + ccsMinimizeRoundtrips + ) + ) + ) + ); + final String action = ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards"; + assertActionUnauthorized(exception, FAILURE_STORE_ACCESS, action, indexToSearch); + } + + private void testBackingFailureIndexAccess(boolean ccsMinimizeRoundtrips, String backingFailureIndexName) throws IOException { + // direct access to backing failure index is subject to the user's permissions + // it might fail in some cases and work in others + Request failureIndexSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", + backingFailureIndexName, + ccsMinimizeRoundtrips + ) + ); + + // user with access to all should be able to search the backing failure index + assertSearchResponseContainsIndices(performRequestWithUser(ALL_ACCESS, failureIndexSearchRequest), backingFailureIndexName); + + // user with data only access should not be able to search the backing failure index + { + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser(DATA_ACCESS, failureIndexSearchRequest) + ); + final String action = ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards"; + assertActionUnauthorized(exception, DATA_ACCESS, action, backingFailureIndexName); + } + + // for user with access to failure store, it depends on the underlying action that is being sent to the remote cluster + if (ccsMinimizeRoundtrips) { + // this is a special case where indices:data/read/search will be sent to a remote cluster + // and the request to backing failure store index will be authorized based on the datastream + // which grants access to backing failure store indices (granted by read_failure_store privilege) + // from a security perspective, this is a valid use case and there is no way to prevent this with RCS1 security model + // since from the fulfilling cluster perspective this request is no different from any other local search request + assertSearchResponseContainsIndices( + performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest), + backingFailureIndexName + ); + } else { + // in this case, the user does not have the necessary permissions to search the backing failure index + // the request to failure store backing index is authorized based on the datastream + // which does not grant access to the indices:admin/search/search_shards action + // this action is granted by read_cross_cluster privilege which is currently + // not supporting the failure backing indices (only data backing indices) + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest) + ); + assertActionUnauthorized(exception, FAILURE_STORE_ACCESS, "indices:admin/search/search_shards", backingFailureIndexName); + } + + // user with manage failure store access should be able to search the backing failure index + assertSearchResponseContainsIndices( + performRequestWithUser(MANAGE_FAILURE_STORE_ACCESS, failureIndexSearchRequest), + backingFailureIndexName + ); + + assertSearchResponseContainsIndices( + performRequestWithUser(BACKING_FAILURE_STORE_INDEX_ACCESS, failureIndexSearchRequest), + backingFailureIndexName + ); + + } + + private void testCcsWithDataSelectorSupported(String backingDataIndexName, boolean ccsMinimizeRoundtrips) throws IOException { + final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS }; for (String user : users) { // query remote cluster using ::data selector should succeed final boolean alsoSearchLocally = randomBoolean(); @@ -97,6 +285,17 @@ public void testRCS1CrossClusterSearch() throws Exception { : new String[] { backingDataIndexName }; assertSearchResponseContainsIndices(performRequestWithUser(user, dataSearchRequest), expectedIndices); } + } + + private void testCcsWithFailuresSelectorNotSupported(boolean ccsMinimizeRoundtrips) { + final String[] users = { + FAILURE_STORE_ACCESS, + DATA_ACCESS, + ALL_ACCESS, + MANAGE_FAILURE_STORE_ACCESS, + BACKING_DATA_INDEX_ACCESS, + BACKING_FAILURE_STORE_INDEX_ACCESS, + ONLY_READ_FAILURE_STORE_ACCESS }; for (String user : users) { // query remote cluster using ::failures selector should fail (regardless of the user's permissions) final ResponseException exception = expectThrows( @@ -116,85 +315,28 @@ public void testRCS1CrossClusterSearch() throws Exception { ); assertFailuresSelectorNotSupported(exception); } - { - // direct access to backing failure index is subject to the user's permissions - // it might fail in some cases and work in others - Request failureIndexSearchRequest = new Request( - "GET", - String.format( - Locale.ROOT, - "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", - backingFailureIndexName, - ccsMinimizeRoundtrips - ) - ); - - // user with access to all should be able to search the backing failure index - assertSearchResponseContainsIndices(performRequestWithUser(ALL_ACCESS, failureIndexSearchRequest), backingFailureIndexName); - - // user with data only access should not be able to search the backing failure index - { - final ResponseException exception = expectThrows( - ResponseException.class, - () -> performRequestWithUser(DATA_ACCESS, failureIndexSearchRequest) - ); - final String action = ccsMinimizeRoundtrips ? "indices:data/read/search" : "indices:admin/search/search_shards"; - assertActionUnauthorized(exception, DATA_ACCESS, action, backingFailureIndexName); - } - - // for user with access to failure store, it depends on the underlying action that is being sent to the remote cluster - if (ccsMinimizeRoundtrips) { - // this is a special case where indices:data/read/search will be sent to a remote cluster - // and the request to backing failure store index will be authorized based on the datastream - // which grants access to backing failure store indices (granted by read_failure_store privilege) - // from a security perspective, this is a valid use case and there is no way to prevent this with RCS1 security model - // since from the fulfilling cluster perspective this request is no different from any other local search request - assertSearchResponseContainsIndices( - performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest), - backingFailureIndexName - ); - } else { - // in this case, the user does not have the necessary permissions to search the backing failure index - // the request to failure store backing index is authorized based on the datastream - // which does not grant access to the indices:admin/search/search_shards action - // this action is granted by read_cross_cluster privilege which is currently - // not supporting the failure backing indices (only data backing indices) - final ResponseException exception = expectThrows( - ResponseException.class, - () -> performRequestWithUser(FAILURE_STORE_ACCESS, failureIndexSearchRequest) - ); - assertActionUnauthorized(exception, FAILURE_STORE_ACCESS, "indices:admin/search/search_shards", backingFailureIndexName); - } - - // user with manage failure store access should be able to search the backing failure index - assertSearchResponseContainsIndices( - performRequestWithUser(MANAGE_FAILURE_STORE_ACCESS, failureIndexSearchRequest), - backingFailureIndexName - ); - - { - // user with only read_failure_store only access should not be able to resolve remote clusters - var exc = expectThrows( - ResponseException.class, - () -> performRequestWithUser(READ_FAILURE_STORE_ACCESS, new Request("GET", "/_resolve/cluster/" + REMOTE_CLUSTER_ALIAS)) - ); - assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat( - exc.getMessage(), - containsString( - "action [" - + "indices:admin/resolve/cluster" - + "] is unauthorized for user [" - + READ_FAILURE_STORE_ACCESS - + "] " - + "with effective roles [" - + READ_FAILURE_STORE_ACCESS - + "]" - ) - ); - } + } - } + private void testResolveRemoteClustersIsUnauthorized() { + // user with only read_failure_store access should not be able to resolve remote clusters + var exc = expectThrows( + ResponseException.class, + () -> performRequestWithUser(ONLY_READ_FAILURE_STORE_ACCESS, new Request("GET", "/_resolve/cluster/" + REMOTE_CLUSTER_ALIAS)) + ); + assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + assertThat( + exc.getMessage(), + containsString( + "action [" + + "indices:admin/resolve/cluster" + + "] is unauthorized for user [" + + ONLY_READ_FAILURE_STORE_ACCESS + + "] " + + "with effective roles [" + + ONLY_READ_FAILURE_STORE_ACCESS + + "]" + ) + ); } private static void setupLocalDataOnQueryCluster() throws IOException { @@ -248,11 +390,19 @@ private static void setupUserAndRoleOnQueryCluster() throws IOException { }"""); createUser(adminClient(), MANAGE_FAILURE_STORE_ACCESS, PASS, MANAGE_FAILURE_STORE_ACCESS); - createRole(adminClient(), READ_FAILURE_STORE_ACCESS, """ + createRole(adminClient(), ONLY_READ_FAILURE_STORE_ACCESS, """ + { + }"""); + createUser(adminClient(), ONLY_READ_FAILURE_STORE_ACCESS, PASS, ONLY_READ_FAILURE_STORE_ACCESS); + createRole(adminClient(), BACKING_FAILURE_STORE_INDEX_ACCESS, """ { - }"""); - createUser(adminClient(), READ_FAILURE_STORE_ACCESS, PASS, READ_FAILURE_STORE_ACCESS); + createUser(adminClient(), BACKING_FAILURE_STORE_INDEX_ACCESS, PASS, BACKING_FAILURE_STORE_INDEX_ACCESS); + createRole(adminClient(), BACKING_DATA_INDEX_ACCESS, """ + { + }"""); + createUser(adminClient(), BACKING_DATA_INDEX_ACCESS, PASS, BACKING_DATA_INDEX_ACCESS); + } private static void createRole(RestClient client, String role, String roleDescriptor) throws IOException { @@ -316,7 +466,7 @@ private static void setupRoleAndUserOnFulfillingCluster() throws IOException { }"""); putUserOnFulfillingCluster(ALL_ACCESS, ALL_ACCESS); - putRoleOnFulfillingCluster(READ_FAILURE_STORE_ACCESS, """ + putRoleOnFulfillingCluster(ONLY_READ_FAILURE_STORE_ACCESS, """ { "indices": [ { @@ -325,7 +475,29 @@ private static void setupRoleAndUserOnFulfillingCluster() throws IOException { } ] }"""); - putUserOnFulfillingCluster(READ_FAILURE_STORE_ACCESS, READ_FAILURE_STORE_ACCESS); + putUserOnFulfillingCluster(ONLY_READ_FAILURE_STORE_ACCESS, ONLY_READ_FAILURE_STORE_ACCESS); + + putRoleOnFulfillingCluster(BACKING_DATA_INDEX_ACCESS, """ + { + "indices": [ + { + "names": [".ds-test*"], + "privileges": ["read", "read_cross_cluster"] + } + ] + }"""); + putUserOnFulfillingCluster(BACKING_DATA_INDEX_ACCESS, BACKING_DATA_INDEX_ACCESS); + + putRoleOnFulfillingCluster(BACKING_FAILURE_STORE_INDEX_ACCESS, """ + { + "indices": [ + { + "names": [".fs-test*"], + "privileges": ["read", "read_cross_cluster"] + } + ] + }"""); + putUserOnFulfillingCluster(BACKING_FAILURE_STORE_INDEX_ACCESS, BACKING_FAILURE_STORE_INDEX_ACCESS); } private static void putRoleOnFulfillingCluster(String roleName, String roleDescriptor) throws IOException { From 62664ac940e358f1008090b53e5b8a193ad01533 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 25 Mar 2025 18:54:55 +0000 Subject: [PATCH 19/23] [CI] Auto commit changes from spotless --- .../AbstractRemoteClusterSecurityFailureStoreRestIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index 007b8e091de9d..d71945306b4af 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Collectors; From 0fe3ec54fe5e402af1fbde505a69bae5b8512167 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 26 Mar 2025 15:04:16 +0100 Subject: [PATCH 20/23] prevent using ::data selector as well --- .../metadata/IndexNameExpressionResolver.java | 8 +-- .../metadata/SelectorResolverTests.java | 59 +++++++++++-------- ...moteClusterSecurityFailureStoreRestIT.java | 4 +- ...ClusterSecurityRCS1FailureStoreRestIT.java | 34 +++++++++-- ...ClusterSecurityRCS2FailureStoreRestIT.java | 27 +++++++-- 5 files changed, 92 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 8c89f00884894..b820cdb492f33 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2368,7 +2368,7 @@ private static V splitSelectorExpression(String expression, BiFunction expression, suffix); String expressionBase = expression.substring(0, lastDoubleColon); ensureNoMoreSelectorSeparators(expressionBase, expression); - ensureNoCrossClusterExpressionWithFailuresSelector(expressionBase, selector, expression); + ensureNoCrossClusterExpressionWithSelectorSeparator(expressionBase, selector, expression); return bindFunction.apply(expressionBase, suffix); } // Otherwise accept the default @@ -2423,12 +2423,12 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S * Checks the expression for cross-cluster syntax and throws an exception if it is combined with ::failures selector. * @throws IllegalArgumentException if cross-cluster syntax is detected after parsing the selector expression */ - private static void ensureNoCrossClusterExpressionWithFailuresSelector( + private static void ensureNoCrossClusterExpressionWithSelectorSeparator( String expressionWithoutSelector, IndexComponentSelector selector, String originalExpression ) { - if (selector == IndexComponentSelector.FAILURES) { + if (selector != null) { if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) { throw new IllegalArgumentException( "Invalid usage of [" @@ -2436,7 +2436,7 @@ private static void ensureNoCrossClusterExpressionWithFailuresSelector( + selector.getKey() + "] selector in [" + originalExpression - + "], failures selector is not supported with cross-cluster expressions" + + "], selectors are not supported with cross-cluster expressions" ); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java index 5c64d181f95e7..424937c8f099a 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java @@ -17,6 +17,8 @@ import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.test.ESTestCase; +import java.util.Set; + import static org.elasticsearch.action.support.IndexComponentSelector.DATA; import static org.elasticsearch.action.support.IndexComponentSelector.FAILURES; import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context; @@ -74,11 +76,8 @@ public void testResolveExpression() { // Empty index name is not necessarily disallowed, but will be filtered out in the next steps of resolution assertThat(resolve(selectorsAllowed, "::data"), equalTo(new ResolvedExpression("", DATA))); assertThat(resolve(selectorsAllowed, "::failures"), equalTo(new ResolvedExpression("", FAILURES))); - // Remote cluster syntax is respected, even if code higher up the call stack is likely to already have handled it already - assertThat(resolve(selectorsAllowed, "cluster:index::data"), equalTo(new ResolvedExpression("cluster:index", DATA))); - // CCS with an empty index name is not necessarily disallowed, though other code in the resolution logic will likely throw - assertThat(resolve(selectorsAllowed, "cluster:::data"), equalTo(new ResolvedExpression("cluster:", DATA))); - // Same for empty cluster and index names + // CCS with an empty index and cluster name is not necessarily disallowed, though other code in the resolution logic will likely + // throw assertThat(resolve(selectorsAllowed, ":::data"), equalTo(new ResolvedExpression(":", DATA))); assertThat(resolve(selectorsAllowed, ":::failures"), equalTo(new ResolvedExpression(":", FAILURES))); // Any more prefix colon characters will trigger the multiple separators error logic @@ -88,25 +87,37 @@ public void testResolveExpression() { // Suffix case is not supported because there is no component named with the empty string expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::")); - // remote cluster syntax is not allowed with ::failures selector - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:index::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(noSelectors, "cluster:index::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index-*::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:*::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:index-*::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:*::failures"); - // even with an empty index name - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:index::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:index::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:failures::failures"); - assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:data::failures"); - } - - public void assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(Context context, String expression) { - var e = expectThrows(IllegalArgumentException.class, () -> resolve(context, expression)); - assertThat(e.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + // remote cluster syntax is not allowed with :: selectors + final Set remoteClusterExpressionsWithSelectors = Set.of( + "cluster:index::failures", + "cluster-*:index::failures", + "cluster-*:index-*::failures", + "cluster-*:*::failures", + "*:index-*::failures", + "*:*::failures", + "*:-test*,*::failures", + "cluster:::failures", + "failures:index::failures", + "data:index::failures", + "failures:failures::failures", + "data:data::failures", + "cluster:index::data", + "cluster-*:index::data", + "cluster-*:index-*::data", + "cluster-*:*::data", + "*:index-*::data", + "*:*::data", + "cluster:::data", + "failures:index::data", + "data:index::data", + "failures:failures::data", + "data:data::data", + "*:-test*,*::data" + ); + for (String expression : remoteClusterExpressionsWithSelectors) { + var e = expectThrows(IllegalArgumentException.class, () -> resolve(selectorsAllowed, expression)); + assertThat(e.getMessage(), containsString("selectors are not supported with cross-cluster expressions")); + } } public void testResolveMatchAllToSelectors() { diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index d71945306b4af..c56be94455343 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -169,9 +169,9 @@ protected Tuple getSingleDataAndFailureIndices(String dataStream return new Tuple<>(indices.v1().get(0), indices.v2().get(0)); } - protected static void assertFailuresSelectorNotSupported(ResponseException exception) { + protected static void assertSelectorsNotSupported(ResponseException exception) { assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions")); + assertThat(exception.getMessage(), containsString("selectors are not supported with cross-cluster expressions")); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index bbd90a7d58177..685ada5964a16 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -83,9 +83,9 @@ public void testRCS1CrossClusterSearch() throws Exception { final String otherBackingDataIndexName = otherBackingIndices.v1(); final String otherBackingFailureIndexName = otherBackingIndices.v2(); - testCcsWithDataSelectorSupported(backingDataIndexName, ccsMinimizeRoundtrips); + testCcsWithDataSelectorNotSupported(ccsMinimizeRoundtrips); testCcsWithFailuresSelectorNotSupported(ccsMinimizeRoundtrips); - + testCcsWithoutSelectorsSupported(backingDataIndexName, ccsMinimizeRoundtrips); testSearchingUnauthorizedIndices(otherBackingFailureIndexName, otherBackingDataIndexName, ccsMinimizeRoundtrips); testSearchingWithAccessToAllIndices(ccsMinimizeRoundtrips, backingDataIndexName, otherBackingDataIndexName); testBackingFailureIndexAccess(ccsMinimizeRoundtrips, backingFailureIndexName); @@ -264,10 +264,9 @@ private void testBackingFailureIndexAccess(boolean ccsMinimizeRoundtrips, String } - private void testCcsWithDataSelectorSupported(String backingDataIndexName, boolean ccsMinimizeRoundtrips) throws IOException { + public void testCcsWithoutSelectorsSupported(String backingDataIndexName, boolean ccsMinimizeRoundtrips) throws IOException { final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS }; for (String user : users) { - // query remote cluster using ::data selector should succeed final boolean alsoSearchLocally = randomBoolean(); final Request dataSearchRequest = new Request( "GET", @@ -276,7 +275,7 @@ private void testCcsWithDataSelectorSupported(String backingDataIndexName, boole "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", alsoSearchLocally ? "local_index," : "", randomFrom("my_remote_cluster", "*", "my_remote_*"), - randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), + randomFrom("test1", "test*", "*", backingDataIndexName), ccsMinimizeRoundtrips ) ); @@ -287,6 +286,29 @@ private void testCcsWithDataSelectorSupported(String backingDataIndexName, boole } } + private void testCcsWithDataSelectorNotSupported(boolean ccsMinimizeRoundtrips) throws IOException { + final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS, ALL_ACCESS }; + for (String user : users) { + // query remote cluster using ::data selector should not succeed + final boolean alsoSearchLocally = randomBoolean(); + final Request dataSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s:%s/_search?ccs_minimize_roundtrips=%s", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("test1::data", "test*::data", "*::data", "non-existing::data"), + ccsMinimizeRoundtrips + ) + ); + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithUser(user, dataSearchRequest) + ); + assertSelectorsNotSupported(exception); + } + } + private void testCcsWithFailuresSelectorNotSupported(boolean ccsMinimizeRoundtrips) { final String[] users = { FAILURE_STORE_ACCESS, @@ -313,7 +335,7 @@ private void testCcsWithFailuresSelectorNotSupported(boolean ccsMinimizeRoundtri ) ) ); - assertFailuresSelectorNotSupported(exception); + assertSelectorsNotSupported(exception); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java index 95c07bf93ef7b..85bb4b9af4d8f 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java @@ -89,7 +89,7 @@ public void testRCS2CrossClusterSearch() throws Exception { final String backingDataIndexName = backingIndices.v1(); final String backingFailureIndexName = backingIndices.v2(); { - // query remote cluster using ::data selector should succeed + // query remote cluster without selectors should succeed final boolean alsoSearchLocally = randomBoolean(); final Request dataSearchRequest = new Request( "GET", @@ -98,7 +98,7 @@ public void testRCS2CrossClusterSearch() throws Exception { "/%s%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false", alsoSearchLocally ? "local_index," : "", randomFrom("my_remote_cluster", "*", "my_remote_*"), - randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName), + randomFrom("test1", "test*", "*", backingDataIndexName), ccsMinimizeRoundtrips ) ); @@ -107,6 +107,25 @@ public void testRCS2CrossClusterSearch() throws Exception { : new String[] { backingDataIndexName }; assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(dataSearchRequest), expectedIndices); } + { + // query remote cluster using ::data selector should fail + final boolean alsoSearchLocally = randomBoolean(); + final Request dataSearchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("test1::data", "test*::data", "*::data", "non-existing::data"), + ccsMinimizeRoundtrips + ) + ); + final ResponseException exception = expectThrows( + ResponseException.class, + () -> performRequestWithRemoteSearchUser(dataSearchRequest) + ); + assertSelectorsNotSupported(exception); + } { // query remote cluster using ::failures selector should fail final ResponseException exception = expectThrows( @@ -117,13 +136,13 @@ public void testRCS2CrossClusterSearch() throws Exception { String.format( Locale.ROOT, "/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s", - randomFrom("test1::failures", "test*::failures", "*::failures"), + randomFrom("test1::failures", "test*::failures", "*::failures", "non-existing::failures"), ccsMinimizeRoundtrips ) ) ) ); - assertFailuresSelectorNotSupported(exception); + assertSelectorsNotSupported(exception); } { // direct access to backing failure index is not allowed - no explicit read privileges over .fs-* indices From 4baa39ac870a596dddd47dacd95b6d018db19ff1 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 26 Mar 2025 22:17:18 +0100 Subject: [PATCH 21/23] consolidate error messages and exceptions thrown --- .../metadata/IndexNameExpressionResolver.java | 17 +++++------------ .../cluster/metadata/SelectorResolverTests.java | 4 ++-- .../xpack/esql/parser/StatementParserTests.java | 8 +++++++- ...RemoteClusterSecurityFailureStoreRestIT.java | 2 +- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index b820cdb492f33..97ca52332510f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -2368,7 +2368,7 @@ private static V splitSelectorExpression(String expression, BiFunction expression, suffix); String expressionBase = expression.substring(0, lastDoubleColon); ensureNoMoreSelectorSeparators(expressionBase, expression); - ensureNoCrossClusterExpressionWithSelectorSeparator(expressionBase, selector, expression); + ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(expressionBase, selector, expression); return bindFunction.apply(expressionBase, suffix); } // Otherwise accept the default @@ -2420,24 +2420,17 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S } /** - * Checks the expression for cross-cluster syntax and throws an exception if it is combined with ::failures selector. - * @throws IllegalArgumentException if cross-cluster syntax is detected after parsing the selector expression + * Checks the expression for remote cluster pattern and throws an exception if it is combined with :: selectors. + * @throws InvalidIndexNameException if remote cluster pattern is detected after parsing the selector expression */ - private static void ensureNoCrossClusterExpressionWithSelectorSeparator( + private static void ensureNotMixingRemoteClusterExpressionWithSelectorSeparator( String expressionWithoutSelector, IndexComponentSelector selector, String originalExpression ) { if (selector != null) { if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) { - throw new IllegalArgumentException( - "Invalid usage of [" - + SELECTOR_SEPARATOR - + selector.getKey() - + "] selector in [" - + originalExpression - + "], selectors are not supported with cross-cluster expressions" - ); + throw new InvalidIndexNameException(originalExpression, "Selectors are not yet supported on remote cluster patterns"); } } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java index 424937c8f099a..aa5d3a59cc609 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SelectorResolverTests.java @@ -115,8 +115,8 @@ public void testResolveExpression() { "*:-test*,*::data" ); for (String expression : remoteClusterExpressionsWithSelectors) { - var e = expectThrows(IllegalArgumentException.class, () -> resolve(selectorsAllowed, expression)); - assertThat(e.getMessage(), containsString("selectors are not supported with cross-cluster expressions")); + var e = expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, expression)); + assertThat(e.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns")); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index 561b260114358..e60234d5ffbf7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -636,7 +636,13 @@ public void testInvalidCharacterInIndexPattern() { expectDoubleColonErrorWithLineNumber(command, "*:*::failures", parseLineNumber + 3); // Too many colons - expectInvalidIndexNameErrorWithLineNumber(command, "\"index:::data\"", lineNumber, "index:", "must not contain ':'"); + expectInvalidIndexNameErrorWithLineNumber( + command, + "\"index:::data\"", + lineNumber, + "index:::data", + "Selectors are not yet supported on remote cluster patterns" + ); expectInvalidIndexNameErrorWithLineNumber( command, "\"index::::data\"", diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java index c56be94455343..91c6919a3a5fc 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java @@ -171,7 +171,7 @@ protected Tuple getSingleDataAndFailureIndices(String dataStream protected static void assertSelectorsNotSupported(ResponseException exception) { assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403)); - assertThat(exception.getMessage(), containsString("selectors are not supported with cross-cluster expressions")); + assertThat(exception.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns")); } } From 581120055583662ed4fa0b8b3c604114488cfa8a Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 26 Mar 2025 22:20:16 +0100 Subject: [PATCH 22/23] fix failing test --- .../RemoteClusterSecurityRCS1FailureStoreRestIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 685ada5964a16..28eb8bcbabca1 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -123,7 +123,7 @@ private void testSearchingWithAccessToAllIndices( "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", alsoSearchLocally ? "local_index," : "", randomFrom("my_remote_cluster", "*", "my_remote_*"), - randomFrom("*", "*::data"), + "*", ccsMinimizeRoundtrips ) ); From 7d1dd3e2f02e2f4e6ba4ce84d65a859a19a5918d Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 27 Mar 2025 08:57:18 +0100 Subject: [PATCH 23/23] fix failing test ::data is not allowed --- .../RemoteClusterSecurityRCS1FailureStoreRestIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java index 28eb8bcbabca1..8f6fce383f00d 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java @@ -181,7 +181,7 @@ private void testSearchingUnauthorizedIndices( boolean ccsMinimizeRoundtrips ) { // try searching remote index for which user has no access - final String indexToSearch = randomFrom("other1", "other1::data", otherBackingFailureIndexName, otherBackingDataIndexName); + final String indexToSearch = randomFrom("other1", otherBackingFailureIndexName, otherBackingDataIndexName); final ResponseException exception = expectThrows( ResponseException.class, () -> performRequestWithUser(