From 950c44799c3f6835e966d60e31b69a780615db4d Mon Sep 17 00:00:00 2001 From: Nils Bandener Date: Mon, 15 Jul 2024 10:48:06 +0200 Subject: [PATCH] Fixes Signed-off-by: Nils Bandener --- .../security/DefaultObjectMapper.java | 17 ++++++++ .../security/securityconf/impl/CType.java | 30 ++++++++++---- .../impl/SecurityDynamicConfiguration.java | 40 +++++++++++++++++++ .../securityconf/impl/v6/ConfigV6.java | 9 +++++ .../org/opensearch/security/ConfigTests.java | 8 ++-- .../support/SecurityIndexHandlerTest.java | 33 --------------- 6 files changed, 93 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/opensearch/security/DefaultObjectMapper.java b/src/main/java/org/opensearch/security/DefaultObjectMapper.java index d7ec09f5d9..de12e7fbdb 100644 --- a/src/main/java/org/opensearch/security/DefaultObjectMapper.java +++ b/src/main/java/org/opensearch/security/DefaultObjectMapper.java @@ -267,6 +267,23 @@ public static T readValue(String string, JavaType jt) throws IOException { } } + @SuppressWarnings("removal") + public static 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) () -> objectMapper.convertValue(jsonNode, jt)); + } catch (final PrivilegedActionException e) { + throw (IOException) e.getCause(); + } + } + + public static TypeFactory getTypeFactory() { return objectMapper.getTypeFactory(); } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/CType.java b/src/main/java/org/opensearch/security/securityconf/impl/CType.java index 87e1e38709..cf481bdd0a 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/CType.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/CType.java @@ -78,7 +78,7 @@ public class CType implements Comparable> { ConfigV7.class, 3, false, - new OldConfigVersion<>(1, ConfigV6.class, ConfigV7::new) + new OldConfigVersion<>(1, ConfigV6.class, ConfigV7::new, ConfigV6::convertMapKeyToV7) ); public static final CType INTERNALUSERS = new CType<>( "internalusers", @@ -109,6 +109,7 @@ public class CType implements Comparable> { public static final CType WHITELIST = new CType<>("whitelist", "whitelist", WhitelistingSettings.class, 9, true); private final String name; + private final String nameUpperCase; private final Class configClass; private final String configFileName; private final boolean emptyIfMissing; @@ -126,6 +127,7 @@ private CType( OldConfigVersion... 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 { private final int versionNumber; private final Class oldType; - private final Function conversionFunction; + private final Function entryConversionFunction; + private final Function mapKeyConversionFunction; + + public OldConfigVersion(int versionNumber, Class oldType, Function entryConversionFunction) { + this.versionNumber = versionNumber; + this.oldType = oldType; + this.entryConversionFunction = entryConversionFunction; + this.mapKeyConversionFunction = Function.identity(); + } - public OldConfigVersion(int versionNumber, Class oldType, Function conversionFunction) { + public OldConfigVersion(int versionNumber, Class oldType, Function entryConversionFunction, Function 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 getOldType() { return oldType; } - public Function getConversionFunction() { - return conversionFunction; + public Function getEntryConversionFunction() { + return entryConversionFunction; } public SecurityDynamicConfiguration parseJson(CType ctype, String json, boolean acceptInvalid) @@ -246,7 +262,7 @@ public SecurityDynamicConfiguration convert(SecurityDynamicConfiguratio SecurityDynamicConfiguration newConfig = SecurityDynamicConfiguration.empty(ctype); for (Map.Entry oldEntry : oldConfig.getCEntries().entrySet()) { - newConfig.putCEntry(oldEntry.getKey(), conversionFunction.apply(oldEntry.getValue())); + newConfig.putCEntry(mapKeyConversionFunction.apply(oldEntry.getKey()), entryConversionFunction.apply(oldEntry.getValue())); } return newConfig; diff --git a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java index 0c6d434765..ee96d1c479 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -52,6 +52,8 @@ public class SecurityDynamicConfiguration implements ToXContent { + public static final int CURRENT_VERSION = 2; + private static final TypeReference> typeRefMSO = new TypeReference>() { }; @@ -78,6 +80,10 @@ public static SecurityDynamicConfiguration fromJson(String json, CType 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 SecurityDynamicConfiguration fromJson( String json, CType ctype, @@ -92,6 +98,12 @@ public static SecurityDynamicConfiguration 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 SecurityDynamicConfiguration 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 */ diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java index 78758e0603..59b759970e 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java +++ b/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 diff --git a/src/test/java/org/opensearch/security/ConfigTests.java b/src/test/java/org/opensearch/security/ConfigTests.java index 8864a2485e..b34f74e711 100644 --- a/src/test/java/org/opensearch/security/ConfigTests.java +++ b/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); } } diff --git a/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java b/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java index 007b4f1d13..f1d62fc5f4 100644 --- a/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java +++ b/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.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 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(