Removes support for txt file for specifying initial admin password an…

…d prevents password from being printed in the logs (opensearch-project#3850) Signed-off-by: Darshit Chanpura <[email protected]> Signed-off-by: Prabhas Kurapati <[email protected]>
prabhask5 · Jan 11, 2024 · e6d74f6 · e6d74f6
1 parent 1f2d289
commit e6d74f6
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 90 deletions.
diff --git a/.github/actions/start-opensearch-with-one-plugin/action.yml b/.github/actions/start-opensearch-with-one-plugin/action.yml
@@ -71,23 +71,22 @@ runs:
         'y' | .\opensearch-${{ inputs.opensearch-version }}-SNAPSHOT\bin\opensearch-plugin.bat install file:$(pwd)\${{ inputs.plugin-name }}.zip
       shell: pwsh
 
-    - name: Write password to opensearch_initial_admin_password txt
-      run:
-        echo ${{ inputs.admin-password }} >> ./opensearch-${{ env.OPENSEARCH_VERSION }}-SNAPSHOT/config/opensearch_initial_admin_password.txt
-      shell: bash
-
     # Run any configuration scripts
     - name: Run Setup Script for Linux
       if: ${{ runner.os == 'Linux' && inputs.setup-script-name != '' }}
       run: |
         echo "running linux setup"
+        export OPENSEARCH_INITIAL_ADMIN_PASSWORD=${{ inputs.admin-password }}
         chmod +x ./${{ inputs.setup-script-name }}.sh
         ./${{ inputs.setup-script-name }}.sh
       shell: bash
 
     - name: Run Setup Script for Windows
       if: ${{ runner.os == 'Windows' && inputs.setup-script-name != '' }}
-      run: .\${{ inputs.setup-script-name }}.bat
+      run: |
+        echo "running windows setup"
+        $env:OPENSEARCH_INITIAL_ADMIN_PASSWORD="${{ inputs.admin-password }}"
+        .\${{ inputs.setup-script-name }}.bat
       shell: pwsh
 
     # Run OpenSearch

diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java
@@ -334,9 +334,8 @@ public enum RolesMappingResolution {
     public static final boolean EXTENSIONS_BWC_PLUGIN_MODE_DEFAULT = false;
     // CS-ENFORCE-SINGLE
 
-    // Variables for initial admin password support
+    // Variable for initial admin password support
     public static final String OPENSEARCH_INITIAL_ADMIN_PASSWORD = "OPENSEARCH_INITIAL_ADMIN_PASSWORD";
-    public static final String OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT = "opensearch_initial_admin_password.txt";
 
     public static Set<String> getSettingAsSet(
         final Settings settings,

diff --git a/src/main/java/org/opensearch/security/tools/democonfig/Installer.java b/src/main/java/org/opensearch/security/tools/democonfig/Installer.java
@@ -428,9 +428,7 @@ void finishScriptExecution() {
             }
 
             System.out.println(
-                "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/"
-                    + SecuritySettingsConfigurer.ADMIN_PASSWORD
-                    + "."
+                "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/<your-custom-admin-password>."
             );
             System.out.println("### (Ignore the SSL certificate warning because we installed self-signed demo certificates)");
 

diff --git a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java
@@ -28,6 +28,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.dlic.rest.validation.PasswordValidator;
 import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
@@ -91,12 +92,34 @@ public void configureSecuritySettings() {
         writeSecurityConfigToOpenSearchYML();
     }
 
+    /**
+     * Checks if security plugin is already configured. If so, the script execution will exit.
+     */
+    void checkIfSecurityPluginIsAlreadyConfigured() {
+        // Check if the configuration file contains the 'plugins.security' string
+        if (installer.OPENSEARCH_CONF_FILE != null && new File(installer.OPENSEARCH_CONF_FILE).exists()) {
+            try (BufferedReader br = new BufferedReader(new FileReader(installer.OPENSEARCH_CONF_FILE, StandardCharsets.UTF_8))) {
+                String line;
+                while ((line = br.readLine()) != null) {
+                    if (line.toLowerCase().contains("plugins.security")) {
+                        System.out.println(installer.OPENSEARCH_CONF_FILE + " seems to be already configured for Security. Quit.");
+                        System.exit(installer.skip_updates);
+                    }
+                }
+            } catch (IOException e) {
+                System.err.println("Error reading configuration file.");
+                System.exit(-1);
+            }
+        } else {
+            System.err.println("OpenSearch configuration file does not exist. Quit.");
+            System.exit(-1);
+        }
+    }
+
     /**
      * Replaces the admin password in internal_users.yml with the custom or generated password
      */
     void updateAdminPassword() {
-        String initialAdminPassword = System.getenv().get(ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD);
-        String ADMIN_PASSWORD_FILE_PATH = installer.OPENSEARCH_CONF_DIR + ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT;
         String INTERNAL_USERS_FILE_PATH = installer.OPENSEARCH_CONF_DIR + "opensearch-security" + File.separator + "internal_users.yml";
         boolean shouldValidatePassword = installer.environment.equals(ExecutionEnvironment.DEMO);
         try {
@@ -107,21 +130,10 @@ void updateAdminPassword() {
                     .build()
             );
 
-            // Read custom password
-            if (initialAdminPassword != null && !initialAdminPassword.isEmpty()) {
+            // Read custom password from environment variable
+            String initialAdminPassword = System.getenv().get(ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD);
+            if (!Strings.isNullOrEmpty(initialAdminPassword)) {
                 ADMIN_PASSWORD = initialAdminPassword;
-            } else {
-                File adminPasswordFile = new File(ADMIN_PASSWORD_FILE_PATH);
-                if (adminPasswordFile.exists() && adminPasswordFile.length() > 0) {
-                    try (BufferedReader br = new BufferedReader(new FileReader(ADMIN_PASSWORD_FILE_PATH, StandardCharsets.UTF_8))) {
-                        ADMIN_PASSWORD = br.readLine();
-                    } catch (IOException e) {
-                        System.out.println(
-                            "Error reading admin password from " + ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT + "."
-                        );
-                        System.exit(-1);
-                    }
-                }
             }
 
             // If script execution environment is set to demo, validate custom password, else if set to test, skip validation
@@ -133,15 +145,13 @@ void updateAdminPassword() {
             }
 
             // if ADMIN_PASSWORD is still an empty string, it implies no custom password was provided. We exit the setup.
-            if (ADMIN_PASSWORD.isEmpty()) {
+            if (Strings.isNullOrEmpty(ADMIN_PASSWORD)) {
                 System.out.println("No custom admin password found. Please provide a password.");
                 System.exit(-1);
             }
 
-            // print the password to the logs
-            System.out.println("\t***************************************************");
-            System.out.println("\t\tADMIN PASSWORD SET TO: " + ADMIN_PASSWORD);
-            System.out.println("\t***************************************************");
+            // Print an update to the logs
+            System.out.println("Admin password set successfully.");
 
             writePasswordToInternalUsersFile(ADMIN_PASSWORD, INTERNAL_USERS_FILE_PATH);
 
@@ -188,30 +198,6 @@ void writePasswordToInternalUsersFile(String adminPassword, String internalUsers
         Files.move(tempFilePath, internalUsersPath, java.nio.file.StandardCopyOption.REPLACE_EXISTING);
     }
 
-    /**
-     * Checks if security plugin is already configured. If so, the script execution will not continue.
-     */
-    void checkIfSecurityPluginIsAlreadyConfigured() {
-        // Check if the configuration file contains the 'plugins.security' string
-        if (installer.OPENSEARCH_CONF_FILE != null && new File(installer.OPENSEARCH_CONF_FILE).exists()) {
-            try (BufferedReader br = new BufferedReader(new FileReader(installer.OPENSEARCH_CONF_FILE, StandardCharsets.UTF_8))) {
-                String line;
-                while ((line = br.readLine()) != null) {
-                    if (line.toLowerCase().contains("plugins.security")) {
-                        System.out.println(installer.OPENSEARCH_CONF_FILE + " seems to be already configured for Security. Quit.");
-                        System.exit(installer.skip_updates);
-                    }
-                }
-            } catch (IOException e) {
-                System.err.println("Error reading configuration file.");
-                System.exit(-1);
-            }
-        } else {
-            System.err.println("OpenSearch configuration file does not exist. Quit.");
-            System.exit(-1);
-        }
-    }
-
     /**
      * Update opensearch.yml with security configuration information
      */

diff --git a/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java b/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java
@@ -394,9 +394,7 @@ public void testFinishScriptExecution() {
             + System.lineSeparator()
             + "### After that you can also use the Security Plugin ConfigurationGUI"
             + System.lineSeparator()
-            + "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/"
-            + SecuritySettingsConfigurer.ADMIN_PASSWORD
-            + "."
+            + "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/<your-custom-admin-password>."
             + System.lineSeparator()
             + "### (Ignore the SSL certificate warning because we installed self-signed demo certificates)"
             + System.lineSeparator();
@@ -454,9 +452,7 @@ public void testFinishScriptExecution_withInitSecurityEnabled() {
             + System.lineSeparator()
             + "### To use the Security Plugin ConfigurationGUI"
             + System.lineSeparator()
-            + "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/"
-            + SecuritySettingsConfigurer.ADMIN_PASSWORD
-            + "."
+            + "### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/<your-custom-admin-password>."
             + System.lineSeparator()
             + "### (Ignore the SSL certificate warning because we installed self-signed demo certificates)"
             + System.lineSeparator();

diff --git a/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java b/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java
@@ -13,11 +13,9 @@
 
 // CS-SUPPRESS-SINGLE: RegexpSingleline extension key-word is used in file ext variable
 import java.io.BufferedReader;
-import java.io.BufferedWriter;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileReader;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
@@ -45,7 +43,6 @@
 import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.createDirectory;
 import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.createFile;
 import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.deleteDirectoryRecursive;
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@@ -79,42 +76,25 @@ public void tearDown() throws NoSuchFieldException, IllegalAccessException {
         System.setErr(originalErr);
         System.setIn(originalIn);
         deleteDirectoryRecursive(installer.OPENSEARCH_CONF_DIR);
-        unsetEnv(adminPasswordKey);
+        unsetEnvVariables();
         Installer.resetInstance();
     }
 
     @Test
     public void testUpdateAdminPasswordWithCustomPassword() throws NoSuchFieldException, IllegalAccessException {
-        String customPassword = "myStrongPassword123"; // generateStrongPassword();
+        String customPassword = "myStrongPassword123";
         setEnv(adminPasswordKey, customPassword);
 
         securitySettingsConfigurer.updateAdminPassword();
 
         assertThat(customPassword, is(equalTo(SecuritySettingsConfigurer.ADMIN_PASSWORD)));
 
-        verifyStdOutContainsString("ADMIN PASSWORD SET TO: " + customPassword);
-    }
-
-    @Test
-    public void testUpdateAdminPasswordWithFilePassword() throws IOException {
-        String customPassword = "myStrongPassword123";
-        String initialAdminPasswordTxt = installer.OPENSEARCH_CONF_DIR + adminPasswordKey + ".txt";
-        createFile(initialAdminPasswordTxt);
-
-        try (BufferedWriter writer = new BufferedWriter(new FileWriter(initialAdminPasswordTxt, StandardCharsets.UTF_8))) {
-            writer.write(customPassword);
-        } catch (IOException e) {
-            throw new IOException("Unable to update the internal users file with the hashed password.");
-        }
-
-        securitySettingsConfigurer.updateAdminPassword();
-
-        assertEquals(customPassword, SecuritySettingsConfigurer.ADMIN_PASSWORD);
-        verifyStdOutContainsString("ADMIN PASSWORD SET TO: " + customPassword);
+        verifyStdOutContainsString("Admin password set successfully.");
     }
 
     @Test
     public void testUpdateAdminPassword_noPasswordSupplied() {
+        SecuritySettingsConfigurer.ADMIN_PASSWORD = ""; // to ensure 0 flaky-ness
         try {
             System.setSecurityManager(new NoExitSecurityManager());
             securitySettingsConfigurer.updateAdminPassword();
@@ -150,7 +130,8 @@ public void testUpdateAdminPasswordWithWeakPassword_skipPasswordValidation() thr
         securitySettingsConfigurer.updateAdminPassword();
 
         assertThat("weakpassword", is(equalTo(SecuritySettingsConfigurer.ADMIN_PASSWORD)));
-        verifyStdOutContainsString("ADMIN PASSWORD SET TO: weakpassword");
+
+        verifyStdOutContainsString("Admin password set successfully.");
     }
 
     @Test
@@ -298,7 +279,7 @@ public static void setEnv(String key, String value) throws NoSuchFieldException,
     }
 
     @SuppressWarnings("unchecked")
-    public static void unsetEnv(String key) throws NoSuchFieldException, IllegalAccessException {
+    public static void unsetEnvVariables() throws NoSuchFieldException, IllegalAccessException {
         Class<?>[] classes = Collections.class.getDeclaredClasses();
         Map<String, String> env = System.getenv();
         for (Class<?> cl : classes) {
@@ -307,7 +288,7 @@ public static void unsetEnv(String key) throws NoSuchFieldException, IllegalAcce
                 field.setAccessible(true);
                 Object obj = field.get(env);
                 Map<String, String> map = (Map<String, String>) obj;
-                map.remove(key);
+                map.clear();
             }
         }
     }

diff --git a/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java b/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java
@@ -11,10 +11,13 @@
 
 package org.opensearch.security.tools.democonfig.util;
 
+/**
+ * Helper class to allow capturing and testing exit codes and block test execution from exiting mid-way
+ */
 public class NoExitSecurityManager extends SecurityManager {
     @Override
     public void checkPermission(java.security.Permission perm) {
-        // Allow everything except System.exit code 0 &b -1
+        // Allow everything except System.exit code 0 & -1
         if (perm instanceof java.lang.RuntimePermission && ("exitVM.0".equals(perm.getName()) || "exitVM.-1".equals(perm.getName()))) {
             StringBuilder sb = new StringBuilder();
             sb.append("System.exit(");