Merge branch 'main_origin' into integration-tests-windows

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();

src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java

@@ -65,6 +65,7 @@
 import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.net.URIBuilder;
+import org.apache.http.HttpHeaders;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -77,6 +78,7 @@
 import static java.util.Objects.requireNonNull;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 
 /**
@@ -284,7 +286,26 @@ public HttpResponse(CloseableHttpResponse inner) throws IllegalStateException, I
             this.header = inner.getHeaders();
             this.statusCode = inner.getCode();
             this.statusReason = inner.getReasonPhrase();
+
             inner.close();
+
+            if (this.body.length() != 0) {
+                verifyContentType();
+            }
+        }
+
+        private void verifyContentType() {
+            final String contentType = this.getHeader(HttpHeaders.CONTENT_TYPE).getValue();
+            if (contentType.contains("application/json")) {
+                assertThat("Response body format was not json, body: " + body, body.charAt(0), equalTo('{'));
+            } else {
+                assertThat(
+                    "Response body format was json, whereas content-type was " + contentType + ", body: " + body,
+                    body.charAt(0),
+                    not(equalTo('{'))
+                );
+            }
+
         }
 
         public String getContentType() {

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) {

src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java

@@ -14,6 +14,7 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.function.Supplier;
 
 import com.google.common.collect.Lists;
@@ -26,7 +27,9 @@
 import org.opensearch.client.Client;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.service.ClusterService;
+import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.env.Environment;
@@ -47,9 +50,10 @@
 
 public class TransportUserInjectorIntegTest extends SingleClusterTest {
 
+    public static final String TEST_INJECTED_USER = "test_injected_user";
+
     public static class UserInjectorPlugin extends Plugin implements ActionPlugin {
         Settings settings;
-        public static String injectedUser = null;
 
         public UserInjectorPlugin(final Settings settings, final Path configPath) {
             this.settings = settings;
@@ -69,17 +73,24 @@ public Collection<Object> createComponents(
             IndexNameExpressionResolver indexNameExpressionResolver,
             Supplier<RepositoriesService> repositoriesServiceSupplier
         ) {
-            if (injectedUser != null) threadPool.getThreadContext()
-                .putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUser);
+            if (!Strings.isNullOrEmpty(settings.get(TEST_INJECTED_USER))) threadPool.getThreadContext()
+                .putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, settings.get(TEST_INJECTED_USER));
             return new ArrayList<>();
         }
+
+        @Override
+        public List<Setting<?>> getSettings() {
+            List<Setting<?>> settings = new ArrayList<Setting<?>>();
+            settings.add(Setting.simpleString(TEST_INJECTED_USER, Setting.Property.NodeScope, Setting.Property.Filtered));
+            return settings;
+        }
     }
 
     @Test
     public void testSecurityUserInjection() throws Exception {
         final Settings clusterNodeSettings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, true).build();
         setup(clusterNodeSettings, new DynamicSecurityConfig().setSecurityRolesMapping("roles_transport_inject_user.yml"), Settings.EMPTY);
-        final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
+        final Settings.Builder tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
             .put(minimumSecuritySettings(Settings.EMPTY).get(0))
             .put("cluster.name", clusterInfo.clustername)
             .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data")
@@ -89,14 +100,13 @@ public void testSecurityUserInjection() throws Exception {
             .put("discovery.initial_state_timeout", "8s")
             .put("plugins.security.allow_default_init_securityindex", "true")
             .put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, true)
-            .putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort)
-            .build();
+            .putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort);
 
         // 1. without user injection
         try (
             Node node = new PluginAwareNode(
                 false,
-                tcSettings,
+                tcSettings.build(),
                 Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
             ).start()
         ) {
@@ -106,12 +116,11 @@ public void testSecurityUserInjection() throws Exception {
         }
 
         // 2. with invalid backend roles
-        UserInjectorPlugin.injectedUser = "ttt|kkk";
         Exception exception = null;
         try (
             Node node = new PluginAwareNode(
                 false,
-                tcSettings,
+                tcSettings.put(TEST_INJECTED_USER, "ttt|kkk").build(),
                 Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
             ).start()
         ) {
@@ -126,11 +135,10 @@ public void testSecurityUserInjection() throws Exception {
         }
 
         // 3. with valid backend roles for injected user
-        UserInjectorPlugin.injectedUser = "injectedadmin|injecttest";
         try (
             Node node = new PluginAwareNode(
                 false,
-                tcSettings,
+                tcSettings.put(TEST_INJECTED_USER, "injectedadmin|injecttest").build(),
                 Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
             ).start()
         ) {
@@ -146,7 +154,7 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception {
             .put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, false)
             .build();
         setup(clusterNodeSettings, new DynamicSecurityConfig().setSecurityRolesMapping("roles_transport_inject_user.yml"), Settings.EMPTY);
-        final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
+        final Settings.Builder tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
             .put(minimumSecuritySettings(Settings.EMPTY).get(0))
             .put("cluster.name", clusterInfo.clustername)
             .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data")
@@ -156,14 +164,13 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception {
             .put("discovery.initial_state_timeout", "8s")
             .put("plugins.security.allow_default_init_securityindex", "true")
             .put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, false)
-            .putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort)
-            .build();
+            .putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort);
 
         // 1. without user injection
         try (
             Node node = new PluginAwareNode(
                 false,
-                tcSettings,
+                tcSettings.build(),
                 Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
             ).start()
         ) {
@@ -173,11 +180,10 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception {
         }
 
         // with invalid backend roles
-        UserInjectorPlugin.injectedUser = "ttt|kkk";
         try (
             Node node = new PluginAwareNode(
                 false,
-                tcSettings,
+                tcSettings.put(TEST_INJECTED_USER, "ttt|kkk").build(),
                 Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
             ).start()
         ) {

src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java

@@ -90,6 +90,10 @@
 import org.opensearch.security.test.helper.cluster.ClusterInfo;
 import org.opensearch.security.test.helper.file.FileHelper;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
+
 public class RestHelper {
 
     protected final Logger log = LogManager.getLogger(RestHelper.class);
@@ -402,6 +406,29 @@ public HttpResponse(SimpleHttpResponse inner) throws IllegalStateException, IOEx
             this.statusCode = inner.getCode();
             this.statusReason = inner.getReasonPhrase();
             this.protocolVersion = inner.getVersion();
+
+            if (this.body.length() != 0) {
+                verifyBodyContentType();
+            }
+        }
+
+        private void verifyBodyContentType() {
+            final String contentType = this.getHeaders()
+                .stream()
+                .filter(h -> HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(h.getName()))
+                .map(Header::getValue)
+                .findFirst()
+                .orElseThrow(() -> new RuntimeException("No content type found. Headers:\n" + getHeaders() + "\n\nBody:\n" + body));
+
+            if (contentType.contains("application/json")) {
+                assertThat("Response body format was not json, body: " + body, body.charAt(0), equalTo('{'));
+            } else {
+                assertThat(
+                    "Response body format was json, whereas content-type was " + contentType + ", body: " + body,
+                    body.charAt(0),
+                    not(equalTo('{'))
+                );
+            }
         }
 
         public String getContentType() {