From 0add5ecb92bb88a39146f0352b618aaf1120ad32 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 22 Nov 2023 19:26:43 +0000 Subject: [PATCH] Shrink operation privileges evaluation (#3716) ### Description Bug fix. Shrink, or resize operations weren't properly evaluated. More in the task: https://github.com/opensearch-project/security/issues/2141 ### Issues Resolved https://github.com/opensearch-project/security/issues/2141 Is this a backport? If so, please add backport PR # and/or commits # ### Testing [Please provide details of testing done: unit testing, integration testing and manual testing] ### Check List - [x] New functionality includes testing - [x] New functionality has been documented - [x] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Maciej Mierzwa (cherry picked from commit 3c01fdea5f91286e908cd9c150a1a0edfa09a0ce) Signed-off-by: github-actions[bot] --- .../security/SearchOperationTest.java | 68 ++++++++++++------- .../resolver/IndexResolverReplacer.java | 5 ++ 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java index fa99a2fdc7..651ddc2303 100644 --- a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java @@ -28,6 +28,8 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; +import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; import org.opensearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.opensearch.action.admin.cluster.repositories.put.PutRepositoryRequest; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; @@ -83,6 +85,7 @@ import org.opensearch.client.indices.PutMappingRequest; import org.opensearch.client.indices.ResizeRequest; import org.opensearch.client.indices.ResizeResponse; +import org.opensearch.cluster.health.ClusterHealthStatus; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexTemplateMetadata; import org.opensearch.common.settings.Settings; @@ -119,6 +122,7 @@ import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.client.RequestOptions.DEFAULT; import static org.opensearch.core.rest.RestStatus.ACCEPTED; +import static org.opensearch.core.rest.RestStatus.BAD_REQUEST; import static org.opensearch.core.rest.RestStatus.FORBIDDEN; import static org.opensearch.core.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -335,22 +339,24 @@ public class SearchOperationTest { * indices with names prefixed by the {@link #INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX} */ static final User USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES = new User("index-operation-tester").roles( - new Role("index-manager").indexPermissions( - "indices:admin/create", - "indices:admin/get", - "indices:admin/delete", - "indices:admin/close", - "indices:admin/close*", - "indices:admin/open", - "indices:admin/resize", - "indices:monitor/stats", - "indices:monitor/settings/get", - "indices:admin/settings/update", - "indices:admin/mapping/put", - "indices:admin/mappings/get", - "indices:admin/cache/clear", - "indices:admin/aliases" - ).on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*")) + new Role("index-manager").clusterPermissions("cluster:monitor/health") + .indexPermissions( + "indices:admin/create", + "indices:admin/get", + "indices:admin/delete", + "indices:admin/close", + "indices:admin/close*", + "indices:admin/open", + "indices:admin/resize", + "indices:monitor/stats", + "indices:monitor/settings/get", + "indices:admin/settings/update", + "indices:admin/mapping/put", + "indices:admin/mappings/get", + "indices:admin/cache/clear", + "indices:admin/aliases" + ) + .on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*")) ); private static User USER_ALLOWED_TO_CREATE_INDEX = new User("user-allowed-to-create-index").roles( @@ -2274,14 +2280,15 @@ public void openIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void shrinkIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_source"); - Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).put("index.number_of_shards", 2).build(); String targetIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_target"); + Settings sourceIndexSettings = Settings.builder() + .put("index.number_of_replicas", 1) + .put("index.blocks.write", true) + .put("index.number_of_shards", 4) + .build(); IndexOperationsHelper.createIndex(cluster, sourceIndexName, sourceIndexSettings); try ( @@ -2289,6 +2296,17 @@ public void shrinkIndex_positive() throws IOException { USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES ) ) { + ClusterHealthResponse healthResponse = restHighLevelClient.cluster() + .health( + new ClusterHealthRequest(sourceIndexName).waitForNoRelocatingShards(true) + .waitForActiveShards(4) + .waitForNoInitializingShards(true) + .waitForGreenStatus(), + DEFAULT + ); + + assertThat(healthResponse.getStatus(), is(ClusterHealthStatus.GREEN)); + ResizeRequest resizeRequest = new ResizeRequest(targetIndexName, sourceIndexName); ResizeResponse response = restHighLevelClient.indices().shrink(resizeRequest, DEFAULT); @@ -2331,10 +2349,7 @@ public void shrinkIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void cloneIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("clone_index_positive_source"); Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build(); @@ -2351,6 +2366,10 @@ public void cloneIndex_positive() throws IOException { assertThat(response, isSuccessfulResizeResponse(targetIndexName)); assertThat(cluster, indexExists(targetIndexName)); + + // can't clone the same index twice, target already exists + ResizeRequest repeatResizeRequest = new ResizeRequest(targetIndexName, sourceIndexName); + assertThatThrownBy(() -> restHighLevelClient.indices().clone(repeatResizeRequest, DEFAULT), statusException(BAD_REQUEST)); } } @@ -2388,10 +2407,7 @@ public void cloneIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void splitIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("split_index_positive_source"); Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build(); diff --git a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java index 3ebfbce29b..d7aeb776c7 100644 --- a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java @@ -56,6 +56,7 @@ import org.opensearch.action.admin.indices.datastream.CreateDataStreamAction; import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest; import org.opensearch.action.admin.indices.resolve.ResolveIndexAction; +import org.opensearch.action.admin.indices.shrink.ResizeRequest; import org.opensearch.action.admin.indices.template.put.PutComponentTemplateAction; import org.opensearch.action.bulk.BulkRequest; import org.opensearch.action.bulk.BulkShardRequest; @@ -777,6 +778,10 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid return false; } ((CreateIndexRequest) request).index(newIndices.length != 1 ? null : newIndices[0]); + } else if (request instanceof ResizeRequest) { + // clone or shrink operations + provider.provide(((ResizeRequest) request).indices(), request, true); + provider.provide(((ResizeRequest) request).getTargetIndexRequest().indices(), request, true); } else if (request instanceof CreateDataStreamAction.Request) { provider.provide(((CreateDataStreamAction.Request) request).indices(), request, false); } else if (request instanceof ReindexRequest) {