Shrink operation privileges evaluation (opensearch-project#3716)

### Description Bug fix. Shrink, or resize operations weren't properly evaluated. More in the task: opensearch-project#2141 ### Issues Resolved opensearch-project#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 <[email protected]>
cwperks · Nov 22, 2023 · 3c01fde · 3c01fde
1 parent e9d3f26
commit 3c01fde
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 26 deletions.
diff --git 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(
@@ -2272,21 +2278,33 @@ 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 (
             RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(
                 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);
 
@@ -2329,10 +2347,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();
@@ -2349,6 +2364,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));
         }
     }
 
@@ -2386,10 +2405,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
@@ -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) {