Fixes

Signed-off-by: Nils Bandener <[email protected]>

src/main/java/org/opensearch/security/DefaultObjectMapper.java

@@ -267,6 +267,23 @@ public static <T> T readValue(String string, JavaType jt) throws IOException {
         }
     }
 
+    @SuppressWarnings("removal")
+    public static <T> T convertValue(JsonNode jsonNode, JavaType jt) throws IOException {
+
+        final SecurityManager sm = System.getSecurityManager();
+
+        if (sm != null) {
+            sm.checkPermission(new SpecialPermission());
+        }
+
+        try {
+            return AccessController.doPrivileged((PrivilegedExceptionAction<T>) () -> objectMapper.convertValue(jsonNode, jt));
+        } catch (final PrivilegedActionException e) {
+            throw (IOException) e.getCause();
+        }
+    }
+
+
     public static TypeFactory getTypeFactory() {
         return objectMapper.getTypeFactory();
     }

src/main/java/org/opensearch/security/securityconf/impl/CType.java

@@ -78,7 +78,7 @@ public class CType<T> implements Comparable<CType<?>> {
         ConfigV7.class,
         3,
         false,
-        new OldConfigVersion<>(1, ConfigV6.class, ConfigV7::new)
+        new OldConfigVersion<>(1, ConfigV6.class, ConfigV7::new, ConfigV6::convertMapKeyToV7)
     );
     public static final CType<InternalUserV7> INTERNALUSERS = new CType<>(
         "internalusers",
@@ -109,6 +109,7 @@ public class CType<T> implements Comparable<CType<?>> {
     public static final CType<WhitelistingSettings> WHITELIST = new CType<>("whitelist", "whitelist", WhitelistingSettings.class, 9, true);
 
     private final String name;
+    private final String nameUpperCase;
     private final Class<T> configClass;
     private final String configFileName;
     private final boolean emptyIfMissing;
@@ -126,6 +127,7 @@ private CType(
         OldConfigVersion<?, T>... oldConfigVersions
     ) {
         this.name = name;
+        this.nameUpperCase = name.toUpperCase();
         this.configClass = configClass;
         this.ord = ord;
         this.configFileName = configFileName + ".yml";
@@ -208,15 +210,29 @@ public int compareTo(CType<?> cType) {
         return this.ord - cType.ord;
     }
 
+    @Override
+    public String toString() {
+        return this.nameUpperCase;
+    }
+
     public static class OldConfigVersion<OldType, NewType> {
         private final int versionNumber;
         private final Class<OldType> oldType;
-        private final Function<OldType, NewType> conversionFunction;
+        private final Function<OldType, NewType> entryConversionFunction;
+        private final Function<String, String> mapKeyConversionFunction;
+
+        public OldConfigVersion(int versionNumber, Class<OldType> oldType, Function<OldType, NewType> entryConversionFunction) {
+            this.versionNumber = versionNumber;
+            this.oldType = oldType;
+            this.entryConversionFunction = entryConversionFunction;
+            this.mapKeyConversionFunction = Function.identity();
+        }
 
-        public OldConfigVersion(int versionNumber, Class<OldType> oldType, Function<OldType, NewType> conversionFunction) {
+        public OldConfigVersion(int versionNumber, Class<OldType> oldType, Function<OldType, NewType> entryConversionFunction, Function<String, String> mapKeyConversionFunction) {
             this.versionNumber = versionNumber;
             this.oldType = oldType;
-            this.conversionFunction = conversionFunction;
+            this.entryConversionFunction = entryConversionFunction;
+            this.mapKeyConversionFunction = mapKeyConversionFunction;
         }
 
         public int getVersionNumber() {
@@ -227,8 +243,8 @@ public Class<OldType> getOldType() {
             return oldType;
         }
 
-        public Function<OldType, NewType> getConversionFunction() {
-            return conversionFunction;
+        public Function<OldType, NewType> getEntryConversionFunction() {
+            return entryConversionFunction;
         }
 
         public SecurityDynamicConfiguration<NewType> parseJson(CType<NewType> ctype, String json, boolean acceptInvalid)
@@ -246,7 +262,7 @@ public SecurityDynamicConfiguration<NewType> convert(SecurityDynamicConfiguratio
             SecurityDynamicConfiguration<NewType> newConfig = SecurityDynamicConfiguration.empty(ctype);
 
             for (Map.Entry<String, OldType> oldEntry : oldConfig.getCEntries().entrySet()) {
-                newConfig.putCEntry(oldEntry.getKey(), conversionFunction.apply(oldEntry.getValue()));
+                newConfig.putCEntry(mapKeyConversionFunction.apply(oldEntry.getKey()), entryConversionFunction.apply(oldEntry.getValue()));
             }
 
             return newConfig;

src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java

@@ -52,6 +52,8 @@
 
 public class SecurityDynamicConfiguration<T> implements ToXContent {
 
+    public static final int CURRENT_VERSION = 2;
+
     private static final TypeReference<HashMap<String, Object>> typeRefMSO = new TypeReference<HashMap<String, Object>>() {
     };
 
@@ -78,6 +80,10 @@ public static <T> SecurityDynamicConfiguration<T> fromJson(String json, CType<T>
         return fromJson(json, ctype, version, seqNo, primaryTerm, false);
     }
 
+    /**
+     * Creates the SecurityDynamicConfiguration instance from the given JSON. If a config version is found, which
+     * is not the current one, it will be automatically converted into the current configuration version.
+     */
     public static <T> SecurityDynamicConfiguration<T> fromJson(
         String json,
         CType<T> ctype,
@@ -92,6 +98,12 @@ public static <T> SecurityDynamicConfiguration<T> fromJson(
 
             if (oldConfigVersion != null) {
                 sdc = oldConfigVersion.parseJson(ctype, json, acceptInvalid);
+                if (sdc._meta == null) {
+                    sdc._meta = new Meta();
+                    sdc._meta.setConfig_version(CURRENT_VERSION);
+                    sdc._meta.setType(ctype.toLCString());
+                }
+                version = CURRENT_VERSION;
             } else {
                 sdc = DefaultObjectMapper.readValue(
                     json,
@@ -113,6 +125,34 @@ public static <T> SecurityDynamicConfiguration<T> fromJson(
         return sdc;
     }
 
+    /**
+     * Creates the SecurityDynamicConfiguration instance from the given JsonNode. If a config version is found, which
+     * is not the current one, no conversion will be performed. The configuration will be returned as it was found.
+     */
+    public static SecurityDynamicConfiguration<?> fromNodeWithoutAutoConversion(JsonNode jsonNode, CType<?> ctype, int version, long seqNo, long primaryTerm) throws IOException {
+        Class<?> configClass;
+        CType.OldConfigVersion<?, ?> oldConfigVersion = ctype.findOldConfigVersion(version);
+
+        if (oldConfigVersion != null) {
+            configClass = oldConfigVersion.getOldType();
+        } else {
+            configClass = ctype.getConfigClass();
+        }
+
+        SecurityDynamicConfiguration<?> sdc =  DefaultObjectMapper.convertValue(
+                jsonNode,
+                DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, configClass)
+        );
+
+        validate(sdc, version, ctype);
+
+        sdc.seqNo = seqNo;
+        sdc.primaryTerm = primaryTerm;
+        sdc.version = version;
+
+        return sdc;
+    }
+
     /**
      * For testing only
      */

src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java

@@ -45,11 +45,20 @@
 
 import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.auth.internal.InternalAuthenticationBackend;
+import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.securityconf.impl.DashboardSignInOption;
 import org.opensearch.security.setting.DeprecatedSettings;
 
 public class ConfigV6 {
 
+    public static String convertMapKeyToV7(String mapKey) {
+        if (mapKey.equals("opendistro_security")) {
+            return CType.CONFIG.toLCString();
+        } else {
+            return mapKey;
+        }
+    }
+
     public Dynamic dynamic;
 
     @Override

src/test/java/org/opensearch/security/ConfigTests.java

@@ -55,8 +55,8 @@ public class ConfigTests {
     @Test
     public void testEmptyConfig() throws Exception {
         Assert.assertNotSame(
-            SecurityDynamicConfiguration.empty(CType.CONFIG).deepClone(),
-            SecurityDynamicConfiguration.empty(CType.CONFIG)
+            SecurityDynamicConfiguration.empty(CType.ROLES).deepClone(),
+            SecurityDynamicConfiguration.empty(CType.ROLES)
         );
     }
 
@@ -119,7 +119,7 @@ private void check(String file, CType<?> cType) throws Exception {
         // Assert.assertTrue(dc.getCEntries().size() > 0);
         String jsonSerialize = DefaultObjectMapper.objectMapper.writeValueAsString(dc);
         SecurityDynamicConfiguration<?> conf = SecurityDynamicConfiguration.fromJson(jsonSerialize, cType, configVersion, 0, 0);
-        SecurityDynamicConfiguration.fromJson(Strings.toString(XContentType.JSON, conf), cType, configVersion, 0, 0);
+        SecurityDynamicConfiguration.fromJson(Strings.toString(XContentType.JSON, conf), cType, SecurityDynamicConfiguration.CURRENT_VERSION, 0, 0);
 
     }
 
@@ -132,6 +132,6 @@ private SecurityDynamicConfiguration<?> load(String file, CType<?> cType) throws
             assertThat(cType.toLCString(), is(jsonNode.get("_meta").get("type").asText()));
             configVersion = jsonNode.get("_meta").get("config_version").asInt();
         }
-        return SecurityDynamicConfiguration.fromNode(jsonNode, cType, configVersion, 0, 0);
+        return SecurityDynamicConfiguration.fromNodeWithoutAutoConversion(jsonNode, cType, configVersion, 0, 0);
     }
 }

src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java

@@ -379,39 +379,6 @@ public void testLoadConfiguration_shouldFailIfNoRequiredConfigInResponse() {
         verify(listener).onFailure(any());
     }
 
-    @Test
-    public void testLoadConfiguration_shouldFailForUnsupportedVersion() {
-        final var listener = spy(
-            ActionListener.<ConfigurationMap>wrap(
-                r -> fail("Unexpected behave"),
-                e -> assertThat(e.getMessage(), is("Version 1 is not supported for CONFIG"))
-            )
-        );
-        doAnswer(invocation -> {
-
-            final var objectMapper = DefaultObjectMapper.objectMapper;
-
-            ActionListener<MultiGetResponse> actionListener = invocation.getArgument(1);
-            final var getResult = mock(GetResult.class);
-            final var r = new MultiGetResponse(new MultiGetItemResponse[] { new MultiGetItemResponse(new GetResponse(getResult), null) });
-            when(getResult.getId()).thenReturn(CType.CONFIG.toLCString());
-            when(getResult.isExists()).thenReturn(true);
-
-            final var oldVersionJson = objectMapper.createObjectNode()
-                .set("opendistro_security", objectMapper.createObjectNode().set("dynamic", objectMapper.createObjectNode()))
-                .toString()
-                .getBytes(StandardCharsets.UTF_8);
-            final var configResponse = objectMapper.createObjectNode().put(CType.CONFIG.toLCString(), oldVersionJson);
-            final var source = objectMapper.writeValueAsBytes(configResponse);
-            when(getResult.sourceRef()).thenReturn(new BytesArray(source, 0, source.length));
-            actionListener.onResponse(r);
-            return null;
-        }).when(client).multiGet(any(MultiGetRequest.class), any());
-        securityIndexHandler.loadConfiguration(configuration(), listener);
-
-        verify(listener).onFailure(any());
-    }
-
     @Test
     public void testLoadConfiguration_shouldFailForUnparseableConfig() {
         final var listener = spy(