Merge branch 'main_origin' into SearchOperationTest-flakiness-fix

MaciejMierzwa · Nov 24, 2023 · 7874cd4 · 7874cd4
2 parents 7847c46 + 3c01fde
commit 7874cd4
Show file tree

Hide file tree

Showing 108 changed files with 243 additions and 153 deletions.
diff --git a/.github/workflows/add-untriaged.yml b/.github/workflows/add-untriaged.yml
@@ -8,7 +8,7 @@ jobs:
   apply-label:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/github-script@v6
+      - uses: actions/github-script@v7
         with:
           script: |
             github.rest.issues.addLabels({

diff --git a/.github/workflows/code-hygiene.yml b/.github/workflows/code-hygiene.yml
@@ -22,7 +22,7 @@ jobs:
       - uses: actions/setup-java@v3
         with:
           distribution: temurin # Temurin is a distribution of adoptium
-          java-version: 11
+          java-version: 17
 
       - uses: gradle/gradle-build-action@v2
         with:

diff --git a/build.gradle b/build.gradle
@@ -32,6 +32,7 @@ buildscript {
         jjwt_version = '0.12.3'
         guava_version = '32.1.3-jre'
         jaxb_version = '2.3.9'
+        spring_version = '5.3.31'
 
         if (buildVersionQualifier) {
             opensearch_build += "-${buildVersionQualifier}"
@@ -488,6 +489,9 @@ configurations {
             // for spotbugs dependency conflict
             force "org.apache.commons:commons-lang3:${versions.commonslang}"
 
+            // for spotless transitive dependency CVE
+            force "org.eclipse.platform:org.eclipse.core.runtime:3.29.0"
+
             // For integrationTest
             force "org.apache.httpcomponents:httpclient:4.5.14"
             force "org.apache.httpcomponents:httpcore:4.4.16"
@@ -682,7 +686,7 @@ dependencies {
     testImplementation "org.apache.kafka:kafka-clients:${kafka_version}:test"
     testImplementation 'commons-validator:commons-validator:1.7'
     testImplementation 'org.springframework.kafka:spring-kafka-test:2.9.13'
-    testImplementation 'org.springframework:spring-beans:5.3.30'
+    testImplementation "org.springframework:spring-beans:${spring_version}"
     testImplementation 'org.junit.jupiter:junit-jupiter:5.10.1'
     testImplementation 'org.junit.jupiter:junit-jupiter-api:5.10.1'
     // Only osx-x86_64, osx-aarch_64, linux-x86_64, linux-aarch_64, windows-x86_64 are available
@@ -694,7 +698,7 @@ dependencies {
     testCompileOnly 'org.apiguardian:apiguardian-api:1.1.2'
     // Kafka test execution
     testRuntimeOnly 'org.springframework.retry:spring-retry:1.3.4'
-    testRuntimeOnly ('org.springframework:spring-core:5.3.30') {
+    testRuntimeOnly ("org.springframework:spring-core:${spring_version}") {
         exclude(group:'org.springframework', module: 'spring-jcl' )
     }
     testRuntimeOnly 'org.scala-lang:scala-library:2.13.12'

diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml
@@ -121,6 +121,7 @@
     <module name="IllegalImport"> <!-- defaults to sun.* packages -->
       <property name="severity" value="error"/>
       <property name="illegalPkgs" value="org.apache.cxf.rs.security.jose"/>
+      <property name="illegalClasses" value="org.apache.hc.core5.http.HttpStatus"/>
     </module>
     <module name="RedundantImport">
       <property name="severity" value="error"/>

diff --git a/gradle/formatting.gradle b/gradle/formatting.gradle
@@ -17,6 +17,10 @@ allprojects {
 			eclipse().configFile rootProject.file('formatter/formatterConfig.xml')
 			trimTrailingWhitespace()
 			endWithNewline();
+			custom 'Replace illegal HttpStatus import w/ correct one', {
+				// e.g., replace org.apache.hc.core5.http.HttpStatus with org.apache.http.HttpStatus
+				it.replaceAll('org.apache.hc.core5.http.HttpStatus', 'org.apache.http.HttpStatus')
+			}
 
 			// See DEVELOPER_GUIDE.md for details of when to enable this.
 			if (System.getProperty('spotless.paddedcell') != null) {

diff --git a/release-notes/opensearch-security.release-notes-2.11.1.0.md b/release-notes/opensearch-security.release-notes-2.11.1.0.md
@@ -0,0 +1,7 @@
+## 2023-11-21 Version 2.11.1.0
+
+Compatible with OpenSearch 2.11.1
+
+### Bug Fixes
+* Fix regression on concurrent gzipped requests ([#3599](https://github.com/opensearch-project/security/pull/3599))
+* Fix issue with response content-types changed in 2.11 ([#3721](https://github.com/opensearch-project/security/pull/3721))
diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java
@@ -30,6 +30,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;
@@ -87,6 +89,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;
@@ -124,6 +127,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;
@@ -2277,21 +2281,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);
 
@@ -2334,10 +2350,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();
@@ -2354,6 +2367,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));
         }
     }
 
@@ -2391,10 +2408,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/integrationTest/java/org/opensearch/security/http/AsyncTests.java b/src/integrationTest/java/org/opensearch/security/http/AsyncTests.java
@@ -18,7 +18,7 @@
 import java.util.concurrent.TimeUnit;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;

diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
@@ -22,8 +22,8 @@
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
 import org.apache.hc.core5.http.Header;
-import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.http.HttpStatus;
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;

diff --git a/src/integrationTest/java/org/opensearch/security/http/ServiceAccountAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/ServiceAccountAuthenticationTest.java
@@ -15,7 +15,7 @@
 import java.util.Map;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;

diff --git a/src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java b/src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java
@@ -12,7 +12,7 @@
 package org.opensearch.security.rest;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;

diff --git a/src/integrationTest/java/org/opensearch/security/rest/CompressionTests.java b/src/integrationTest/java/org/opensearch/security/rest/CompressionTests.java
@@ -20,9 +20,9 @@
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
 import org.apache.hc.core5.http.ContentType;
-import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.http.HttpStatus;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;

diff --git a/src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java b/src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java
@@ -22,7 +22,7 @@
 import java.util.stream.Collectors;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;

diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/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() {

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

diff --git a/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java b/src/main/java/org/opensearch/security/tools/democonfig/Certificates.java
@@ -1,3 +1,14 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
 package org.opensearch.security.tools.democonfig;
 
 /**

diff --git a/src/main/java/org/opensearch/security/tools/democonfig/ExecutionEnvironment.java b/src/main/java/org/opensearch/security/tools/democonfig/ExecutionEnvironment.java
@@ -1,3 +1,14 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
 package org.opensearch.security.tools.democonfig;
 
 /**

diff --git a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java
@@ -212,9 +212,13 @@ static String buildSecurityConfigString() {
 
         securityConfigLines.append("plugins.security.authcz.admin_dn:\n  - CN=kirk,OU=client,O=client,L=test, C=de\n\n");
 
-        securityConfigLines.append("plugins.security.system_indices.enabled: true\n" + "plugins.security.system_indices.indices: [")
-            .append(SYSTEM_INDICES)
-            .append("]\n");
+        securityConfigLines.append("plugins.security.audit.type:  internal_opensearch\n");
+        securityConfigLines.append("plugins.security.enable_snapshot_restore_privilege:  true\n");
+        securityConfigLines.append("plugins.security.check_snapshot_restore_write_privileges:  true\n");
+        securityConfigLines.append("plugins.security.restapi.roles_enabled:  [\"all_access\", \"security_rest_api_access\"]\n");
+
+        securityConfigLines.append("plugins.security.system_indices.enabled: true\n");
+        securityConfigLines.append("plugins.security.system_indices.indices: [").append(SYSTEM_INDICES).append("]\n");
 
         if (!isNetworkHostAlreadyPresent(OPENSEARCH_CONF_FILE)) {
             if (cluster_mode) {

diff --git a/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendIntegTest.java b/src/test/java/com/amazon/dlic/auth/ldap/LdapBackendIntegTest.java
@@ -11,8 +11,8 @@
 
 package com.amazon.dlic.auth.ldap;
 
-import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.http.HttpStatus;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;

diff --git a/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendIntegTest2.java b/src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendIntegTest2.java
@@ -11,8 +11,8 @@
 
 package com.amazon.dlic.auth.ldap2;
 
-import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.http.HttpStatus;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;

diff --git a/src/test/java/org/opensearch/security/AdvancedSecurityMigrationTests.java b/src/test/java/org/opensearch/security/AdvancedSecurityMigrationTests.java
@@ -15,7 +15,7 @@
 import java.util.Arrays;
 
 import org.apache.hc.core5.http.Header;
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;

diff --git a/src/test/java/org/opensearch/security/AggregationTests.java b/src/test/java/org/opensearch/security/AggregationTests.java
@@ -26,7 +26,7 @@
 
 package org.opensearch.security;
 
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.Assert;
 import org.junit.Test;
 

diff --git a/src/test/java/org/opensearch/security/DataStreamIntegrationTests.java b/src/test/java/org/opensearch/security/DataStreamIntegrationTests.java
@@ -11,7 +11,7 @@
 
 package org.opensearch.security;
 
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.Assert;
 import org.junit.Test;
 

diff --git a/src/test/java/org/opensearch/security/EncryptionInTransitMigrationTests.java b/src/test/java/org/opensearch/security/EncryptionInTransitMigrationTests.java
@@ -10,7 +10,7 @@
  */
 package org.opensearch.security;
 
-import org.apache.hc.core5.http.HttpStatus;
+import org.apache.http.HttpStatus;
 import org.junit.Assert;
 import org.junit.Test;