Fix legacy check in SecurityAdmin (#2052)

* Fix legacy check in SecurityAdmin Signed-off-by: Craig Perkins <[email protected]> * Add unit test for legacy index check logic Signed-off-by: Craig Perkins <[email protected]> * Add SECURITY_ADMIN_TESTS to describe tests performed Signed-off-by: Craig Perkins <[email protected]> * Address lint error Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit f83a7e8)
opensearch-project · Sep 7, 2022 · cd7d969 · cd7d969
1 parent b20efea
commit cd7d969
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java
@@ -713,7 +713,8 @@ public static int execute(final String[] args) throws Exception {
 
             final boolean legacy = createLegacyMode || (indexExists
                     && securityIndex.getMappings() != null
-                    && securityIndex.getMappings().get(index) != null);
+                    && securityIndex.getMappings().get(index) != null
+                    && securityIndex.getMappings().get(index).getSourceAsMap().containsKey("security"));
 
             if(legacy) {
                 System.out.println("Legacy index '"+index+"' (ES 6) detected (or forced). You should migrate the configuration!");

diff --git a/src/test/java/org/opensearch/security/SecurityAdminTests.java b/src/test/java/org/opensearch/security/SecurityAdminTests.java
@@ -17,7 +17,9 @@
 
 package org.opensearch.security;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -27,6 +29,7 @@
 
 import org.opensearch.common.settings.Settings;
 import org.opensearch.security.ssl.util.SSLConfigConstants;
+import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.test.DynamicSecurityConfig;
 import org.opensearch.security.test.SingleClusterTest;
 import org.opensearch.security.test.helper.file.FileHelper;
@@ -494,6 +497,48 @@ public void testSecurityAdminValidateConfig() throws Exception {
         Assert.assertNotEquals(0, returnCode);
     }
 
+    @Test
+    public void testIsLegacySecurityIndexOnV7Index() throws Exception {
+        final Settings settings = Settings.builder()
+                .put("plugins.security.ssl.http.enabled",true)
+                .put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("node-0-keystore.jks"))
+                .put("plugins.security.ssl.http.truststore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("truststore.jks"))
+                .build();
+        setup(Settings.EMPTY, null, settings, false);
+
+        final String prefix = getResourceFolder()==null?"":getResourceFolder()+"/";
+
+        List<String> argsAsList = new ArrayList<>();
+        argsAsList.add("-ts");
+        argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix+"truststore.jks").toFile().getAbsolutePath());
+        argsAsList.add("-ks");
+        argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix+"kirk-keystore.jks").toFile().getAbsolutePath());
+        argsAsList.add("-p");
+        argsAsList.add(String.valueOf(clusterInfo.httpPort));
+        argsAsList.add("-cn");
+        argsAsList.add(clusterInfo.clustername);
+        addDirectoryPath(argsAsList, TEST_RESOURCE_ABSOLUTE_PATH);
+        argsAsList.add("-nhnv");
+
+        // Execute first time to create the index
+        int returnCode = SecurityAdmin.execute(argsAsList.toArray(new String[0]));
+        Assert.assertEquals(0, returnCode);
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        PrintStream ps = new PrintStream(baos);
+        PrintStream old = System.out;
+        System.setOut(ps);
+
+        returnCode = SecurityAdmin.execute(argsAsList.toArray(new String[0]));
+        Assert.assertEquals(0, returnCode);
+
+        System.out.flush();
+        System.setOut(old);
+        String standardOut = baos.toString();
+        String legacyIndexOutput = "Legacy index '"+ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX+"' (ES 6) detected (or forced). You should migrate the configuration!";
+        Assert.assertFalse(standardOut.contains(legacyIndexOutput));
+    }
+
     private void addDirectoryPath(final List<String> args, final String path) {
         args.add("-cd");
         args.add(path);

diff --git a/tools/SECURITY_ADMIN_TESTS.md b/tools/SECURITY_ADMIN_TESTS.md
@@ -0,0 +1,76 @@
+## Security Admin Tests
+
+A collection of tests to perform when making changes to `securityadmin.sh`
+
+### Running Security Admin
+
+Details about the Security Admin tool can be found on the [OpenSearch Documentation Website](https://opensearch.org/docs/latest/security-plugin/configuration/security-admin/).
+
+When running a cluster with the demo configuration, run the `securityadmin.sh` tool using:
+
+```
+./securityadmin.sh -cd ../../../config/opensearch-security/ -icl -nhnv \
+  -cacert ../../../config/root-ca.pem \
+  -cert ../../../config/kirk.pem \
+  -key ../../../config/kirk-key.pem
+```
+
+### Legacy Check Tests
+
+#### ODFE:<=0.10.0 (ES 6)
+
+In opendistro-for-elasticsearch:0.10.0 and before (See a full list of ODFE versions [here](https://opendistro.github.io/for-elasticsearch-docs/version-history/)), opendistro-for-elasticsearch (ODFE) security was configured with the legacy Security Config v6 format. 
+
+When running `securityadmin.sh` with the security index in the legacy v6 format, the following line will appear in the output when running the tool.
+
+```
+Legacy index '.opendistro_security' (ES 6) detected (or forced). You should migrate the configuration!
+````
+
+For information on how to migrate the security config from v6 to v7, see the [Backup, restore, and migrate](https://opensearch.org/docs/latest/security-plugin/configuration/security-admin/#backup-restore-and-migrate) section on the Security Admin Documentation page. 
+
+#### OpenSearch and ODFE:>=1.0.0 (ES 7)
+
+OpenSearch clusters and clusters running opendistro-for-elasticsearch:>=1.0.0 use the Security Config v7 format. When running the tool with the security index the in v7 format, the output will resemble:
+
+```
+./securityadmin.sh -cd ../../../config/opensearch-security/ -icl -nhnv \
+>   -cacert ../../../config/root-ca.pem \
+>   -cert ../../../config/kirk.pem \
+>   -key ../../../config/kirk-key.pem
+**************************************************************************
+** This tool will be deprecated in the next major release of OpenSearch **
+** https://github.com/opensearch-project/security/issues/1755           **
+**************************************************************************
+Security Admin v7
+Will connect to localhost:9200 ... done
+Connected as "CN=kirk,OU=client,O=client,L=test,C=de"
+OpenSearch Version: 2.2.0
+Contacting opensearch cluster 'opensearch' and wait for YELLOW clusterstate ...
+Clustername: opensearch-cluster
+Clusterstate: GREEN
+Number of nodes: 2
+Number of data nodes: 2
+.opendistro_security index already exists, so we do not need to create one.
+Populate config from /usr/share/opensearch/config/opensearch-security
+Will update '/config' with ../../../config/opensearch-security/config.yml
+   SUCC: Configuration for 'config' created or updated
+Will update '/roles' with ../../../config/opensearch-security/roles.yml
+   SUCC: Configuration for 'roles' created or updated
+Will update '/rolesmapping' with ../../../config/opensearch-security/roles_mapping.yml
+   SUCC: Configuration for 'rolesmapping' created or updated
+Will update '/internalusers' with ../../../config/opensearch-security/internal_users.yml
+   SUCC: Configuration for 'internalusers' created or updated
+Will update '/actiongroups' with ../../../config/opensearch-security/action_groups.yml
+   SUCC: Configuration for 'actiongroups' created or updated
+Will update '/tenants' with ../../../config/opensearch-security/tenants.yml
+   SUCC: Configuration for 'tenants' created or updated
+Will update '/nodesdn' with ../../../config/opensearch-security/nodes_dn.yml
+   SUCC: Configuration for 'nodesdn' created or updated
+Will update '/whitelist' with ../../../config/opensearch-security/whitelist.yml
+   SUCC: Configuration for 'whitelist' created or updated
+Will update '/audit' with ../../../config/opensearch-security/audit.yml
+   SUCC: Configuration for 'audit' created or updated
+Will update '/allowlist' with ../../../config/opensearch-security/allowlist.yml
+   SUCC: Configuration for 'allowlist' created or updated
+```