[Backport 2.x] Synchronize major changes from main (#3092)

Manual backports have been failing for a while, specifically the failure on #2926 clued me into the issue, we've been manually resolving these conflicts, but we should be trying to keep everything in sync as much as possible, this set of backports lumped together attempts to handle that. ### Issues Resolved - Resolves #3090 ### Testing Lots of `/gradlew testClasses`, CI checks should verify everything else. Signed-off-by: Peter Nied <[email protected]> Signed-off-by: Andrey Pleskach <[email protected]> Co-authored-by: Andrey Pleskach <[email protected]>
opensearch-project · Aug 3, 2023 · 159698f · 159698f
1 parent e499a8b
commit 159698f
Show file tree

Hide file tree

Showing 9 changed files with 237 additions and 126 deletions.
diff --git a/build.gradle b/build.gradle
diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/JwtVerifier.java b/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/JwtVerifier.java
@@ -12,7 +12,7 @@
 package com.amazon.dlic.auth.http.jwt.keybyoidc;
 
 import com.google.common.base.Strings;
-import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang3.StringEscapeUtils;
 import org.apache.cxf.rs.security.jose.jwa.SignatureAlgorithm;
 import org.apache.cxf.rs.security.jose.jwk.JsonWebKey;
 import org.apache.cxf.rs.security.jose.jwk.KeyType;

diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
@@ -168,9 +168,7 @@ private AuthTokenProcessorAction.Response handleImpl(
 
         try {
 
-            SamlResponse samlResponse = new SamlResponse(saml2Settings, null);
-            samlResponse.setDestinationUrl(acsEndpoint);
-            samlResponse.loadXmlFromBase64(samlResponseBase64);
+            SamlResponse samlResponse = new SamlResponse(saml2Settings, acsEndpoint, samlResponseBase64);
 
             if (!samlResponse.isValid(samlRequestId)) {
                 log.warn("Error while validating SAML response in /_opendistro/_security/api/authtoken");

diff --git a/src/main/java/org/opensearch/security/auth/RolesInjector.java b/src/main/java/org/opensearch/security/auth/RolesInjector.java
@@ -18,7 +18,7 @@
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
-import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 

diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java
@@ -34,7 +34,7 @@
 import java.util.stream.Collectors;
 
 import com.google.common.collect.Maps;
-import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 

diff --git a/src/test/java/org/opensearch/security/ConfigTests.java b/src/test/java/org/opensearch/security/ConfigTests.java
@@ -18,11 +18,12 @@
 package org.opensearch.security;
 
 import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
-import org.apache.commons.io.FileUtils;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -107,7 +108,7 @@ public void testParseSg67Config() throws Exception {
 
     private void check(String file, CType cType) throws Exception {
         final String adjustedFilePath = SingleClusterTest.TEST_RESOURCE_RELATIVE_PATH + file;
-        JsonNode jsonNode = YAML.readTree(FileUtils.readFileToString(new File(adjustedFilePath), "UTF-8"));
+        JsonNode jsonNode = YAML.readTree(Files.readString(new File(adjustedFilePath).toPath(), StandardCharsets.UTF_8));
         int configVersion = 1;
         System.out.println("%%%%%%%% THIS IS A LINE OF INTEREST %%%%%%%");
         if (jsonNode.get("_meta") != null) {
@@ -128,7 +129,7 @@ private void check(String file, CType cType) throws Exception {
 
     private SecurityDynamicConfiguration<?> load(String file, CType cType) throws Exception {
         final String adjustedFilePath = SingleClusterTest.TEST_RESOURCE_RELATIVE_PATH + file;
-        JsonNode jsonNode = YAML.readTree(FileUtils.readFileToString(new File(adjustedFilePath), "UTF-8"));
+        JsonNode jsonNode = YAML.readTree(Files.readString(new File(adjustedFilePath).toPath(), StandardCharsets.UTF_8));
         int configVersion = 1;
 
         System.out.println("%%%%%%%% THIS IS A LINE OF INTEREST LOAD: CONFIG VERSION: %%%%%%%");

diff --git a/src/test/java/org/opensearch/security/HttpIntegrationTests.java b/src/test/java/org/opensearch/security/HttpIntegrationTests.java
@@ -26,18 +26,13 @@
 
 package org.opensearch.security;
 
-import java.io.File;
-import java.nio.charset.StandardCharsets;
-
 import com.fasterxml.jackson.databind.JsonNode;
-import org.apache.commons.io.FileUtils;
 import org.apache.http.HttpStatus;
 import org.apache.http.NoHttpResponseException;
 import org.apache.http.message.BasicHeader;
 import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
-
 import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
 import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
 import org.opensearch.action.admin.indices.create.CreateIndexRequest;
@@ -57,6 +52,10 @@
 import org.opensearch.security.test.helper.rest.RestHelper;
 import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;
 
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+
 import static org.opensearch.security.DefaultObjectMapper.readTree;
 
 public class HttpIntegrationTests extends SingleClusterTest {
@@ -574,7 +573,7 @@ public void testHTTPPlaintextErrMsg() throws Exception {
             rh.executeGetRequest("", encodeBasicHeader("worf", "worf"));
             Assert.fail("NoHttpResponseException expected");
         } catch (NoHttpResponseException e) {
-            String log = FileUtils.readFileToString(new File("unittest.log"), StandardCharsets.UTF_8);
+            String log = Files.readString(new File("unittest.log").toPath(), StandardCharsets.UTF_8);
             Assert.assertTrue(log, log.contains("speaks http plaintext instead of ssl, will close the channel"));
         } catch (Exception e) {
             Assert.fail("NoHttpResponseException expected but was " + e.getClass() + "#" + e.getMessage());

diff --git a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java
@@ -29,6 +29,11 @@
 // CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -40,7 +45,6 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
-import org.apache.commons.io.FileUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -129,7 +133,7 @@ public final synchronized ClusterInfo startCluster(
 
         switch (clusterState) {
             case UNINITIALIZED:
-                FileUtils.deleteDirectory(new File("./target/data/" + clustername));
+                deleteTestsDataDirectory();
                 break;
             case STARTED:
                 closeAllNodes();
@@ -251,19 +255,15 @@ public void run() {
             PluginAwareNode node = new PluginAwareNode(setting.clusterManagerNode, settingsForNode, setting.getPlugins());
             System.out.println(node.settings());
 
-            new Thread(new Runnable() {
-
-                @Override
-                public void run() {
-                    try {
-                        node.start();
-                        latch.countDown();
-                    } catch (Exception e) {
-                        e.printStackTrace();
-                        log.error("Unable to start node: ", e);
-                        err.set(e);
-                        latch.countDown();
-                    }
+            new Thread(() -> {
+                try {
+                    node.start();
+                    latch.countDown();
+                } catch (Exception e) {
+                    e.printStackTrace();
+                    log.error("Unable to start node: ", e);
+                    err.set(e);
+                    latch.countDown();
                 }
             }).start();
             opensearchNodes.add(node);
@@ -308,9 +308,30 @@ public void run() {
         return cInfo;
     }
 
-    public final void stopCluster() throws Exception {
+    public void stopCluster() throws Exception {
         closeAllNodes();
-        FileUtils.deleteDirectory(new File("./target/data/" + clustername));
+        deleteTestsDataDirectory();
+    }
+
+    private void deleteTestsDataDirectory() throws IOException {
+        final File testsDataDir = new File("target/data/" + clustername);
+        if (testsDataDir.exists()) {
+            Files.walkFileTree(testsDataDir.toPath(), new SimpleFileVisitor<>() {
+                @Override
+                public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+                    System.out.println("Deleting file " + file.getFileName());
+                    Files.delete(file);
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
+                    System.out.println("Deleting directory " + dir.getFileName());
+                    Files.delete(dir);
+                    return FileVisitResult.CONTINUE;
+                }
+            });
+        }
     }
 
     private void closeAllNodes() throws Exception {

diff --git a/src/test/java/org/opensearch/security/test/helper/file/FileHelper.java b/src/test/java/org/opensearch/security/test/helper/file/FileHelper.java
@@ -32,6 +32,8 @@
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
@@ -42,11 +44,11 @@
 import java.nio.file.Paths;
 import java.security.KeyStore;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
 import org.opensearch.core.common.bytes.BytesReference;
+import org.opensearch.common.io.Streams;
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
@@ -94,10 +96,14 @@ public static Path getAbsoluteFilePathFromClassPath(final String fileNameFromCla
         return null;
     }
 
-    public static final String loadFile(final String file) throws IOException {
-        final StringWriter sw = new StringWriter();
-        IOUtils.copy(FileHelper.class.getResourceAsStream("/" + file), sw, StandardCharsets.UTF_8);
-        return sw.toString();
+    public static String loadFile(final String file) throws IOException {
+        try (
+            final StringWriter sw = new StringWriter();
+            final Reader reader = new InputStreamReader(FileHelper.class.getResourceAsStream("/" + file), StandardCharsets.UTF_8)
+        ) {
+            Streams.copy(reader, sw);
+            return sw.toString();
+        }
     }
 
     public static BytesReference readYamlContent(final String file) {