Correcting setupSslOnlyMode to use AbstractSecurityUnitTest.hasCustom…

…TransportSettings() (opensearch-project#1057)
lbreinig · Mar 3, 2021 · 1486e9f · 1486e9f
1 parent a2fb4bb
commit 1486e9f
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 15 deletions.
diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/security/ssl/SSLTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/security/ssl/SSLTest.java
@@ -954,7 +954,7 @@ public void testHttpsAndNodeSSLKeyStoreExtendedUsageEnabled() throws Exception {
 
             .build();
 
-        setupSslOnlyMode(settings, true);
+        setupSslOnlyMode(settings);
 
         RestHelper rh = restHelper();
         rh.enableHTTPClientSSL = true;
@@ -1031,7 +1031,7 @@ public void testHttpsAndNodeSSLPemExtendedUsageEnabled() throws Exception {
             .put(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_HTTP_PEMTRUSTEDCAS_FILEPATH, FileHelper. getAbsoluteFilePathFromClassPath("ssl/root-ca.pem"))
             .build();
 
-        setupSslOnlyMode(settings, true);
+        setupSslOnlyMode(settings);
 
         RestHelper rh = restHelper();
         rh.enableHTTPClientSSL = true;

diff --git a/...st/java/com/amazon/opendistroforelasticsearch/security/test/AbstractSecurityUnitTest.java b/...st/java/com/amazon/opendistroforelasticsearch/security/test/AbstractSecurityUnitTest.java
@@ -219,7 +219,7 @@ protected void initialize(ClusterInfo info, Settings initTransportClientSettings
         }
     }
 
-    protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslOnly, boolean hasCustomTransportSettings) {
+    protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslOnly, Settings other) {
 
         final String prefix = getResourceFolder()==null?"":getResourceFolder()+"/";
 
@@ -228,7 +228,7 @@ protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslO
                 .put(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_ENABLE_OPENSSL_IF_AVAILABLE, allowOpenSSL);
 
         // If custom transport settings are not defined use defaults
-        if (!hasCustomTransportSettings) {
+        if (!hasCustomTransportSettings(other)) {
             builder.put(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_KEYSTORE_ALIAS, "node-0")
                 .put(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_KEYSTORE_FILEPATH,
                     FileHelper.getAbsoluteFilePathFromClassPath(prefix+"node-0-keystore.jks"))
@@ -241,24 +241,26 @@ protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslO
             builder.put(ConfigConstants.OPENDISTRO_SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST, false);
         }
 
+        builder.put(other);
+
         return builder;
     }
 
     protected NodeSettingsSupplier minimumSecuritySettings(Settings other) {
         return new NodeSettingsSupplier() {
             @Override
             public Settings get(int i) {
-                return minimumSecuritySettingsBuilder(i, false, hasCustomTransportSettings(other)).put(other).build();
+                return minimumSecuritySettingsBuilder(i, false, other).build();
             }
         };
     }
 
-    protected NodeSettingsSupplier minimumSecuritySettingsSslOnly(Settings other, boolean hasCustomTransportSettings) {
+    protected NodeSettingsSupplier minimumSecuritySettingsSslOnly(Settings other) {
 
         return new NodeSettingsSupplier() {
             @Override
             public Settings get(int i) {
-                return minimumSecuritySettingsBuilder(i, true, hasCustomTransportSettings).put(other).build();
+                return minimumSecuritySettingsBuilder(i, true, other).build();
             }
         };
     }
@@ -271,7 +273,7 @@ public Settings get(int i) {
                 if (i == nonSSLNodeNum) {
                     return Settings.builder().build();
                 }
-                return minimumSecuritySettingsBuilder(i, true, false).put(other).build();
+                return minimumSecuritySettingsBuilder(i, true, other).build();
             }
         };
     }
@@ -307,7 +309,8 @@ protected String getType() {
      * @return boolean flag indicating if transport settings are defined
      */
     protected boolean hasCustomTransportSettings(Settings customSettings) {
-        // Note: current only doing this for PEMCERT settings
-        return customSettings.get(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PEMCERT_FILEPATH) != null;
+        // If Transport key extended usage is enabled this is true
+        return Boolean.parseBoolean(customSettings.get(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENDED_KEY_USAGE_ENABLED))  ||
+                customSettings.get(SSLConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_PEMCERT_FILEPATH) != null;
     }
 }
diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/security/test/SingleClusterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/security/test/SingleClusterTest.java
@@ -103,12 +103,8 @@ protected void setup(Settings initTransportClientSettings, DynamicSecurityConfig
     }
 
     protected void setupSslOnlyMode(Settings nodeOverride) throws Exception {
-        setupSslOnlyMode(nodeOverride, false);
-    }
-
-    protected void setupSslOnlyMode(Settings nodeOverride, boolean hasCustomTransportSettings) throws Exception {
         Assert.assertNull("No cluster", clusterInfo);
-        clusterInfo = clusterHelper.startCluster(minimumSecuritySettingsSslOnly(nodeOverride, hasCustomTransportSettings), ClusterConfiguration.DEFAULT);
+        clusterInfo = clusterHelper.startCluster(minimumSecuritySettingsSslOnly(nodeOverride), ClusterConfiguration.DEFAULT);
     }
 
     protected void setupSslOnlyModeWithMasterNodeWithoutSSL(Settings nodeOverride) throws Exception {