Merge branch '2.x' into backport/backport-2374-to-2.x

opensearch-project · Jan 6, 2023 · 9786c6c · 9786c6c
2 parents ed75c55 + 6a40584
commit 9786c6c
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 19 deletions.
diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java
@@ -216,10 +216,6 @@ public String[] extractRoles(JwtClaims claims) {
             roles = ((Collection<String>) rolesObject).toArray(new String[0]);
         }
 
-        for (int i = 0; i < roles.length; i++) {
-            roles[i] = roles[i].trim();
-        }
-
         return roles;
     }
 

diff --git a/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java
@@ -314,6 +314,45 @@ public void shouldNotEscapeSamlEntities() throws Exception {
         Assert.assertEquals("ABC/Admin", samlAuthenticator.httpJwtAuthenticator.extractRoles(jwt.getClaims())[0]);
     }
 
+    @Test
+    public void shouldNotTrimWhitespaceInJwtRoles() throws Exception {
+        mockSamlIdpServer.setAuthenticateUser("ABC/User1");
+        mockSamlIdpServer.setEndpointQueryString(null);
+        mockSamlIdpServer.setSpSignatureCertificate(spSigningCertificate);
+        mockSamlIdpServer.setEncryptAssertion(true);
+        mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList(" ABC/Admin "));
+
+        Settings settings = Settings.builder().put(IDP_METADATA_URL, mockSamlIdpServer.getMetadataUri())
+                .put("kibana_url", "http://wherever").put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
+                .put("sp.signature_private_key", "-BEGIN PRIVATE KEY-\n"
+                        + Base64.getEncoder().encodeToString(spSigningPrivateKey.getEncoded()) + "-END PRIVATE KEY-")
+                .put("exchange_key", "abc").put("roles_key", "roles").put("path.home", ".").build();
+
+        HTTPSamlAuthenticator samlAuthenticator = new HTTPSamlAuthenticator(settings, null);
+
+        AuthenticateHeaders authenticateHeaders = getAutenticateHeaders(samlAuthenticator);
+
+        String encodedSamlResponse = mockSamlIdpServer.handleSsoGetRequestURI(authenticateHeaders.location);
+
+        RestRequest tokenRestRequest = buildTokenExchangeRestRequest(encodedSamlResponse, authenticateHeaders);
+        TestRestChannel tokenRestChannel = new TestRestChannel(tokenRestRequest);
+
+        samlAuthenticator.reRequestAuthentication(tokenRestChannel, null);
+
+        String responseJson = new String(BytesReference.toBytes(tokenRestChannel.response.content()));
+        HashMap<String, Object> response = DefaultObjectMapper.objectMapper.readValue(responseJson,
+                new TypeReference<HashMap<String, Object>>() {
+                });
+        String authorization = (String) response.get("authorization");
+
+        Assert.assertNotNull("Expected authorization attribute in JSON: " + responseJson, authorization);
+
+        JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(authorization.replaceAll("\\s*bearer\\s*", ""));
+        JwtToken jwt = jwtConsumer.getJwtToken();
+
+        Assert.assertEquals("ABC/Admin", samlAuthenticator.httpJwtAuthenticator.extractRoles(jwt.getClaims())[0]);
+    }
+
     @Test
     public void testMetadataBody() throws Exception {
         mockSamlIdpServer.setSignResponses(true);

diff --git a/tools/audit_config_migrater.bat b/tools/audit_config_migrater.bat
@@ -1,16 +1,19 @@
 @echo off
-set SCRIPT_DIR=%~dp0
+set DIR=%~dp0
 
 echo "**************************************************************************"
 echo "** This tool will be deprecated in the next major release of OpenSearch **"
 echo "** https://github.com/opensearch-project/security/issues/1755           **"
 echo "**************************************************************************"
 
-rem comparing to empty string makes this equivalent to bash -v check on env var
-if not "%OPENSEARCH_JAVA_HOME%" == "" (
+if defined OPENSEARCH_JAVA_HOME (
   set BIN_PATH="%OPENSEARCH_JAVA_HOME%\bin\java.exe"
-) else (
+) else if defined JAVA_HOME (
   set BIN_PATH="%JAVA_HOME%\bin\java.exe"
+) else (
+  echo Unable to find java runtime
+  echo OPENSEARCH_JAVA_HOME or JAVA_HOME must be defined
+  exit /b 1
 )
 
-%BIN_PATH% -cp "%SCRIPT_DIR%\..\..\opendistro_security_ssl\*;%SCRIPT_DIR%\..\deps\*;%SCRIPT_DIR%\..\*;%SCRIPT_DIR%\..\..\..\lib\*" org.opensearch.security.tools.AuditConfigMigrater %*
+%BIN_PATH% -cp "%DIR%\..\*;%DIR%\..\..\..\lib\*;%DIR%\..\deps\*" org.opensearch.security.tools.AuditConfigMigrater %*
diff --git a/tools/hash.bat b/tools/hash.bat
@@ -1,17 +1,20 @@
 @echo off
-set SCRIPT_DIR=%~dp0
+set DIR=%~dp0
 
 echo "**************************************************************************"
 echo "** This tool will be deprecated in the next major release of OpenSearch **"
 echo "** https://github.com/opensearch-project/security/issues/1755           **"
 echo "**************************************************************************"
 
-rem comparing to empty string makes this equivalent to bash -v check on env var
-if not "%OPENSEARCH_JAVA_HOME%" == "" (
+if defined OPENSEARCH_JAVA_HOME (
   set BIN_PATH="%OPENSEARCH_JAVA_HOME%\bin\java.exe"
-) else (
+) else if defined JAVA_HOME (
   set BIN_PATH="%JAVA_HOME%\bin\java.exe"
+) else (
+  echo Unable to find java runtime
+  echo OPENSEARCH_JAVA_HOME or JAVA_HOME must be defined
+  exit /b 1
 )
 
-%BIN_PATH% -cp "%SCRIPT_DIR%\..\..\opendistro_security_ssl\*;%SCRIPT_DIR%\..\deps\*;%SCRIPT_DIR%\..\*;%SCRIPT_DIR%\..\..\..\lib\*" org.opensearch.security.tools.Hasher %*
+%BIN_PATH% -cp "%DIR%\..\*;%DIR%\..\..\..\lib\*;%DIR%\..\deps\*" org.opensearch.security.tools.Hasher %*
 
diff --git a/tools/securityadmin.bat b/tools/securityadmin.bat
@@ -1,16 +1,19 @@
 @echo off
-set SCRIPT_DIR=%~dp0
+set DIR=%~dp0
 
 echo "**************************************************************************"
 echo "** This tool will be deprecated in the next major release of OpenSearch **"
 echo "** https://github.com/opensearch-project/security/issues/1755           **"
 echo "**************************************************************************"
 
-rem comparing to empty string makes this equivalent to bash -v check on env var
-if not "%OPENSEARCH_JAVA_HOME%" == "" (
+if defined OPENSEARCH_JAVA_HOME (
   set BIN_PATH="%OPENSEARCH_JAVA_HOME%\bin\java.exe"
-) else (
+) else if defined JAVA_HOME (
   set BIN_PATH="%JAVA_HOME%\bin\java.exe"
+) else (
+  echo Unable to find java runtime
+  echo OPENSEARCH_JAVA_HOME or JAVA_HOME must be defined
+  exit /b 1
 )
 
-%BIN_PATH% -Dorg.apache.logging.log4j.simplelog.StatusLogger.level=OFF -cp "%SCRIPT_DIR%\..\..\opendistro_security-ssl\*;%SCRIPT_DIR%\..\deps\*;%SCRIPT_DIR%\..\*;%SCRIPT_DIR%\..\..\..\lib\*" org.opensearch.security.tools.SecurityAdmin %* 2> nul
+%BIN_PATH% -Dorg.apache.logging.log4j.simplelog.StatusLogger.level=OFF -cp "%DIR%\..\*;%DIR%\..\..\..\lib\*;%DIR%\..\deps\*" org.opensearch.security.tools.SecurityAdmin %* 2> nul