diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java new file mode 100644 index 0000000000..c3ef83528a --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java @@ -0,0 +1,119 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ +package org.opensearch.security.legacy; + +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.Assert; +import org.junit.ClassRule; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; + +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class LegacyConfigV6AutoConversionTest { + static final TestSecurityConfig LEGACY_CONFIG = new TestSecurityConfig()// + .rawConfigurationDocumentYaml( + "config", + "opendistro_security:\n" + + " dynamic:\n" + + " authc:\n" + + " basic_internal_auth_domain:\n" + + " http_enabled: true\n" + + " order: 4\n" + + " http_authenticator:\n" + + " type: basic\n" + + " challenge: true\n" + + " authentication_backend:\n" + + " type: intern\n" + ) + .rawConfigurationDocumentYaml( + "internalusers", + "admin:\n" + + " readonly: true\n" + + " hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG\n" + + " roles:\n" + + " - admin\n" + + " attributes:\n" + + " attribute1: value1\n" + ) + .rawConfigurationDocumentYaml( + "roles", + "all_access_role:\n" + + " readonly: true\n" + + " cluster:\n" + + " - UNLIMITED\n" + + " indices:\n" + + " '*':\n" + + " '*':\n" + + " - UNLIMITED\n" + + " tenants:\n" + + " admin_tenant: RW\n" + ) + .rawConfigurationDocumentYaml("rolesmapping", "all_access_role:\n" + " readonly: true\n" + " backendroles:\n" + " - admin")// + .rawConfigurationDocumentYaml("actiongroups", "dummy:\n" + " permissions: []"); + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) + .config(LEGACY_CONFIG) + .nodeSettings(Map.of("plugins.security.restapi.roles_enabled.0", "all_access_role")) + .build(); + + @Test + public void authc() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.get("_opendistro/_security/authinfo"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals(response.getBody(), true, response.getBooleanFromJsonBody("/tenants/admin")); + } + } + + @Test + public void configRestApiReturnsV6Config() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.get("_opendistro/_security/api/roles/all_access_role"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals( + "Expected v6 format", + "{\"all_access_role\":{\"readonly\":true,\"hidden\":false,\"cluster\":[\"UNLIMITED\"],\"tenants\":{\"admin_tenant\":\"RW\"},\"indices\":{\"*\":{\"*\":[\"UNLIMITED\"]}}}}", + response.getBody() + ); + } + } + + /** + * This must be the last test executed, as it changes the config index + */ + @Test + public void zzz_migrateApi() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.post("_opendistro/_security/api/migrate"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals(response.getBody(), "Migration completed.", response.getTextFromJsonBody("/message")); + response = client.get("_opendistro/_security/api/roles/all_access_role"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals( + "Expected v7 format", + "Migrated from v6 (all types mapped)", + response.getTextFromJsonBody("/all_access_role/description") + ); + } + } + +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java index a1ea3720ba..9edf77f75c 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -43,6 +43,9 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -94,6 +97,13 @@ public class TestSecurityConfig { private Map actionGroups = new LinkedHashMap<>(); + /** + * A map from document id to a string containing config JSON. + * If this is not null, it will be used ALTERNATIVELY to all other configuration contained in this class. + * Can be used to simulate invalid configuration or legacy configuration. + */ + private Map rawConfigurationDocuments; + private String indexName = ".opendistro_security"; public TestSecurityConfig() { @@ -212,6 +222,27 @@ public List actionGroups() { return List.copyOf(actionGroups.values()); } + /** + * Specifies raw document content for the configuration index as YAML document. If this method is used, + * then ONLY the raw documents will be written to the configuration index. Any other configuration specified + * by the roles() or users() method will be ignored. + * Can be used to simulate invalid configuration or legacy configuration. + */ + public TestSecurityConfig rawConfigurationDocumentYaml(String configTypeId, String configDocumentAsYaml) { + try { + if (this.rawConfigurationDocuments == null) { + this.rawConfigurationDocuments = new LinkedHashMap<>(); + } + + JsonNode node = new ObjectMapper(new YAMLFactory()).readTree(configDocumentAsYaml); + + this.rawConfigurationDocuments.put(configTypeId, new ObjectMapper().writeValueAsString(node)); + return this; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + public static class Config implements ToXContentObject { private boolean anonymousAuth; @@ -964,15 +995,24 @@ public void initIndex(Client client) { } client.admin().indices().create(new CreateIndexRequest(indexName).settings(settings)).actionGet(); - writeSingleEntryConfigToIndex(client, CType.CONFIG, config); - if (auditConfiguration != null) { - writeSingleEntryConfigToIndex(client, CType.AUDIT, "config", auditConfiguration); + if (rawConfigurationDocuments == null) { + writeSingleEntryConfigToIndex(client, CType.CONFIG, config); + if (auditConfiguration != null) { + writeSingleEntryConfigToIndex(client, CType.AUDIT, "config", auditConfiguration); + } + writeConfigToIndex(client, CType.ROLES, roles); + writeConfigToIndex(client, CType.INTERNALUSERS, internalUsers); + writeConfigToIndex(client, CType.ROLESMAPPING, rolesMapping); + writeEmptyConfigToIndex(client, CType.ACTIONGROUPS); + writeEmptyConfigToIndex(client, CType.TENANTS); + } else { + // Write raw configuration alternatively to the normal configuration + + for (Map.Entry entry : this.rawConfigurationDocuments.entrySet()) { + writeConfigToIndex(client, entry.getKey(), entry.getValue()); + } } - writeConfigToIndex(client, CType.ROLES, roles); - writeConfigToIndex(client, CType.INTERNALUSERS, internalUsers); - writeConfigToIndex(client, CType.ROLESMAPPING, rolesMapping); - writeEmptyConfigToIndex(client, CType.ACTIONGROUPS); - writeEmptyConfigToIndex(client, CType.TENANTS); + } public void updateInternalUsersConfiguration(Client client, List users) { @@ -987,11 +1027,11 @@ static String hashPassword(final String clearTextPassword) { return passwordHasher.hash(clearTextPassword.toCharArray()); } - private void writeEmptyConfigToIndex(Client client, CType configType) { + private void writeEmptyConfigToIndex(Client client, CType configType) { writeConfigToIndex(client, configType, Collections.emptyMap()); } - private void writeConfigToIndex(Client client, CType configType, Map config) { + private void writeConfigToIndex(Client client, CType configType, Map config) { try { String json = configToJson(configType, config); @@ -1008,11 +1048,23 @@ private void writeConfigToIndex(Client client, CType configType, Map config) { + private void updateConfigInIndex(Client client, CType configType, Map config) { try { String json = configToJson(configType, config); BytesReference bytesReference = toByteReference(json); @@ -1025,7 +1077,7 @@ private void updateConfigInIndex(Client client, CType configType, Map config) throws IOException { + private static String configToJson(CType configType, Map config) throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); @@ -1043,11 +1095,11 @@ private static String configToJson(CType configType, Map configType, ToXContentObject config) { writeSingleEntryConfigToIndex(client, configType, configType.toLCString(), config); } - private void writeSingleEntryConfigToIndex(Client client, CType configType, String configurationRoot, ToXContentObject config) { + private void writeSingleEntryConfigToIndex(Client client, CType configType, String configurationRoot, ToXContentObject config) { try { XContentBuilder builder = XContentFactory.jsonBuilder(); diff --git a/src/integrationTest/resources/log4j2-test.properties b/src/integrationTest/resources/log4j2-test.properties index ee1bd5b455..7e0e0c7956 100644 --- a/src/integrationTest/resources/log4j2-test.properties +++ b/src/integrationTest/resources/log4j2-test.properties @@ -46,4 +46,4 @@ logger.backendreg.appenderRef.capturing.ref = logCapturingAppender #logger.ldap.name=com.amazon.dlic.auth.ldap.backend.LDAPAuthenticationBackend logger.ldap.name=com.amazon.dlic.auth.ldap.backend logger.ldap.level=TRACE -logger.ldap.appenderRef.capturing.ref = logCapturingAppender +logger.ldap.appenderRef.capturing.ref = logCapturingAppender \ No newline at end of file diff --git a/src/main/java/org/opensearch/security/DefaultObjectMapper.java b/src/main/java/org/opensearch/security/DefaultObjectMapper.java index 2d18667c54..532e2eb603 100644 --- a/src/main/java/org/opensearch/security/DefaultObjectMapper.java +++ b/src/main/java/org/opensearch/security/DefaultObjectMapper.java @@ -266,6 +266,22 @@ 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/configuration/ConfigurationChangeListener.java b/src/main/java/org/opensearch/security/configuration/ConfigurationChangeListener.java index 024bd55157..cc410c0158 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationChangeListener.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationChangeListener.java @@ -26,11 +26,6 @@ package org.opensearch.security.configuration; -import java.util.Map; - -import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; - /** * Callback function on change particular configuration */ @@ -39,5 +34,5 @@ public interface ConfigurationChangeListener { /** * @param configuration not null updated configuration on that was subscribe current listener */ - void onChange(Map> typeToConfig); + void onChange(ConfigurationMap typeToConfig); } diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java b/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java index ec4a1e3e38..571bb802db 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java @@ -29,8 +29,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -40,7 +38,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.LegacyESVersion; import org.opensearch.action.get.GetResponse; import org.opensearch.action.get.MultiGetItemResponse; import org.opensearch.action.get.MultiGetRequest; @@ -57,8 +54,12 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auditlog.config.AuditConfig; +import org.opensearch.security.securityconf.Migration; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v6.RoleV6; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.securityconf.impl.v7.TenantV7; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.ConfigHelper; import org.opensearch.security.support.SecurityUtils; @@ -95,10 +96,10 @@ boolean isAuditConfigDocPresentInIndex() { return isAuditConfigDocPresentInIndex.get(); } - Map> load(final CType[] events, long timeout, TimeUnit timeUnit, boolean acceptInvalid) - throws InterruptedException, TimeoutException { + ConfigurationMap load(final CType[] events, long timeout, TimeUnit timeUnit, boolean acceptInvalid) throws InterruptedException, + TimeoutException { final CountDownLatch latch = new CountDownLatch(events.length); - final Map> rs = new HashMap<>(events.length); + ConfigurationMap.Builder result = new ConfigurationMap.Builder(); final boolean isDebugEnabled = log.isDebugEnabled(); loadAsync(events, new ConfigCallback() { @@ -119,7 +120,8 @@ public void success(SecurityDynamicConfiguration dConf) { isAuditConfigDocPresentInIndex.set(true); } - rs.put(dConf.getCType(), dConf); + result.with(dConf); + latch.countDown(); if (isDebugEnabled) { log.debug( @@ -143,36 +145,19 @@ public void singleFailure(Failure failure) { @Override public void noData(String id) { - CType cType = CType.fromString(id); - - // when index was created with ES 6 there are no separate tenants. So we load just empty ones. - // when index was created with ES 7 and type not "security" (ES 6 type) there are no rolemappings anymore. - if (cs.state().metadata().index(securityIndex).getCreationVersion().before(LegacyESVersion.V_7_0_0)) { - // created with SG 6 - // skip tenants - - if (isDebugEnabled) { - log.debug("Skip tenants because we not yet migrated to ES 7 (index was created with ES 6)"); - } - - if (cType == CType.TENANTS) { - rs.put(cType, SecurityDynamicConfiguration.empty()); - latch.countDown(); - return; - } - } + CType cType = CType.fromString(id); // Since NODESDN is newly introduced data-type applying for existing clusters as well, we make it backward compatible by // returning valid empty // SecurityDynamicConfiguration. // Same idea for new setting WHITELIST/ALLOWLIST - if (cType == CType.NODESDN || cType == CType.WHITELIST || cType == CType.ALLOWLIST) { + if (cType == CType.NODESDN || cType == CType.WHITELIST || cType == CType.ALLOWLIST || cType == CType.TENANTS) { try { SecurityDynamicConfiguration empty = ConfigHelper.createEmptySdc( cType, ConfigurationRepository.getDefaultConfigVersion() ); - rs.put(cType, empty); + result.with(empty); latch.countDown(); return; } catch (Exception e) { @@ -190,7 +175,7 @@ public void noData(String id) { ConfigurationRepository.getDefaultConfigVersion() ); empty.putCObject("config", AuditConfig.from(settings)); - rs.put(cType, empty); + result.with(empty); latch.countDown(); return; } catch (Exception e) { @@ -222,10 +207,26 @@ public void failure(Throwable t) { ); } - return rs; + SecurityDynamicConfiguration roleConfig = result.get(CType.ROLES); + SecurityDynamicConfiguration tenantConfig = result.get(CType.TENANTS); + + if (roleConfig != null + && roleConfig.getAutoConvertedFrom() != null + && (tenantConfig == null || tenantConfig.getCEntries().isEmpty())) { + // Special case for configuration that was auto-converted from v6: + // We need to generate tenant config from role config. + // Having such a special case here is not optimal, but IMHO acceptable, as this + // should be only a temporary measure until V6 configuration is completely discontinued. + @SuppressWarnings("unchecked") + SecurityDynamicConfiguration roleV6config = (SecurityDynamicConfiguration) roleConfig.getAutoConvertedFrom(); + SecurityDynamicConfiguration tenants = Migration.createTenants(roleV6config); + result.with(tenants); + } + + return result.build(); } - void loadAsync(final CType[] events, final ConfigCallback callback, boolean acceptInvalid) { + void loadAsync(final CType[] events, final ConfigCallback callback, boolean acceptInvalid) { if (events == null || events.length == 0) { log.warn("No config events requested to load"); return; diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java b/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java new file mode 100644 index 0000000000..ef349e20fc --- /dev/null +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java @@ -0,0 +1,110 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.configuration; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import com.google.common.collect.ImmutableMap; + +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; + +/** + * Allows type safe access of configuration instances via the configuration type + */ +public class ConfigurationMap { + public static final ConfigurationMap EMPTY = new ConfigurationMap(ImmutableMap.of()); + + private final ImmutableMap, SecurityDynamicConfiguration> map; + + private ConfigurationMap(ImmutableMap, SecurityDynamicConfiguration> map) { + this.map = map; + } + + public SecurityDynamicConfiguration get(CType ctype) { + @SuppressWarnings("unchecked") + SecurityDynamicConfiguration config = (SecurityDynamicConfiguration) map.get(ctype); + + if (config == null) { + return null; + } + + if (!config.getCType().equals(ctype)) { + throw new RuntimeException("Stored configuration does not match type: " + ctype + "; " + config); + } + + return config; + } + + public boolean containsKey(CType ctype) { + return map.containsKey(ctype); + } + + public Set> keySet() { + return map.keySet(); + } + + public int size() { + return this.map.size(); + } + + public ImmutableMap, SecurityDynamicConfiguration> rawMap() { + return this.map; + } + + public static ConfigurationMap of(SecurityDynamicConfiguration... configs) { + Builder builder = new Builder(); + + for (SecurityDynamicConfiguration config : configs) { + builder.with(config); + } + + return builder.build(); + } + + public static class Builder { + private Map, SecurityDynamicConfiguration> map = new HashMap<>(); + + public Builder() {} + + public Builder with(SecurityDynamicConfiguration config) { + map.put(config.getCType(), config); + return this; + } + + public Builder with(ConfigurationMap configurationMap) { + map.putAll(configurationMap.map); + return this; + } + + public SecurityDynamicConfiguration get(CType ctype) { + @SuppressWarnings("unchecked") + SecurityDynamicConfiguration config = (SecurityDynamicConfiguration) map.get(ctype); + + if (config == null) { + return null; + } + + if (!config.getCType().equals(ctype)) { + throw new RuntimeException("Stored configuration does not match type: " + ctype + "; " + config); + } + + return config; + } + + public ConfigurationMap build() { + return new ConfigurationMap(ImmutableMap.copyOf(this.map)); + } + } +} diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index 4bfe2d0f94..f8fe70ca1a 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -31,7 +31,6 @@ import java.text.SimpleDateFormat; import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.HashMap; @@ -98,7 +97,7 @@ public class ConfigurationRepository implements ClusterStateListener { private final String securityIndex; private final Client client; - private final Cache> configCache; + private final Cache, SecurityDynamicConfiguration> configCache; private final List configurationChangedListener; private final ConfigurationLoaderSecurity7 cl; private final Settings settings; @@ -281,7 +280,7 @@ private void initalizeClusterConfiguration(final boolean installDefaultConfig) { while (!dynamicConfigFactory.isInitialized()) { try { LOGGER.debug("Try to load config ..."); - reloadConfiguration(Arrays.asList(CType.values()), true); + reloadConfiguration(CType.values(), true); break; } catch (Exception e) { LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e))); @@ -508,21 +507,23 @@ public void setDynamicConfigFactory(DynamicConfigFactory dynamicConfigFactory) { * @param configurationType * @return can also return empty in case it was never loaded */ - public SecurityDynamicConfiguration getConfiguration(CType configurationType) { + public SecurityDynamicConfiguration getConfiguration(CType configurationType) { SecurityDynamicConfiguration conf = configCache.getIfPresent(configurationType); if (conf != null) { - return conf.deepClone(); + @SuppressWarnings("unchecked") + SecurityDynamicConfiguration result = (SecurityDynamicConfiguration) conf.deepClone(); + return result; } - return SecurityDynamicConfiguration.empty(); + return SecurityDynamicConfiguration.empty(configurationType); } private final Lock LOCK = new ReentrantLock(); - public boolean reloadConfiguration(final Collection configTypes) throws ConfigUpdateAlreadyInProgressException { + public boolean reloadConfiguration(final Collection> configTypes) throws ConfigUpdateAlreadyInProgressException { return reloadConfiguration(configTypes, false); } - private boolean reloadConfiguration(final Collection configTypes, final boolean fromBackgroundThread) + private boolean reloadConfiguration(final Collection> configTypes, final boolean fromBackgroundThread) throws ConfigUpdateAlreadyInProgressException { if (!fromBackgroundThread && !initalizeConfigTask.isDone()) { LOGGER.warn("Unable to reload configuration, initalization thread has not yet completed."); @@ -531,7 +532,7 @@ private boolean reloadConfiguration(final Collection configTypes, final b return loadConfigurationWithLock(configTypes); } - private boolean loadConfigurationWithLock(Collection configTypes) { + private boolean loadConfigurationWithLock(Collection> configTypes) { try { if (LOCK.tryLock(60, TimeUnit.SECONDS)) { try { @@ -549,13 +550,13 @@ private boolean loadConfigurationWithLock(Collection configTypes) { } } - private void reloadConfiguration0(Collection configTypes, boolean acceptInvalid) { - final Map> loaded = getConfigurationsFromIndex(configTypes, false, acceptInvalid); + private void reloadConfiguration0(Collection> configTypes, boolean acceptInvalid) { + ConfigurationMap loaded = getConfigurationsFromIndex(configTypes, false, acceptInvalid); notifyConfigurationListeners(loaded); } - private void notifyConfigurationListeners(final Map> configuration) { - configCache.putAll(configuration); + private void notifyConfigurationListeners(ConfigurationMap configuration) { + configCache.putAll(configuration.rawMap()); notifyAboutChanges(configuration); } @@ -563,7 +564,7 @@ public synchronized void subscribeOnChange(ConfigurationChangeListener listener) configurationChangedListener.add(listener); } - private synchronized void notifyAboutChanges(Map> typeToConfig) { + private synchronized void notifyAboutChanges(ConfigurationMap typeToConfig) { for (ConfigurationChangeListener listener : configurationChangedListener) { try { LOGGER.debug("Notify {} listener about change configuration with type {}", listener); @@ -581,21 +582,18 @@ private synchronized void notifyAboutChanges(Map> getConfigurationsFromIndex( - Collection configTypes, - boolean logComplianceEvent - ) { + public ConfigurationMap getConfigurationsFromIndex(Collection> configTypes, boolean logComplianceEvent) { return getConfigurationsFromIndex(configTypes, logComplianceEvent, this.acceptInvalid); } - public Map> getConfigurationsFromIndex( - Collection configTypes, + public ConfigurationMap getConfigurationsFromIndex( + Collection> configTypes, boolean logComplianceEvent, boolean acceptInvalid ) { final ThreadContext threadContext = threadPool.getThreadContext(); - final Map> retVal = new HashMap<>(); + final ConfigurationMap.Builder resultBuilder = new ConfigurationMap.Builder(); try (StoredContext ctx = threadContext.stashContext()) { threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true"); @@ -609,15 +607,15 @@ public Map> getConfigurationsFromIndex( } else { LOGGER.debug("security index exists and was created with ES 7 (new layout)"); } - retVal.putAll( - validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size()) + resultBuilder.with( + validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size()) ); } else { // wait (and use new layout) LOGGER.debug("security index not exists (yet)"); - retVal.putAll( - validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size()) + resultBuilder.with( + validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size()) ); } @@ -625,20 +623,31 @@ public Map> getConfigurationsFromIndex( throw new OpenSearchException(e); } + ConfigurationMap result = resultBuilder.build(); + if (logComplianceEvent && auditLog.getComplianceConfig() != null && auditLog.getComplianceConfig().isEnabled()) { - CType configurationType = configTypes.iterator().next(); + CType configurationType = configTypes.iterator().next(); Map fields = new HashMap(); - fields.put(configurationType.toLCString(), Strings.toString(MediaTypeRegistry.JSON, retVal.get(configurationType))); + fields.put(configurationType.toLCString(), Strings.toString(MediaTypeRegistry.JSON, result.get(configurationType))); auditLog.logDocumentRead(this.securityIndex, configurationType.toLCString(), null, fields); } - return retVal; + return result; + } + + public SecurityDynamicConfiguration getUnconvertedConfigurationFromIndex(CType configType, boolean logComplianceEvent) { + ConfigurationMap map = this.getConfigurationsFromIndex(List.of(configType), logComplianceEvent, this.acceptInvalid); + SecurityDynamicConfiguration result = map.get(configType); + if (result != null && result.getAutoConvertedFrom() != null) { + result = result.getAutoConvertedFrom(); + } + + return result; } - private Map> validate(Map> conf, int expectedSize) - throws InvalidConfigException { + private ConfigurationMap validate(ConfigurationMap conf, int expectedSize) throws InvalidConfigException { - if (conf == null || conf.size() != expectedSize) { + if (conf == null || conf.size() < expectedSize) { throw new InvalidConfigException("Retrieved only partial configuration"); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java index eb82bed908..d1289471a6 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java @@ -12,7 +12,6 @@ package org.opensearch.security.dlic.rest.api; import java.io.IOException; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -376,7 +375,7 @@ protected final ValidationResult loadConfiguration(final } protected ValidationResult> loadConfiguration( - final CType cType, + final CType cType, boolean omitSensitiveData, final boolean logComplianceEvent ) { @@ -451,20 +450,18 @@ public RequestContentValidator createRequestContentValidator(Object... params) { }; } - protected abstract CType getConfigType(); + protected abstract CType getConfigType(); - protected final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { + protected final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { SecurityDynamicConfiguration loaded = securityApiDependencies.configurationRepository() - .getConfigurationsFromIndex(List.of(config), logComplianceEvent) - .get(config) + .getUnconvertedConfigurationFromIndex(config, logComplianceEvent) .deepClone(); return DynamicConfigFactory.addStatics(loaded); } - protected final SecurityDynamicConfiguration loadAndRedact(final CType config, boolean logComplianceEvent) { + protected final SecurityDynamicConfiguration loadAndRedact(final CType config, boolean logComplianceEvent) { SecurityDynamicConfiguration loaded = securityApiDependencies.configurationRepository() - .getConfigurationsFromIndex(List.of(config), logComplianceEvent) - .get(config) + .getUnconvertedConfigurationFromIndex(config, logComplianceEvent) .deepCloneWithRedaction(); return DynamicConfigFactory.addStatics(loaded); } @@ -496,7 +493,7 @@ public final void onFailure(Exception e) { public static ActionFuture saveAndUpdateConfigs( final SecurityApiDependencies dependencies, final Client client, - final CType cType, + final CType cType, final SecurityDynamicConfiguration configuration ) { final var request = createIndexRequestForConfig(dependencies, cType, configuration); @@ -506,7 +503,7 @@ public static ActionFuture saveAndUpdateConfigs( public static void saveAndUpdateConfigsAsync( final SecurityApiDependencies dependencies, final Client client, - final CType cType, + final CType cType, final SecurityDynamicConfiguration configuration, final ActionListener actionListener ) { @@ -516,7 +513,7 @@ public static void saveAndUpdateConfigsAsync( private static IndexRequest createIndexRequestForConfig( final SecurityApiDependencies dependencies, - final CType cType, + final CType cType, final SecurityDynamicConfiguration configuration ) { configuration.removeStatic(); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java index 199f6b088a..ad9aa656da 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java @@ -74,7 +74,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.INTERNALUSERS; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java index 0b5dfd1499..e54e7d87af 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java @@ -88,7 +88,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.ACTIONGROUPS; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java index b7d4761993..8462ec3fcf 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java @@ -106,7 +106,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.ALLOWLIST; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java index 997bd85bdd..a5bf9c6b9b 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java @@ -233,7 +233,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.AUDIT; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java index bc37f41d6e..b77af85db3 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java @@ -44,7 +44,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java index e1cd51e95d..1a769a9b71 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java @@ -63,7 +63,7 @@ public String getName() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java index f295ab8c1c..0ece877b20 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java @@ -69,7 +69,7 @@ public class ConfigUpgradeApiAction extends AbstractApiAction { private final static Logger LOGGER = LogManager.getLogger(ConfigUpgradeApiAction.class); - private final static Set SUPPORTED_CTYPES = ImmutableSet.of(CType.ROLES); + private final static Set> SUPPORTED_CTYPES = ImmutableSet.of(CType.ROLES); private final static String REQUEST_PARAM_CONFIGS_KEY = "configs"; @@ -130,11 +130,11 @@ void performUpgrade(final RestChannel channel, final RestRequest request, final private ValidationResult> applyDifferences( final RestRequest request, final Client client, - final List> differencesToUpdate + final List, JsonNode>> differencesToUpdate ) { try { final var updatedResources = new ArrayList>(); - for (final Tuple difference : differencesToUpdate) { + for (final Tuple, JsonNode> difference : differencesToUpdate) { updatedResources.add( loadConfiguration(difference.v1(), false, false).map( configuration -> patchEntities(request, difference.v2(), SecurityConfiguration.of(null, configuration)).map( @@ -167,7 +167,7 @@ private ValidationResult> applyDifferences( } - ValidationResult>> verifyHasDifferences(List> diffs) { + ValidationResult, JsonNode>>> verifyHasDifferences(List, JsonNode>> diffs) { if (diffs.isEmpty()) { return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("Unable to upgrade, no differences found")); } @@ -183,9 +183,9 @@ ValidationResult>> verifyHasDifferences(List>> configurationDifferences(final Set configurations) { + private ValidationResult, JsonNode>>> configurationDifferences(final Set> configurations) { try { - final var differences = new ArrayList>>(); + final var differences = new ArrayList, JsonNode>>>(); for (final var configuration : configurations) { differences.add(computeDifferenceToUpdate(configuration)); } @@ -199,7 +199,7 @@ private ValidationResult>> configurationDifferences( } } - ValidationResult> computeDifferenceToUpdate(final CType configType) { + ValidationResult, JsonNode>> computeDifferenceToUpdate(final CType configType) { return withIOException(() -> loadConfiguration(configType, false, false).map(activeRoles -> { final var activeRolesJson = Utils.convertJsonToJackson(activeRoles, true); final var defaultRolesJson = loadConfigFileAsJson(configType); @@ -208,10 +208,10 @@ ValidationResult> computeDifferenceToUpdate(final CType c })); } - private ValidationResult> getAndValidateConfigurationsToUpgrade(final RestRequest request) { + private ValidationResult>> getAndValidateConfigurationsToUpgrade(final RestRequest request) { final String[] configs = request.paramAsStringArray(REQUEST_PARAM_CONFIGS_KEY, null); - final Set configurations; + final Set> configurations; try { configurations = Optional.ofNullable(configs).map(CType::fromStringValues).orElse(SUPPORTED_CTYPES); } catch (final IllegalArgumentException iae) { @@ -261,11 +261,12 @@ private static boolean isRemoveOperation(final JsonNode node) { return node.get("op").asText().equals("remove"); } - private SecurityDynamicConfiguration loadYamlFile(final String filepath, final CType cType) throws IOException { + private SecurityDynamicConfiguration loadYamlFile(final String filepath, final CType cType) throws IOException { return ConfigHelper.fromYamlFile(filepath, cType, ConfigurationRepository.DEFAULT_CONFIG_VERSION, 0, 0); } - JsonNode loadConfigFileAsJson(final CType cType) throws IOException { + @SuppressWarnings("removal") + JsonNode loadConfigFileAsJson(final CType cType) throws IOException { final var cd = securityApiDependencies.configurationRepository().getConfigDirectory(); final var filepath = cType.configFile(Path.of(cd)).toString(); try { @@ -285,7 +286,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { throw new UnsupportedOperationException("This class supports multiple configuration types"); } @@ -353,10 +354,10 @@ public ValidationResult validate(final RestRequest request, final Json /** Tranforms config changes from a raw PATCH into simplier view */ static class ConfigItemChanges { - private final CType config; + private final CType config; private final Map> itemsGroupedByOperation; - public ConfigItemChanges(final CType config, final JsonNode differences) { + public ConfigItemChanges(final CType config, final JsonNode differences) { this.config = config; this.itemsGroupedByOperation = classifyChanges(differences); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java index 2f579ecbd9..766b57c62a 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java @@ -95,7 +95,7 @@ public void onFailure(final Exception e) { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java index 3a16028b54..00c33e0f4f 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java @@ -104,7 +104,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.INTERNALUSERS; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java index b4cf0844f8..c7af84b896 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java @@ -89,7 +89,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java index 2be5778956..53945432d9 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java @@ -84,7 +84,7 @@ public MultiTenancyConfigApiAction( } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.CONFIG; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java index ff44867bd2..d1c53721d8 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java @@ -90,7 +90,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.NODESDN; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java index 4c29d6e934..10abd83f7b 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java @@ -121,7 +121,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.ROLES; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RolesMappingApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/RolesMappingApiAction.java index ccfac457aa..5b117ec48d 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RolesMappingApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RolesMappingApiAction.java @@ -65,7 +65,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.ROLESMAPPING; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java index 2141a35460..57c23f86af 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java @@ -67,7 +67,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.CONFIG; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java index 30b6c862ee..7f4bff50ab 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java @@ -98,7 +98,7 @@ protected void consumeParameters(RestRequest request) { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/TenantsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/TenantsApiAction.java index df2289b44c..7cccd04bb1 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/TenantsApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/TenantsApiAction.java @@ -74,7 +74,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.TENANTS; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ValidateApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ValidateApiAction.java index 1d56ed80f9..58ab3884ae 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ValidateApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ValidateApiAction.java @@ -63,7 +63,7 @@ public List routes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return null; } @@ -129,7 +129,7 @@ protected void validate(RestChannel channel, RestRequest request) throws IOExcep } } - private SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent, boolean acceptInvalid) { + private SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent, boolean acceptInvalid) { SecurityDynamicConfiguration loaded = securityApiDependencies.configurationRepository() .getConfigurationsFromIndex(Collections.singleton(config), logComplianceEvent, acceptInvalid) .get(config) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/WhitelistApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/WhitelistApiAction.java index 2545bb2e23..fd71312910 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/WhitelistApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/WhitelistApiAction.java @@ -104,7 +104,7 @@ public List deprecatedRoutes() { } @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.WHITELIST; } diff --git a/src/main/java/org/opensearch/security/rest/TenantInfoAction.java b/src/main/java/org/opensearch/security/rest/TenantInfoAction.java index bd911463d4..d7b3ef3d1f 100644 --- a/src/main/java/org/opensearch/security/rest/TenantInfoAction.java +++ b/src/main/java/org/opensearch/security/rest/TenantInfoAction.java @@ -179,7 +179,7 @@ private boolean isAuthorized() { return false; } - private final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { + private final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { SecurityDynamicConfiguration loaded = configurationRepository.getConfigurationsFromIndex( Collections.singleton(config), logComplianceEvent diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java index b710602d54..f7105e2386 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java @@ -48,6 +48,7 @@ import org.opensearch.security.auth.internal.InternalAuthenticationBackend; import org.opensearch.security.configuration.ClusterInfoHolder; import org.opensearch.security.configuration.ConfigurationChangeListener; +import org.opensearch.security.configuration.ConfigurationMap; import org.opensearch.security.configuration.ConfigurationRepository; import org.opensearch.security.configuration.StaticResourceException; import org.opensearch.security.hasher.PasswordHasher; @@ -56,11 +57,8 @@ import org.opensearch.security.securityconf.impl.NodesDn; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.WhitelistingSettings; -import org.opensearch.security.securityconf.impl.v6.ActionGroupsV6; import org.opensearch.security.securityconf.impl.v6.ConfigV6; import org.opensearch.security.securityconf.impl.v6.InternalUserV6; -import org.opensearch.security.securityconf.impl.v6.RoleMappingsV6; -import org.opensearch.security.securityconf.impl.v6.RoleV6; import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; import org.opensearch.security.securityconf.impl.v7.ConfigV7; import org.opensearch.security.securityconf.impl.v7.InternalUserV7; @@ -77,17 +75,17 @@ public class DynamicConfigFactory implements Initializable, ConfigurationChangeListener { public static final EventBusBuilder EVENT_BUS_BUILDER = EventBus.builder(); - private static SecurityDynamicConfiguration staticRoles = SecurityDynamicConfiguration.empty(); - private static SecurityDynamicConfiguration staticActionGroups = SecurityDynamicConfiguration.empty(); - private static SecurityDynamicConfiguration staticTenants = SecurityDynamicConfiguration.empty(); + private static SecurityDynamicConfiguration staticRoles = SecurityDynamicConfiguration.empty(CType.ROLES); + private static SecurityDynamicConfiguration staticActionGroups = SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS); + private static SecurityDynamicConfiguration staticTenants = SecurityDynamicConfiguration.empty(CType.TENANTS); private static final WhitelistingSettings defaultWhitelistingSettings = new WhitelistingSettings(); private static final AllowlistingSettings defaultAllowlistingSettings = new AllowlistingSettings(); private static final AuditConfig defaultAuditConfig = AuditConfig.from(Settings.EMPTY); static void resetStatics() { - staticRoles = SecurityDynamicConfiguration.empty(); - staticActionGroups = SecurityDynamicConfiguration.empty(); - staticTenants = SecurityDynamicConfiguration.empty(); + staticRoles = SecurityDynamicConfiguration.empty(CType.ROLES); + staticActionGroups = SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS); + staticTenants = SecurityDynamicConfiguration.empty(CType.TENANTS); } private void loadStaticConfig() throws IOException { @@ -176,17 +174,17 @@ public DynamicConfigFactory( @Override @SuppressWarnings("unchecked") - public void onChange(Map> typeToConfig) { + public void onChange(ConfigurationMap typeToConfig) { - SecurityDynamicConfiguration actionGroups = cr.getConfiguration(CType.ACTIONGROUPS); + SecurityDynamicConfiguration actionGroups = cr.getConfiguration(CType.ACTIONGROUPS); config = cr.getConfiguration(CType.CONFIG); - SecurityDynamicConfiguration internalusers = cr.getConfiguration(CType.INTERNALUSERS); - SecurityDynamicConfiguration roles = cr.getConfiguration(CType.ROLES); - SecurityDynamicConfiguration rolesmapping = cr.getConfiguration(CType.ROLESMAPPING); - SecurityDynamicConfiguration tenants = cr.getConfiguration(CType.TENANTS); - SecurityDynamicConfiguration nodesDn = cr.getConfiguration(CType.NODESDN); - SecurityDynamicConfiguration whitelistingSetting = cr.getConfiguration(CType.WHITELIST); - SecurityDynamicConfiguration allowlistingSetting = cr.getConfiguration(CType.ALLOWLIST); + SecurityDynamicConfiguration internalusers = cr.getConfiguration(CType.INTERNALUSERS); + SecurityDynamicConfiguration roles = cr.getConfiguration(CType.ROLES); + SecurityDynamicConfiguration rolesmapping = cr.getConfiguration(CType.ROLESMAPPING); + SecurityDynamicConfiguration tenants = cr.getConfiguration(CType.TENANTS); + SecurityDynamicConfiguration nodesDn = cr.getConfiguration(CType.NODESDN); + SecurityDynamicConfiguration whitelistingSetting = cr.getConfiguration(CType.WHITELIST); + SecurityDynamicConfiguration allowlistingSetting = cr.getConfiguration(CType.ALLOWLIST); if (log.isDebugEnabled()) { String logmsg = "current config (because of " @@ -244,77 +242,48 @@ public void onChange(Map> typeToConfig) { final InternalUsersModel ium; final ConfigModel cm; final NodesDnModel nm = new NodesDnModelImpl(nodesDn); - final WhitelistingSettings whitelist = (WhitelistingSettings) cr.getConfiguration(CType.WHITELIST).getCEntry("config"); - final AllowlistingSettings allowlist = (AllowlistingSettings) cr.getConfiguration(CType.ALLOWLIST).getCEntry("config"); - final AuditConfig audit = (AuditConfig) cr.getConfiguration(CType.AUDIT).getCEntry("config"); + final WhitelistingSettings whitelist = cr.getConfiguration(CType.WHITELIST).getCEntry("config"); + final AllowlistingSettings allowlist = cr.getConfiguration(CType.ALLOWLIST).getCEntry("config"); + final AuditConfig audit = cr.getConfiguration(CType.AUDIT).getCEntry("config"); - if (config.getImplementingClass() == ConfigV7.class) { - // statics - - if (roles.containsAny(staticRoles)) { - throw new StaticResourceException("Cannot override static roles"); - } - if (!roles.add(staticRoles) && !staticRoles.getCEntries().isEmpty()) { - throw new StaticResourceException("Unable to load static roles"); - } - - log.debug("Static roles loaded ({})", staticRoles.getCEntries().size()); + if (roles.containsAny(staticRoles)) { + throw new StaticResourceException("Cannot override static roles"); + } + if (!roles.add(staticRoles) && !staticRoles.getCEntries().isEmpty()) { + throw new StaticResourceException("Unable to load static roles"); + } - if (actionGroups.containsAny(staticActionGroups)) { - throw new StaticResourceException("Cannot override static action groups"); - } - if (!actionGroups.add(staticActionGroups) && !staticActionGroups.getCEntries().isEmpty()) { - throw new StaticResourceException("Unable to load static action groups"); - } + log.debug("Static roles loaded ({})", staticRoles.getCEntries().size()); - log.debug("Static action groups loaded ({})", staticActionGroups.getCEntries().size()); + if (actionGroups.containsAny(staticActionGroups)) { + throw new StaticResourceException("Cannot override static action groups"); + } + if (!actionGroups.add(staticActionGroups) && !staticActionGroups.getCEntries().isEmpty()) { + throw new StaticResourceException("Unable to load static action groups"); + } - if (tenants.containsAny(staticTenants)) { - throw new StaticResourceException("Cannot override static tenants"); - } - if (!tenants.add(staticTenants) && !staticTenants.getCEntries().isEmpty()) { - throw new StaticResourceException("Unable to load static tenants"); - } + log.debug("Static action groups loaded ({})", staticActionGroups.getCEntries().size()); - log.debug("Static tenants loaded ({})", staticTenants.getCEntries().size()); - - log.debug( - "Static configuration loaded (total roles: {}/total action groups: {}/total tenants: {})", - roles.getCEntries().size(), - actionGroups.getCEntries().size(), - tenants.getCEntries().size() - ); - - // rebuild v7 Models - dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.cih); - ium = new InternalUsersModelV7( - (SecurityDynamicConfiguration) internalusers, - (SecurityDynamicConfiguration) roles, - (SecurityDynamicConfiguration) rolesmapping - ); - cm = new ConfigModelV7( - (SecurityDynamicConfiguration) roles, - (SecurityDynamicConfiguration) rolesmapping, - (SecurityDynamicConfiguration) actionGroups, - (SecurityDynamicConfiguration) tenants, - dcm, - opensearchSettings - ); + if (tenants.containsAny(staticTenants)) { + throw new StaticResourceException("Cannot override static tenants"); + } + if (!tenants.add(staticTenants) && !staticTenants.getCEntries().isEmpty()) { + throw new StaticResourceException("Unable to load static tenants"); + } - } else { + log.debug("Static tenants loaded ({})", staticTenants.getCEntries().size()); - // rebuild v6 Models - dcm = new DynamicConfigModelV6(getConfigV6(config), opensearchSettings, configPath, iab); - ium = new InternalUsersModelV6((SecurityDynamicConfiguration) internalusers); - cm = new ConfigModelV6( - (SecurityDynamicConfiguration) roles, - (SecurityDynamicConfiguration) actionGroups, - (SecurityDynamicConfiguration) rolesmapping, - dcm, - opensearchSettings - ); + log.debug( + "Static configuration loaded (total roles: {}/total action groups: {}/total tenants: {})", + roles.getCEntries().size(), + actionGroups.getCEntries().size(), + tenants.getCEntries().size() + ); - } + // rebuild v7 Models + dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.cih); + ium = new InternalUsersModelV7(internalusers, roles, rolesmapping); + cm = new ConfigModelV7(roles, rolesmapping, actionGroups, tenants, dcm, opensearchSettings); // notify subscribers eventBus.post(cm); @@ -469,12 +438,9 @@ private static class NodesDnModelImpl extends NodesDnModel { SecurityDynamicConfiguration configuration; - @SuppressWarnings("unchecked") - public NodesDnModelImpl(SecurityDynamicConfiguration configuration) { + public NodesDnModelImpl(SecurityDynamicConfiguration configuration) { super(); - this.configuration = null == configuration.getCType() - ? SecurityDynamicConfiguration.empty() - : (SecurityDynamicConfiguration) configuration; + this.configuration = null == configuration.getCType() ? SecurityDynamicConfiguration.empty(CType.NODESDN) : configuration; } @Override diff --git a/src/main/java/org/opensearch/security/securityconf/Migration.java b/src/main/java/org/opensearch/security/securityconf/Migration.java index b5e3ef973d..8153360270 100644 --- a/src/main/java/org/opensearch/security/securityconf/Migration.java +++ b/src/main/java/org/opensearch/security/securityconf/Migration.java @@ -27,13 +27,18 @@ package org.opensearch.security.securityconf; +import java.io.File; +import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.Map.Entry; import java.util.Set; +import com.fasterxml.jackson.databind.JavaType; + import org.opensearch.common.collect.Tuple; import org.opensearch.core.common.Strings; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.securityconf.impl.AllowlistingSettings; import org.opensearch.security.securityconf.impl.CType; @@ -60,20 +65,11 @@ public static Tuple, SecurityDynamicConfigu SecurityDynamicConfiguration rms6 ) throws MigrationException { - final SecurityDynamicConfiguration r7 = SecurityDynamicConfiguration.empty(); - r7.setCType(r6cs.getCType()); + final SecurityDynamicConfiguration r7 = SecurityDynamicConfiguration.empty(CType.ROLES); r7.set_meta(new Meta()); r7.get_meta().setConfig_version(2); r7.get_meta().setType("roles"); - final SecurityDynamicConfiguration t7 = SecurityDynamicConfiguration.empty(); - t7.setCType(CType.TENANTS); - t7.set_meta(new Meta()); - t7.get_meta().setConfig_version(2); - t7.get_meta().setType("tenants"); - - Set dedupTenants = new HashSet<>(); - for (final Entry r6e : r6cs.getCEntries().entrySet()) { final String roleName = r6e.getKey(); final RoleV6 r6 = r6e.getValue(); @@ -86,10 +82,6 @@ public static Tuple, SecurityDynamicConfigu } r7.putCEntry(roleName, new RoleV7(r6)); - - for (Entry tenant : r6.getTenants().entrySet()) { - dedupTenants.add(tenant.getKey()); - } } if (rms6 != null) { @@ -107,20 +99,40 @@ public static Tuple, SecurityDynamicConfigu } } + return new Tuple, SecurityDynamicConfiguration>(r7, createTenants(r6cs)); + } + + /** + * This creates a tenant configuration from the tenant names in a `SecurityDynamicConfiguration` configuration. + * The v6 configuration did not yet have an explicit file to specify all tenants. Thus, these need to be collected + * this way. + */ + public static SecurityDynamicConfiguration createTenants(SecurityDynamicConfiguration r6cs) { + Set dedupTenants = new HashSet<>(); + + for (final Entry entry : r6cs.getCEntries().entrySet()) { + for (Entry tenant : entry.getValue().getTenants().entrySet()) { + dedupTenants.add(tenant.getKey()); + } + } + + SecurityDynamicConfiguration tenantConfig = SecurityDynamicConfiguration.empty(CType.TENANTS, 2); + tenantConfig.set_meta(new Meta()); + tenantConfig.get_meta().setConfig_version(2); + tenantConfig.get_meta().setType("tenants"); + for (String tenantName : dedupTenants) { TenantV7 entry = new TenantV7(); entry.setDescription("Migrated from v6"); - t7.putCEntry(tenantName, entry); + tenantConfig.putCEntry(tenantName, entry); } - return new Tuple, SecurityDynamicConfiguration>(r7, t7); - + return tenantConfig; } public static SecurityDynamicConfiguration migrateConfig(SecurityDynamicConfiguration r6cs) throws MigrationException { - final SecurityDynamicConfiguration c7 = SecurityDynamicConfiguration.empty(); - c7.setCType(r6cs.getCType()); + final SecurityDynamicConfiguration c7 = SecurityDynamicConfiguration.empty(CType.CONFIG); c7.set_meta(new Meta()); c7.get_meta().setConfig_version(2); c7.get_meta().setType("config"); @@ -142,8 +154,7 @@ public static SecurityDynamicConfiguration migrateConfig(SecurityDynam } public static SecurityDynamicConfiguration migrateNodesDn(SecurityDynamicConfiguration nodesDn) { - final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(); - migrated.setCType(nodesDn.getCType()); + final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(CType.NODESDN); migrated.set_meta(new Meta()); migrated.get_meta().setConfig_version(2); migrated.get_meta().setType("nodesdn"); @@ -157,8 +168,7 @@ public static SecurityDynamicConfiguration migrateNodesDn(SecurityDynam public static SecurityDynamicConfiguration migrateWhitelistingSetting( SecurityDynamicConfiguration whitelistingSetting ) { - final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(); - migrated.setCType(whitelistingSetting.getCType()); + final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(CType.WHITELIST); migrated.set_meta(new Meta()); migrated.get_meta().setConfig_version(2); migrated.get_meta().setType("whitelist"); @@ -172,8 +182,7 @@ public static SecurityDynamicConfiguration migrateWhitelis public static SecurityDynamicConfiguration migrateAllowlistingSetting( SecurityDynamicConfiguration allowlistingSetting ) { - final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(); - migrated.setCType(allowlistingSetting.getCType()); + final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(CType.ALLOWLIST); migrated.set_meta(new Meta()); migrated.get_meta().setConfig_version(2); migrated.get_meta().setType("whitelist"); @@ -186,8 +195,7 @@ public static SecurityDynamicConfiguration migrateAllowlis public static SecurityDynamicConfiguration migrateInternalUsers(SecurityDynamicConfiguration r6is) throws MigrationException { - final SecurityDynamicConfiguration i7 = SecurityDynamicConfiguration.empty(); - i7.setCType(r6is.getCType()); + final SecurityDynamicConfiguration i7 = SecurityDynamicConfiguration.empty(CType.INTERNALUSERS); i7.set_meta(new Meta()); i7.get_meta().setConfig_version(2); i7.get_meta().setType("internalusers"); @@ -204,18 +212,15 @@ public static SecurityDynamicConfiguration migrateInternalUsers( public static SecurityDynamicConfiguration migrateActionGroups(SecurityDynamicConfiguration r6as) throws MigrationException { - final SecurityDynamicConfiguration a7 = SecurityDynamicConfiguration.empty(); - a7.setCType(r6as.getCType()); + final SecurityDynamicConfiguration a7 = SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS); a7.set_meta(new Meta()); a7.get_meta().setConfig_version(2); a7.get_meta().setType("actiongroups"); - if (r6as.getImplementingClass().isAssignableFrom(List.class)) { - for (final Entry r6a : r6as.getCEntries().entrySet()) { + for (final Entry r6a : r6as.getCEntries().entrySet()) { + if (r6a.getValue() instanceof List) { a7.putCEntry(r6a.getKey(), new ActionGroupsV7(r6a.getKey(), (List) r6a.getValue())); - } - } else { - for (final Entry r6a : r6as.getCEntries().entrySet()) { + } else { a7.putCEntry(r6a.getKey(), new ActionGroupsV7(r6a.getKey(), (ActionGroupsV6) r6a.getValue())); } } @@ -225,8 +230,7 @@ public static SecurityDynamicConfiguration migrateActionGroups(S public static SecurityDynamicConfiguration migrateRoleMappings(SecurityDynamicConfiguration r6rms) throws MigrationException { - final SecurityDynamicConfiguration rms7 = SecurityDynamicConfiguration.empty(); - rms7.setCType(r6rms.getCType()); + final SecurityDynamicConfiguration rms7 = SecurityDynamicConfiguration.empty(CType.ROLESMAPPING); rms7.set_meta(new Meta()); rms7.get_meta().setConfig_version(2); rms7.get_meta().setType("rolesmapping"); @@ -239,8 +243,7 @@ public static SecurityDynamicConfiguration migrateRoleMappings(S } public static SecurityDynamicConfiguration migrateAudit(SecurityDynamicConfiguration audit) { - final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(); - migrated.setCType(audit.getCType()); + final SecurityDynamicConfiguration migrated = SecurityDynamicConfiguration.empty(CType.AUDIT); migrated.set_meta(new Meta()); migrated.get_meta().setConfig_version(2); migrated.get_meta().setType("audit"); @@ -251,4 +254,13 @@ public static SecurityDynamicConfiguration migrateAudit(SecurityDyn return migrated; } + public static SecurityDynamicConfiguration readYaml(File file, Class type) throws IOException { + if (!file.exists()) { + return SecurityDynamicConfiguration.empty(null); + } + + JavaType javaType = DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, type); + return DefaultObjectMapper.YAML_MAPPER.readValue(file, javaType); + } + } 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 8a51686225..292104e6be 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/CType.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/CType.java @@ -27,17 +27,22 @@ package org.opensearch.security.securityconf.impl; +import java.io.IOException; import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; -import java.util.List; +import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; -import com.google.common.collect.ImmutableMap; +import com.fasterxml.jackson.databind.JavaType; +import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.NonValidatingObjectMapper; import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.securityconf.impl.v6.ActionGroupsV6; import org.opensearch.security.securityconf.impl.v6.ConfigV6; @@ -51,63 +56,154 @@ import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.securityconf.impl.v7.TenantV7; -public enum CType { - - ACTIONGROUPS(toMap(0, List.class, 1, ActionGroupsV6.class, 2, ActionGroupsV7.class), "action_groups.yml", false), - ALLOWLIST(toMap(1, AllowlistingSettings.class, 2, AllowlistingSettings.class), "allowlist.yml", true), - AUDIT(toMap(1, AuditConfig.class, 2, AuditConfig.class), "audit.yml", true), - CONFIG(toMap(1, ConfigV6.class, 2, ConfigV7.class), "config.yml", false), - INTERNALUSERS(toMap(1, InternalUserV6.class, 2, InternalUserV7.class), "internal_users.yml", false), - NODESDN(toMap(1, NodesDn.class, 2, NodesDn.class), "nodes_dn.yml", true), - ROLES(toMap(1, RoleV6.class, 2, RoleV7.class), "roles.yml", false), - ROLESMAPPING(toMap(1, RoleMappingsV6.class, 2, RoleMappingsV7.class), "roles_mapping.yml", false), - TENANTS(toMap(2, TenantV7.class), "tenants.yml", false), - WHITELIST(toMap(1, WhitelistingSettings.class, 2, WhitelistingSettings.class), "whitelist.yml", true); - - public static final List REQUIRED_CONFIG_FILES = Arrays.stream(CType.values()) - .filter(Predicate.not(CType::emptyIfMissing)) - .collect(Collectors.toList()); +/** + * Identifies configuration types. A `CType` instance has a 1-to-1 relationship with a configuration + * business object class, which is referenced in its generic parameter. + *

+ * Additionally, a `CType` can reference older versions of these business objects via the `oldConfigVersions` + * property. + *

+ * In earlier versions, `CType` was an `enum` which leads to a few peculiarities: + *

    + *
  • Each instance needs to have a unique `id` which corresponds to the `ord` property + * from Java enums. OpenSearch uses these ids for its own transport message serialization. + * These ids must not be changed, otherwise backward compatibility will be broken. + *
  • Each instance has several names. One, that is used for identifying the document + * in document ids. Another one that is used for identifying the config type by yaml file names. There + * are a few inconsistencies - for the future, it is recommendable to use identical names. + *
+ */ +public class CType implements Comparable> { - public static final List NOT_REQUIRED_CONFIG_FILES = Arrays.stream(CType.values()) - .filter(CType::emptyIfMissing) - .collect(Collectors.toList()); + private final static Set> allSet = new HashSet<>(); + private static Map> nameToInstanceMap = new HashMap<>(); + private static Map> ordToInstanceMap = new HashMap<>(); - private final Map> implementations; + public static final CType ACTIONGROUPS = new CType<>( + "actiongroups", + "action_groups", + ActionGroupsV7.class, + 0, + false, + new OldConfigVersion<>(1, ActionGroupsV6.class, ActionGroupsV7::new) + ); + public static final CType ALLOWLIST = new CType<>("allowlist", "allowlist", AllowlistingSettings.class, 1, true); + public static final CType AUDIT = new CType<>("audit", "audit", AuditConfig.class, 2, true); + public static final CType CONFIG = new CType<>( + "config", + "config", + ConfigV7.class, + 3, + false, + new OldConfigVersion<>(1, ConfigV6.class, ConfigV7::new, ConfigV6::convertMapKeyToV7) + ); + public static final CType INTERNALUSERS = new CType<>( + "internalusers", + "internal_users", + InternalUserV7.class, + 4, + false, + new OldConfigVersion<>(1, InternalUserV6.class, InternalUserV7::new) + ); + public static final CType NODESDN = new CType<>("nodesdn", "nodes_dn", NodesDn.class, 5, true); + public static final CType ROLES = new CType<>( + "roles", + "roles", + RoleV7.class, + 6, + false, + new OldConfigVersion<>(1, RoleV6.class, RoleV7::new) + ); + public static final CType ROLESMAPPING = new CType<>( + "rolesmapping", + "roles_mapping", + RoleMappingsV7.class, + 7, + false, + new OldConfigVersion<>(1, RoleMappingsV6.class, RoleMappingsV7::new) + ); + public static final CType TENANTS = new CType<>("tenants", "tenants", TenantV7.class, 8, false); + 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; + private final OldConfigVersion[] oldConfigVersions; + private final int id; - private CType(Map> implementations, final String configFileName, final boolean emptyIfMissing) { - this.implementations = implementations; - this.configFileName = configFileName; + @SafeVarargs + @SuppressWarnings("varargs") + private CType( + String name, + String configFileName, + Class configClass, + int id, + boolean emptyIfMissing, + OldConfigVersion... oldConfigVersions + ) { + this.name = name; + this.nameUpperCase = name.toUpperCase(); + this.configClass = configClass; + this.id = id; + this.configFileName = configFileName + ".yml"; this.emptyIfMissing = emptyIfMissing; + this.oldConfigVersions = oldConfigVersions; + + allSet.add(this); + nameToInstanceMap.put(name, this); + ordToInstanceMap.put(id, this); + } + + public Class getConfigClass() { + return this.configClass; } public boolean emptyIfMissing() { return emptyIfMissing; } - public Map> getImplementationClass() { - return Collections.unmodifiableMap(implementations); + public String toLCString() { + return this.name; } - public static CType fromString(String value) { - return CType.valueOf(value.toUpperCase()); + public String name() { + return this.name; } - public String toLCString() { - return this.toString().toLowerCase(); + public int getOrd() { + return id; + } + + public static CType fromString(String value) { + return nameToInstanceMap.get(value.toLowerCase()); + } + + public static Set> values() { + return Collections.unmodifiableSet(allSet); } public static Set lcStringValues() { - return Arrays.stream(CType.values()).map(CType::toLCString).collect(Collectors.toSet()); + return CType.values().stream().map(CType::toLCString).collect(Collectors.toSet()); } - public static Set fromStringValues(String[] strings) { + public static Set> fromStringValues(String[] strings) { return Arrays.stream(strings).map(CType::fromString).collect(Collectors.toSet()); } + public static CType fromOrd(int ord) { + return ordToInstanceMap.get(ord); + } + + public static Set> requiredConfigTypes() { + return values().stream().filter(Predicate.not(CType::emptyIfMissing)).collect(Collectors.toUnmodifiableSet()); + } + + public static Set> notRequiredConfigTypes() { + return values().stream().filter(CType::emptyIfMissing).collect(Collectors.toUnmodifiableSet()); + } + public Path configFile(final Path configDir) { return configDir.resolve(this.configFileName); } @@ -116,11 +212,98 @@ public String configFileName() { return configFileName; } - private static Map> toMap(Object... objects) { - final ImmutableMap.Builder> map = ImmutableMap.builder(); - for (int i = 0; i < objects.length; i = i + 2) { - map.put((Integer) objects[i], (Class) objects[i + 1]); + public OldConfigVersion findOldConfigVersion(int versionNumber) { + for (OldConfigVersion oldConfigVersion : this.oldConfigVersions) { + if (versionNumber == oldConfigVersion.versionNumber) { + return oldConfigVersion; + } } - return map.build(); + + return null; + } + + @Override + public int compareTo(CType cType) { + return this.id - cType.id; + } + + @Override + public String toString() { + return this.nameUpperCase; + } + + @Override + public boolean equals(Object other) { + if (other instanceof CType) { + return ((CType) other).id == this.id; + } else { + return false; + } + } + + @Override + public int hashCode() { + return id; + } + + public static class OldConfigVersion { + private final int versionNumber; + private final Class oldType; + 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 entryConversionFunction, + Function mapKeyConversionFunction + ) { + this.versionNumber = versionNumber; + this.oldType = oldType; + this.entryConversionFunction = entryConversionFunction; + this.mapKeyConversionFunction = mapKeyConversionFunction; + } + + public int getVersionNumber() { + return versionNumber; + } + + public Class getOldType() { + return oldType; + } + + public Function getEntryConversionFunction() { + return entryConversionFunction; + } + + public SecurityDynamicConfiguration parseJson(CType ctype, String json, boolean acceptInvalid) + throws IOException { + JavaType javaType = DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, oldType); + + if (acceptInvalid) { + return convert(NonValidatingObjectMapper.readValue(json, javaType), ctype); + } else { + return convert(DefaultObjectMapper.readValue(json, javaType), ctype); + } + } + + public SecurityDynamicConfiguration convert(SecurityDynamicConfiguration oldConfig, CType ctype) { + SecurityDynamicConfiguration newConfig = SecurityDynamicConfiguration.empty(ctype); + newConfig.setAutoConvertedFrom(oldConfig); + + for (Map.Entry oldEntry : oldConfig.getCEntries().entrySet()) { + newConfig.putCEntry(mapKeyConversionFunction.apply(oldEntry.getKey()), entryConversionFunction.apply(oldEntry.getValue())); + } + + return newConfig; + } + } } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/Meta.java b/src/main/java/org/opensearch/security/securityconf/impl/Meta.java index 2fa4ced06a..c97f72e6b8 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/Meta.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/Meta.java @@ -34,7 +34,7 @@ public class Meta { private String type; private int config_version; - private CType cType; + private CType cType; public String getType() { return type; @@ -54,7 +54,7 @@ public void setConfig_version(int config_version) { } @JsonIgnore - public CType getCType() { + public CType getCType() { return cType; } 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 e1dd1602ac..d99c9ad2d0 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -46,13 +46,14 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.NonValidatingObjectMapper; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; public class SecurityDynamicConfiguration implements ToXContent { + public static final int CURRENT_VERSION = 2; + private static final TypeReference> typeRefMSO = new TypeReference>() { }; @@ -62,11 +63,27 @@ public class SecurityDynamicConfiguration implements ToXContent { private final Object modificationLock = new Object(); private long seqNo = -1; private long primaryTerm = -1; - private CType ctype; + /** + * CType with a proper generic parameter. Will be null for older config versions. If you need the ctype anyway, + * refer to ctypeUnsafe. + */ + private CType ctype; + /** + * CType with a ? generic parameter. Will be not null for older config versions. + */ + private CType ctypeUnsafe; private int version = -1; - public static SecurityDynamicConfiguration empty() { - return new SecurityDynamicConfiguration(); + private SecurityDynamicConfiguration autoConvertedFrom; + + public static SecurityDynamicConfiguration empty(CType ctype) { + return new SecurityDynamicConfiguration(ctype); + } + + public static SecurityDynamicConfiguration empty(CType ctype, int version) { + SecurityDynamicConfiguration result = new SecurityDynamicConfiguration(ctype); + result.version = version; + return result; } @JsonIgnore @@ -74,14 +91,18 @@ public boolean notEmpty() { return !centries.isEmpty(); } - public static SecurityDynamicConfiguration fromJson(String json, CType ctype, int version, long seqNo, long primaryTerm) + public static SecurityDynamicConfiguration fromJson(String json, CType ctype, int version, long seqNo, long primaryTerm) throws IOException { 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, + CType ctype, int version, long seqNo, long primaryTerm, @@ -89,22 +110,26 @@ public static SecurityDynamicConfiguration fromJson( ) throws IOException { SecurityDynamicConfiguration sdc = null; if (ctype != null) { - final Class implementationClass = ctype.getImplementationClass().get(version); - if (implementationClass == null) { - throw new IllegalArgumentException("No implementation class found for " + ctype + " and config version " + version); - } - if (acceptInvalid && version < 2) { - sdc = NonValidatingObjectMapper.readValue( - json, - NonValidatingObjectMapper.getTypeFactory() - .constructParametricType(SecurityDynamicConfiguration.class, implementationClass) - ); + CType.OldConfigVersion oldConfigVersion = ctype.findOldConfigVersion(version); + + 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()); + } + if (sdc.getAutoConvertedFrom() != null) { + sdc.getAutoConvertedFrom().version = version; + } + version = CURRENT_VERSION; } else { sdc = DefaultObjectMapper.readValue( json, - DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, implementationClass) + DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, ctype.getConfigClass()) ); } + validate(sdc, version, ctype); } else { @@ -112,6 +137,61 @@ public static SecurityDynamicConfiguration fromJson( } sdc.ctype = ctype; + sdc.ctypeUnsafe = ctype; + sdc.seqNo = seqNo; + sdc.primaryTerm = primaryTerm; + sdc.version = version; + + if (sdc.getAutoConvertedFrom() != null) { + sdc.getAutoConvertedFrom().seqNo = seqNo; + sdc.getAutoConvertedFrom().primaryTerm = primaryTerm; + sdc.getAutoConvertedFrom().ctypeUnsafe = ctype; + } + + return sdc; + } + + /** + * Creates the SecurityDynamicConfiguration instance from the given JSON . 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 fromJsonWithoutAutoConversion( + String json, + CType ctype, + int version, + long seqNo, + long primaryTerm + ) throws IOException { + return fromNodeWithoutAutoConversion(DefaultObjectMapper.readTree(json), ctype, version, seqNo, primaryTerm); + } + + /** + * 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; @@ -122,18 +202,16 @@ public static SecurityDynamicConfiguration fromJson( /** * For testing only */ - public static SecurityDynamicConfiguration fromMap(Map map, CType ctype, int version) - throws JsonProcessingException { - Class implementationClass = ctype.getImplementationClass().get(version); + public static SecurityDynamicConfiguration fromMap(Map map, CType ctype) throws JsonProcessingException { SecurityDynamicConfiguration result = DefaultObjectMapper.objectMapper.convertValue( map, - DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, implementationClass) + DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, ctype.getConfigClass()) ); result.ctype = ctype; return result; } - public static void validate(SecurityDynamicConfiguration sdc, int version, CType ctype) throws IOException { + public static void validate(SecurityDynamicConfiguration sdc, int version, CType ctype) throws IOException { if (version < 2 && sdc.get_meta() != null) { throw new IOException("A version of " + version + " can not have a _meta key for " + ctype); } @@ -154,9 +232,15 @@ public static void validate(SecurityDynamicConfiguration sdc, int version, CT } - public static SecurityDynamicConfiguration fromNode(JsonNode json, CType ctype, int version, long seqNo, long primaryTerm) + public static SecurityDynamicConfiguration fromNode(JsonNode json, CType ctype, int version, long seqNo, long primaryTerm) throws IOException { - return fromJson(DefaultObjectMapper.writeValueAsString(json, false), ctype, version, seqNo, primaryTerm); + return SecurityDynamicConfiguration.fromJson( + DefaultObjectMapper.writeValueAsString(json, false), + ctype, + version, + seqNo, + primaryTerm + ); } // for Jackson @@ -164,6 +248,12 @@ private SecurityDynamicConfiguration() { super(); } + private SecurityDynamicConfiguration(CType ctype) { + super(); + this.ctype = ctype; + this.ctypeUnsafe = ctype; + } + private Meta _meta; public Meta get_meta() { @@ -295,13 +385,14 @@ public long getPrimaryTerm() { } @JsonIgnore - public CType getCType() { - return ctype; + public void setSeqNoPrimaryTerm(long seqNo, long primaryTerm) { + this.seqNo = seqNo; + this.primaryTerm = primaryTerm; } @JsonIgnore - public void setCType(CType ctype) { - this.ctype = ctype; + public CType getCType() { + return ctype; } @JsonIgnore @@ -309,24 +400,76 @@ public int getVersion() { return version; } + @JsonIgnore + public SecurityDynamicConfiguration getAutoConvertedFrom() { + return autoConvertedFrom; + } + + @JsonIgnore + public void setAutoConvertedFrom(SecurityDynamicConfiguration autoConvertedFrom) { + this.autoConvertedFrom = autoConvertedFrom; + } + @JsonIgnore public Class getImplementingClass() { - return getCType() == null ? null : getCType().getImplementationClass().get(getVersion()); + return getCType() == null ? null : getCType().getConfigClass(); } + @SuppressWarnings("unchecked") @JsonIgnore public SecurityDynamicConfiguration deepClone() { try { - return fromJson(DefaultObjectMapper.writeValueAsString(this, false), ctype, version, seqNo, primaryTerm); + if (ctype != null) { + SecurityDynamicConfiguration result = fromJson( + DefaultObjectMapper.writeValueAsString(this, false), + ctype, + version, + seqNo, + primaryTerm + ); + result.autoConvertedFrom = this.autoConvertedFrom; + return result; + } else { + // We are on a pre-v7 config version. This can be only if we skipped auto conversion. So, we do here the same. + SecurityDynamicConfiguration result = (SecurityDynamicConfiguration) fromJsonWithoutAutoConversion( + DefaultObjectMapper.writeValueAsString(this, false), + ctypeUnsafe, + version, + seqNo, + primaryTerm + ); + return result; + } } catch (Exception e) { throw ExceptionsHelper.convertToOpenSearchException(e); } } + @SuppressWarnings("unchecked") @JsonIgnore public SecurityDynamicConfiguration deepCloneWithRedaction() { try { - return fromJson(DefaultObjectMapper.writeValueAsStringAndRedactSensitive(this), ctype, version, seqNo, primaryTerm); + if (ctype != null) { + SecurityDynamicConfiguration result = fromJson( + DefaultObjectMapper.writeValueAsStringAndRedactSensitive(this), + ctype, + version, + seqNo, + primaryTerm + ); + result.autoConvertedFrom = this.autoConvertedFrom; + return result; + } else { + // We are on a pre-v7 config version. This can be only if we skipped auto conversion. So, we do here the same. + SecurityDynamicConfiguration result = (SecurityDynamicConfiguration) fromJsonWithoutAutoConversion( + DefaultObjectMapper.writeValueAsStringAndRedactSensitive(this), + ctypeUnsafe, + version, + seqNo, + primaryTerm + ); + return result; + } } catch (Exception e) { throw ExceptionsHelper.convertToOpenSearchException(e); } 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/main/java/org/opensearch/security/securityconf/impl/v7/ActionGroupsV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ActionGroupsV7.java index 9ec9c25e5d..44382cb07e 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ActionGroupsV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ActionGroupsV7.java @@ -58,6 +58,14 @@ public ActionGroupsV7(String agName, ActionGroupsV6 ag6) { description = "Migrated from v6"; } + public ActionGroupsV7(ActionGroupsV6 ag6) { + reserved = ag6.isReserved(); + hidden = ag6.isHidden(); + allowed_actions = ag6.getPermissions(); + type = "unknown"; + description = "Migrated from v6"; + } + public ActionGroupsV7(String key, List allowed_actions) { this.allowed_actions = allowed_actions; type = "unknown"; diff --git a/src/main/java/org/opensearch/security/state/SecurityConfig.java b/src/main/java/org/opensearch/security/state/SecurityConfig.java index f8de098365..79f9461875 100644 --- a/src/main/java/org/opensearch/security/state/SecurityConfig.java +++ b/src/main/java/org/opensearch/security/state/SecurityConfig.java @@ -26,20 +26,20 @@ public class SecurityConfig implements Writeable, ToXContent { - private final CType type; + private final CType type; private final Instant lastModified; private final String hash; - public SecurityConfig(final CType type, final String hash, final Instant lastModified) { + public SecurityConfig(final CType type, final String hash, final Instant lastModified) { this.type = type; this.hash = hash; this.lastModified = lastModified; } public SecurityConfig(final StreamInput in) throws IOException { - this.type = in.readEnum(CType.class); + this.type = CType.fromOrd(in.readVInt()); this.hash = in.readString(); this.lastModified = in.readOptionalInstant(); } @@ -48,7 +48,7 @@ public Optional lastModified() { return Optional.ofNullable(lastModified); } - public CType type() { + public CType type() { return type; } @@ -58,7 +58,7 @@ public String hash() { @Override public void writeTo(final StreamOutput out) throws IOException { - out.writeEnum(type); + out.writeVInt(type.getOrd()); out.writeString(hash); out.writeOptionalInstant(lastModified); } @@ -89,7 +89,7 @@ public int hashCode() { public final static class Builder { - private final CType type; + private final CType type; private Instant lastModified; diff --git a/src/main/java/org/opensearch/security/support/ConfigHelper.java b/src/main/java/org/opensearch/security/support/ConfigHelper.java index e8526478f2..6495bdc8ec 100644 --- a/src/main/java/org/opensearch/security/support/ConfigHelper.java +++ b/src/main/java/org/opensearch/security/support/ConfigHelper.java @@ -62,7 +62,7 @@ public class ConfigHelper { private static final Logger LOGGER = LogManager.getLogger(ConfigHelper.class); - public static void uploadFile(Client tc, String filepath, String index, CType cType, int configVersion) throws Exception { + public static void uploadFile(Client tc, String filepath, String index, CType cType, int configVersion) throws Exception { uploadFile(tc, filepath, index, cType, configVersion, false); } @@ -70,7 +70,7 @@ public static void uploadFile( Client tc, String filepath, String index, - CType cType, + CType cType, int configVersion, boolean populateEmptyIfFileMissing ) throws Exception { @@ -110,7 +110,7 @@ public static void uploadFile( }); } - public static Reader createFileOrStringReader(CType cType, int configVersion, String filepath, boolean populateEmptyIfFileMissing) + public static Reader createFileOrStringReader(CType cType, int configVersion, String filepath, boolean populateEmptyIfFileMissing) throws Exception { Reader reader; if (!populateEmptyIfFileMissing || new File(filepath).exists()) { @@ -121,10 +121,9 @@ public static Reader createFileOrStringReader(CType cType, int configVersion, St return reader; } - public static SecurityDynamicConfiguration createEmptySdc(CType cType, int configVersion) throws Exception { - SecurityDynamicConfiguration empty = SecurityDynamicConfiguration.empty(); + public static SecurityDynamicConfiguration createEmptySdc(CType cType, int configVersion) throws Exception { + SecurityDynamicConfiguration empty = SecurityDynamicConfiguration.empty(cType); if (configVersion == 2) { - empty.setCType(cType); empty.set_meta(new Meta()); empty.get_meta().setConfig_version(configVersion); empty.get_meta().setType(cType.toLCString()); @@ -134,7 +133,7 @@ public static SecurityDynamicConfiguration createEmptySdc(CType cType, int co return c; } - public static String createEmptySdcYaml(CType cType, int configVersion) throws Exception { + public static String createEmptySdcYaml(CType cType, int configVersion) throws Exception { return DefaultObjectMapper.YAML_MAPPER.writeValueAsString(createEmptySdc(cType, configVersion)); } @@ -157,7 +156,7 @@ public static BytesReference readXContent(final Reader reader, final MediaType m public static SecurityDynamicConfiguration fromYamlReader( Reader yamlReader, - CType ctype, + CType ctype, int version, long seqNo, long primaryTerm @@ -177,14 +176,19 @@ public static SecurityDynamicConfiguration fromYamlReader( } } - public static SecurityDynamicConfiguration fromYamlFile(String filepath, CType ctype, int version, long seqNo, long primaryTerm) - throws IOException { + public static SecurityDynamicConfiguration fromYamlFile( + String filepath, + CType ctype, + int version, + long seqNo, + long primaryTerm + ) throws IOException { return fromYamlReader(new FileReader(filepath, StandardCharsets.UTF_8), ctype, version, seqNo, primaryTerm); } public static SecurityDynamicConfiguration fromYamlString( String yamlString, - CType ctype, + CType ctype, int version, long seqNo, long primaryTerm diff --git a/src/main/java/org/opensearch/security/support/SecurityIndexHandler.java b/src/main/java/org/opensearch/security/support/SecurityIndexHandler.java index 1ed8a99614..44529de857 100644 --- a/src/main/java/org/opensearch/security/support/SecurityIndexHandler.java +++ b/src/main/java/org/opensearch/security/support/SecurityIndexHandler.java @@ -22,7 +22,6 @@ import java.util.Set; import java.util.stream.Collectors; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.hash.Hashing; import org.apache.logging.log4j.LogManager; @@ -43,6 +42,7 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.configuration.ConfigurationMap; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.state.SecurityConfig; @@ -127,17 +127,14 @@ public void uploadDefaultConfiguration(final Path configDir, final ActionListene } } - public void loadConfiguration( - final Set configuration, - final ActionListener>> listener - ) { + public void loadConfiguration(final Set configuration, final ActionListener listener) { try (final ThreadContext.StoredContext threadContext = client.threadPool().getThreadContext().stashContext()) { client.threadPool().getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true"); - final var configurationTypes = configuration.stream().map(SecurityConfig::type).collect(Collectors.toUnmodifiableList()); + final List> configurationTypes = configuration.stream() + .map(SecurityConfig::type) + .collect(Collectors.toUnmodifiableList()); client.multiGet(newMultiGetRequest(configurationTypes), ActionListener.runBefore(ActionListener.wrap(r -> { - final var cTypeConfigsBuilder = ImmutableMap.>builderWithExpectedSize( - configuration.size() - ); + final var cTypeConfigsBuilder = new ConfigurationMap.Builder(); var hasFailures = false; for (final var item : r.getResponses()) { if (item.isFailed()) { @@ -161,15 +158,14 @@ public void loadConfiguration( hasFailures = true; break; } - cTypeConfigsBuilder.put(cType, config); + cTypeConfigsBuilder.with(config); } else { if (!cType.emptyIfMissing()) { listener.onFailure(new SecurityException("Missing required configuration for type: " + cType)); hasFailures = true; break; } - cTypeConfigsBuilder.put( - cType, + cTypeConfigsBuilder.with( SecurityDynamicConfiguration.fromJson( emptyJsonConfigFor(cType), cType, @@ -187,7 +183,7 @@ public void loadConfiguration( } } - private MultiGetRequest newMultiGetRequest(final List configurationTypes) { + private MultiGetRequest newMultiGetRequest(final List> configurationTypes) { final var request = new MultiGetRequest().realtime(true).refresh(true); for (final var cType : configurationTypes) { request.add(indexName, cType.toLCString()); @@ -196,7 +192,7 @@ private MultiGetRequest newMultiGetRequest(final List configurationTypes) } private SecurityDynamicConfiguration buildDynamicConfiguration( - final CType cType, + final CType cType, final BytesReference bytesRef, final long seqNo, final long primaryTerm diff --git a/src/main/java/org/opensearch/security/support/YamlConfigReader.java b/src/main/java/org/opensearch/security/support/YamlConfigReader.java index 237e5b5bfb..c8096f744c 100644 --- a/src/main/java/org/opensearch/security/support/YamlConfigReader.java +++ b/src/main/java/org/opensearch/security/support/YamlConfigReader.java @@ -40,7 +40,7 @@ public final class YamlConfigReader { private static final Logger LOGGER = LogManager.getLogger(YamlConfigReader.class); - public static BytesReference yamlContentFor(final CType cType, final Path configDir) throws IOException { + public static BytesReference yamlContentFor(final CType cType, final Path configDir) throws IOException { final var yamlXContent = XContentType.YAML.xContent(); try ( final var r = newReader(cType, configDir); @@ -56,7 +56,7 @@ public static BytesReference yamlContentFor(final CType cType, final Path config } } - public static Reader newReader(final CType cType, final Path configDir) throws IOException { + public static Reader newReader(final CType cType, final Path configDir) throws IOException { final var cTypeFile = cType.configFile(configDir); final var fileExists = Files.exists(cTypeFile); if (!fileExists && !cType.emptyIfMissing()) { @@ -71,24 +71,23 @@ public static Reader newReader(final CType cType, final Path configDir) throws I } } - private static SecurityDynamicConfiguration emptyConfigFor(final CType cType) { - final var emptyConfiguration = SecurityDynamicConfiguration.empty(); - emptyConfiguration.setCType(cType); + private static SecurityDynamicConfiguration emptyConfigFor(final CType cType) { + final var emptyConfiguration = SecurityDynamicConfiguration.empty(cType); emptyConfiguration.set_meta(new Meta()); emptyConfiguration.get_meta().setConfig_version(DEFAULT_CONFIG_VERSION); emptyConfiguration.get_meta().setType(cType.toLCString()); return emptyConfiguration; } - public static String emptyJsonConfigFor(final CType cType) throws IOException { + public static String emptyJsonConfigFor(final CType cType) throws IOException { return DefaultObjectMapper.writeValueAsString(emptyConfigFor(cType), false); } - public static String emptyYamlConfigFor(final CType cType) throws IOException { + public static String emptyYamlConfigFor(final CType cType) throws IOException { return DefaultObjectMapper.YAML_MAPPER.writeValueAsString(emptyConfigFor(cType)); } - private static void validateYamlContent(final CType cType, final InputStream in) throws IOException { + private static void validateYamlContent(final CType cType, final InputStream in) throws IOException { SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(in), cType, DEFAULT_CONFIG_VERSION, -1, -1); } diff --git a/src/main/java/org/opensearch/security/tools/Migrater.java b/src/main/java/org/opensearch/security/tools/Migrater.java index 96b79a5993..2dbc009706 100644 --- a/src/main/java/org/opensearch/security/tools/Migrater.java +++ b/src/main/java/org/opensearch/security/tools/Migrater.java @@ -29,13 +29,17 @@ import org.opensearch.common.collect.Tuple; import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.auditlog.config.AuditConfig; import org.opensearch.security.securityconf.Migration; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.NodesDn; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v6.ConfigV6; +import org.opensearch.security.securityconf.impl.v6.InternalUserV6; +import org.opensearch.security.securityconf.impl.v6.RoleMappingsV6; +import org.opensearch.security.securityconf.impl.v6.RoleV6; import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.securityconf.impl.v7.TenantV7; -import org.opensearch.security.support.ConfigHelper; public class Migrater { @@ -89,7 +93,7 @@ public static boolean migrateDirectory(File dir, boolean backup) { return retVal; } - public static boolean migrateFile(File file, CType cType, boolean backup) { + public static boolean migrateFile(File file, CType cType, boolean backup) { final String absolutePath = file.getAbsolutePath(); // NODESDN is newer type and supports populating empty content if file is missing if (!file.exists() && cType != CType.NODESDN) { @@ -118,15 +122,13 @@ public static boolean migrateFile(File file, CType cType, boolean backup) { } if (cType == CType.CONFIG) { - SecurityDynamicConfiguration val = Migration.migrateConfig( - SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(file), CType.CONFIG, 1, 0, 0) - ); + SecurityDynamicConfiguration val = Migration.migrateConfig(Migration.readYaml(file, ConfigV6.class)); return backupAndWrite(file, val, backup); } if (cType == CType.ROLES) { Tuple, SecurityDynamicConfiguration> tup = Migration.migrateRoles( - SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(file), CType.ROLES, 1, 0, 0), + Migration.readYaml(file, RoleV6.class), null ); boolean roles = backupAndWrite(file, tup.v1(), backup); @@ -134,38 +136,22 @@ public static boolean migrateFile(File file, CType cType, boolean backup) { } if (cType == CType.ROLESMAPPING) { - SecurityDynamicConfiguration val = Migration.migrateRoleMappings( - SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(file), CType.ROLESMAPPING, 1, 0, 0) - ); + SecurityDynamicConfiguration val = Migration.migrateRoleMappings(Migration.readYaml(file, RoleMappingsV6.class)); return backupAndWrite(file, val, backup); } if (cType == CType.INTERNALUSERS) { - SecurityDynamicConfiguration val = Migration.migrateInternalUsers( - SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(file), CType.INTERNALUSERS, 1, 0, 0) - ); + SecurityDynamicConfiguration val = Migration.migrateInternalUsers(Migration.readYaml(file, InternalUserV6.class)); return backupAndWrite(file, val, backup); } if (cType == CType.AUDIT) { - SecurityDynamicConfiguration val = Migration.migrateAudit( - SecurityDynamicConfiguration.fromNode(DefaultObjectMapper.YAML_MAPPER.readTree(file), CType.AUDIT, 1, 0, 0) - ); + SecurityDynamicConfiguration val = Migration.migrateAudit(Migration.readYaml(file, AuditConfig.class)); return backupAndWrite(file, val, backup); } if (cType == CType.NODESDN) { - SecurityDynamicConfiguration val = Migration.migrateNodesDn( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree( - ConfigHelper.createFileOrStringReader(CType.NODESDN, 1, file.getAbsolutePath(), true) - ), - CType.NODESDN, - 1, - 0, - 0 - ) - ); + SecurityDynamicConfiguration val = Migration.migrateNodesDn(Migration.readYaml(file, NodesDn.class)); return backupAndWrite(file, val, backup); } } catch (Exception e) { diff --git a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java index a8beb3c07b..af620dc574 100644 --- a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java +++ b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java @@ -126,7 +126,10 @@ import org.opensearch.security.securityconf.impl.NodesDn; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.WhitelistingSettings; +import org.opensearch.security.securityconf.impl.v6.ConfigV6; +import org.opensearch.security.securityconf.impl.v6.InternalUserV6; import org.opensearch.security.securityconf.impl.v6.RoleMappingsV6; +import org.opensearch.security.securityconf.impl.v6.RoleV6; import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; import org.opensearch.security.securityconf.impl.v7.ConfigV7; import org.opensearch.security.securityconf.impl.v7.InternalUserV7; @@ -1547,92 +1550,32 @@ private static int migrate(RestHighLevelClient tc, String index, File backupDir, ) ); SecurityDynamicConfiguration configV7 = Migration.migrateConfig( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree(new File(backupDir, "config.yml")), - CType.CONFIG, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "config.yml"), ConfigV6.class) ); SecurityDynamicConfiguration internalUsersV7 = Migration.migrateInternalUsers( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree(new File(backupDir, "internal_users.yml")), - CType.INTERNALUSERS, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "internal_users.yml"), InternalUserV6.class) ); - SecurityDynamicConfiguration rolesmappingV6 = SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree(new File(backupDir, "roles_mapping.yml")), - CType.ROLESMAPPING, - 1, - 0, - 0 + SecurityDynamicConfiguration rolesmappingV6 = Migration.readYaml( + new File(backupDir, "roles_mapping.yml"), + RoleMappingsV6.class ); + Tuple, SecurityDynamicConfiguration> rolesTenantsV7 = Migration.migrateRoles( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree(new File(backupDir, "roles.yml")), - CType.ROLES, - 1, - 0, - 0 - ), + Migration.readYaml(new File(backupDir, "roles.yml"), RoleV6.class), rolesmappingV6 ); SecurityDynamicConfiguration rolesmappingV7 = Migration.migrateRoleMappings(rolesmappingV6); SecurityDynamicConfiguration nodesDn = Migration.migrateNodesDn( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree( - ConfigHelper.createFileOrStringReader(CType.NODESDN, 1, new File(backupDir, "nodes_dn.yml").getAbsolutePath(), true) - ), - CType.NODESDN, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "nodes_dn.yml"), NodesDn.class) ); SecurityDynamicConfiguration whitelistingSettings = Migration.migrateWhitelistingSetting( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree( - ConfigHelper.createFileOrStringReader( - CType.WHITELIST, - 1, - new File(backupDir, "whitelist.yml").getAbsolutePath(), - true - ) - ), - CType.WHITELIST, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "whitelist.yml"), WhitelistingSettings.class) ); SecurityDynamicConfiguration allowlistingSettings = Migration.migrateAllowlistingSetting( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree( - ConfigHelper.createFileOrStringReader( - CType.ALLOWLIST, - 1, - new File(backupDir, "allowlist.yml").getAbsolutePath(), - true - ) - ), - CType.ALLOWLIST, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "allowlist.yml"), AllowlistingSettings.class) ); SecurityDynamicConfiguration audit = Migration.migrateAudit( - SecurityDynamicConfiguration.fromNode( - DefaultObjectMapper.YAML_MAPPER.readTree(new File(backupDir, "audit.yml")), - CType.AUDIT, - 1, - 0, - 0 - ) + Migration.readYaml(new File(backupDir, "audit.yml"), AuditConfig.class) ); DefaultObjectMapper.YAML_MAPPER.writeValue(new File(v7Dir, "/action_groups.yml"), actionGroupsV7); @@ -1719,7 +1662,7 @@ private static int validateConfig(String cd, String file, String type, int versi return -1; } - private static boolean validateConfigFile(String file, CType cType, int version) { + private static boolean validateConfigFile(String file, CType cType, int version) { try { ConfigHelper.fromYamlFile(file, cType, version == 7 ? 2 : 1, 0, 0); System.out.println(file + " OK"); diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java index 7a0172f748..2148c6caac 100644 --- a/src/main/java/org/opensearch/security/user/UserService.java +++ b/src/main/java/org/opensearch/security/user/UserService.java @@ -84,7 +84,7 @@ public class UserService { final static String FAILED_ACCOUNT_RETRIEVAL_MESSAGE = "The account specified could not be accessed at this time."; final static String AUTH_TOKEN_GENERATION_MESSAGE = "An auth token could not be generated for the specified account."; - private static CType getUserConfigName() { + private static CType getUserConfigName() { return CType.INTERNALUSERS; } @@ -115,7 +115,7 @@ public UserService( * @param config CType whose data is to be loaded in-memory * @return configuration loaded with given CType data */ - protected final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { + protected final SecurityDynamicConfiguration load(final CType config, boolean logComplianceEvent) { SecurityDynamicConfiguration loaded = configurationRepository.getConfigurationsFromIndex( Collections.singleton(config), logComplianceEvent @@ -311,7 +311,7 @@ public AuthToken generateAuthToken(String accountName) throws IOException { public static void saveAndUpdateConfigs( final String indexName, final Client client, - final CType cType, + final CType cType, final SecurityDynamicConfiguration configuration ) { final IndexRequest ir = new IndexRequest(indexName); diff --git a/src/test/java/org/opensearch/security/ConfigTests.java b/src/test/java/org/opensearch/security/ConfigTests.java index 7a4e626d1b..c30da8ed9f 100644 --- a/src/test/java/org/opensearch/security/ConfigTests.java +++ b/src/test/java/org/opensearch/security/ConfigTests.java @@ -54,7 +54,7 @@ public class ConfigTests { @Test public void testEmptyConfig() throws Exception { - Assert.assertNotSame(SecurityDynamicConfiguration.empty().deepClone(), SecurityDynamicConfiguration.empty()); + Assert.assertNotSame(SecurityDynamicConfiguration.empty(CType.ROLES).deepClone(), SecurityDynamicConfiguration.empty(CType.ROLES)); } @Test @@ -102,7 +102,7 @@ public void testParseSg67Config() throws Exception { } - private void check(String file, CType cType) throws Exception { + private void check(String file, CType cType) throws Exception { final String adjustedFilePath = SingleClusterTest.TEST_RESOURCE_RELATIVE_PATH + file; JsonNode jsonNode = YAML.readTree(Files.readString(new File(adjustedFilePath).toPath(), StandardCharsets.UTF_8)); int configVersion = 1; @@ -116,11 +116,17 @@ 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 + ); } - private SecurityDynamicConfiguration load(String file, CType cType) throws Exception { + private SecurityDynamicConfiguration load(String file, CType cType) throws Exception { final String adjustedFilePath = SingleClusterTest.TEST_RESOURCE_RELATIVE_PATH + file; JsonNode jsonNode = YAML.readTree(Files.readString(new File(adjustedFilePath).toPath(), StandardCharsets.UTF_8)); int configVersion = 1; @@ -129,6 +135,6 @@ private SecurityDynamicConfiguration load(String file, CType cType) throws Ex 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/UserServiceUnitTests.java b/src/test/java/org/opensearch/security/UserServiceUnitTests.java index 578273d291..5b18f7ec3f 100644 --- a/src/test/java/org/opensearch/security/UserServiceUnitTests.java +++ b/src/test/java/org/opensearch/security/UserServiceUnitTests.java @@ -100,7 +100,7 @@ public void testAnyUserTypeFilter() { assertThat(true, is(config.getCEntries().containsKey(internalAccountUsername))); } - private SecurityDynamicConfiguration readConfigFromYml(String file, CType cType) throws Exception { + private SecurityDynamicConfiguration readConfigFromYml(String file, CType cType) throws Exception { final ObjectMapper YAML = new ObjectMapper(new YAMLFactory()); final String TEST_RESOURCE_RELATIVE_PATH = "../../resources/test/"; diff --git a/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java b/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java index ac1f57c0f1..3bf6e36dac 100644 --- a/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java +++ b/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java @@ -14,7 +14,6 @@ import java.io.IOException; import java.nio.file.Path; import java.time.Instant; -import java.util.Map; import java.util.Set; import org.junit.Before; @@ -154,7 +153,7 @@ public void getConfiguration_withInvalidConfigurationShouldReturnNewEmptyConfigu ConfigurationRepository configRepository = createConfigurationRepository(Settings.EMPTY); SecurityDynamicConfiguration config = configRepository.getConfiguration(CType.CONFIG); - SecurityDynamicConfiguration emptyConfig = SecurityDynamicConfiguration.empty(); + SecurityDynamicConfiguration emptyConfig = SecurityDynamicConfiguration.empty(CType.CONFIG); assertThat(config, is(instanceOf(SecurityDynamicConfiguration.class))); assertThat(config.getCEntries().size(), is(equalTo(0))); @@ -256,8 +255,8 @@ public void testInitSecurityIndex_shouldUploadConfigIfIndexCreated() throws Exce public void testExecuteConfigurationInitialization_executeInitializationOnlyOnce() throws Exception { doAnswer(invocation -> { @SuppressWarnings("unchecked") - final var listener = (ActionListener>>) invocation.getArgument(1); - listener.onResponse(Map.of()); + final var listener = (ActionListener) invocation.getArgument(1); + listener.onResponse(ConfigurationMap.EMPTY); return null; }).when(securityIndexHandler).loadConfiguration(any(), any()); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java index b91374e725..ee18a57274 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java @@ -13,7 +13,6 @@ import java.io.IOException; import java.util.List; -import java.util.Map; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -32,6 +31,7 @@ import org.opensearch.security.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; @@ -40,7 +40,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doReturn; @RunWith(MockitoJUnitRunner.class) public abstract class AbstractApiActionValidationTest { @@ -62,7 +62,7 @@ public abstract class AbstractApiActionValidationTest { @Mock SecurityDynamicConfiguration configuration; - SecurityDynamicConfiguration rolesConfiguration; + SecurityDynamicConfiguration rolesConfiguration; ObjectMapper objectMapper = DefaultObjectMapper.objectMapper; @@ -100,9 +100,7 @@ void setupRolesConfiguration() throws IOException { config.set("regular_role", objectMapper.createObjectNode().set("cluster_permissions", objectMapper.createArrayNode().add("*"))); rolesConfiguration = SecurityDynamicConfiguration.fromJson(objectMapper.writeValueAsString(config), CType.ROLES, 2, 1, 1); - when(configurationRepository.getConfigurationsFromIndex(List.of(CType.ROLES), false)).thenReturn( - Map.of(CType.ROLES, rolesConfiguration) - ); + doReturn(rolesConfiguration).when(configurationRepository).getUnconvertedConfigurationFromIndex(CType.ROLES, false); } @Test @@ -110,7 +108,7 @@ public void allCrudActionsForDefaultValidatorAreForbidden() throws Exception { final var defaultPessimisticValidator = new AbstractApiAction(null, clusterService, threadPool, securityApiDependencies) { @Override - protected CType getConfigType() { + protected CType getConfigType() { return CType.CONFIG; } }.createEndpointValidator(); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiActionValidationTest.java index 8b2c933b94..9d5e3d595d 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiActionValidationTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; public class ActionGroupsApiActionValidationTest extends AbstractApiActionValidationTest { @@ -68,34 +69,36 @@ public void hasNoRightsToChangeImmutableEntityForRegularUser() throws Exception @Test public void onConfigChangeActionGroupHasSameNameAsRole() throws Exception { - when(configuration.getCType()).thenReturn(CType.ACTIONGROUPS); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ACTIONGROUPS).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); final var ag = objectMapper.createObjectNode() - .put("type", ActionGroupsApiAction.CLUSTER_TYPE) - .set("allowed_actions", objectMapper.createArrayNode().add("indices:*")); + .put("type", ActionGroupsApiAction.CLUSTER_TYPE) + .set("allowed_actions", objectMapper.createArrayNode().add("indices:*")); final var actionGroupsApiActionEndpointValidator = new ActionGroupsApiAction(clusterService, threadPool, securityApiDependencies) - .createEndpointValidator(); + .createEndpointValidator(); - final var result = actionGroupsApiActionEndpointValidator.onConfigChange(SecurityConfiguration.of(ag,"kibana_read_only", configuration)); + final var result = actionGroupsApiActionEndpointValidator.onConfigChange( + SecurityConfiguration.of(ag, "kibana_read_only", configuration) + ); assertFalse(result.isValid()); assertThat(result.status(), is(RestStatus.BAD_REQUEST)); - assertThat(xContentToJsonNode(result.errorMessage()).get("message").asText(), is("kibana_read_only is an existing role. A action group cannot be named with an existing role name.")); + assertThat( + xContentToJsonNode(result.errorMessage()).get("message").asText(), + is("kibana_read_only is an existing role. A action group cannot be named with an existing role name.") + ); } @Test public void onConfigChangeActionGroupHasSelfReference() throws Exception { - when(configuration.getCType()).thenReturn(CType.ACTIONGROUPS); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ACTIONGROUPS).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); final var ag = objectMapper.createObjectNode() - .put("type", ActionGroupsApiAction.INDEX_TYPE) - .set("allowed_actions", objectMapper.createArrayNode().add("ag")); + .put("type", ActionGroupsApiAction.INDEX_TYPE) + .set("allowed_actions", objectMapper.createArrayNode().add("ag")); final var actionGroupsApiActionEndpointValidator = new ActionGroupsApiAction(clusterService, threadPool, securityApiDependencies) - .createEndpointValidator(); + .createEndpointValidator(); - final var result = actionGroupsApiActionEndpointValidator - .onConfigChange(SecurityConfiguration.of(ag,"ag", configuration)); + final var result = actionGroupsApiActionEndpointValidator.onConfigChange(SecurityConfiguration.of(ag, "ag", configuration)); assertFalse(result.isValid()); assertThat(result.status(), is(RestStatus.BAD_REQUEST)); assertThat(xContentToJsonNode(result.errorMessage()).get("message").asText(), is("ag cannot be an allowed_action of itself")); @@ -103,34 +106,32 @@ public void onConfigChangeActionGroupHasSelfReference() throws Exception { @Test public void validateInvalidType() throws Exception { - when(configuration.getCType()).thenReturn(CType.ACTIONGROUPS); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ACTIONGROUPS).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); final var ag = objectMapper.createObjectNode() - .put("type", "some_type_we_know_nothing_about") - .set("allowed_actions", objectMapper.createArrayNode().add("ag")); + .put("type", "some_type_we_know_nothing_about") + .set("allowed_actions", objectMapper.createArrayNode().add("ag")); final var actionGroupsApiActionEndpointValidator = new ActionGroupsApiAction(clusterService, threadPool, securityApiDependencies) - .createEndpointValidator(); + .createEndpointValidator(); - final var result = actionGroupsApiActionEndpointValidator - .onConfigChange(SecurityConfiguration.of(ag,"ag", configuration)); + final var result = actionGroupsApiActionEndpointValidator.onConfigChange(SecurityConfiguration.of(ag, "ag", configuration)); assertFalse(result.isValid()); assertThat(result.status(), is(RestStatus.BAD_REQUEST)); - assertThat(xContentToJsonNode(result.errorMessage()).get("message").asText(), is("Invalid action group type: some_type_we_know_nothing_about. Supported types are: cluster, index.")); + assertThat( + xContentToJsonNode(result.errorMessage()).get("message").asText(), + is("Invalid action group type: some_type_we_know_nothing_about. Supported types are: cluster, index.") + ); } @Test public void passActionGroupWithoutType() throws Exception { - when(configuration.getCType()).thenReturn(CType.ACTIONGROUPS); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ACTIONGROUPS).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); - final var ag = objectMapper.createObjectNode() - .set("allowed_actions", objectMapper.createArrayNode().add("ag")); + final var ag = objectMapper.createObjectNode().set("allowed_actions", objectMapper.createArrayNode().add("ag")); final var actionGroupsApiActionEndpointValidator = new ActionGroupsApiAction(clusterService, threadPool, securityApiDependencies) - .createEndpointValidator(); + .createEndpointValidator(); - final var result = actionGroupsApiActionEndpointValidator - .onConfigChange(SecurityConfiguration.of(ag,"some_ag", configuration)); + final var result = actionGroupsApiActionEndpointValidator.onConfigChange(SecurityConfiguration.of(ag, "some_ag", configuration)); assertTrue(result.isValid()); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java index d6efaa9dd4..9127818d28 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java @@ -42,6 +42,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; public class InternalUsersApiActionValidationTest extends AbstractApiActionValidationTest { @@ -58,8 +59,7 @@ public void setupRolesAndMappings() throws IOException { final var allClusterPermissions = new RoleV7(); allClusterPermissions.setCluster_permissions(List.of("*")); - @SuppressWarnings("unchecked") - final var c = (SecurityDynamicConfiguration) rolesConfiguration; + final var c = rolesConfiguration; c.putCEntry("some_role_with_reserved_mapping", allClusterPermissions); c.putCEntry("some_role_with_hidden_mapping", allClusterPermissions); @@ -81,9 +81,7 @@ public void setupRolesAndMappings() throws IOException { 1, 1 ); - when(configurationRepository.getConfigurationsFromIndex(List.of(CType.ROLESMAPPING), false)).thenReturn( - Map.of(CType.ROLESMAPPING, rolesMappingConfiguration) - ); + doReturn(rolesMappingConfiguration).when(configurationRepository).getUnconvertedConfigurationFromIndex(CType.ROLESMAPPING, false); } @Test diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiActionValidationTest.java index 1caa39a4ff..7232f99b7e 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiActionValidationTest.java @@ -11,14 +11,10 @@ package org.opensearch.security.dlic.rest.api; -import java.util.List; -import java.util.Map; - import org.junit.Before; import org.junit.Test; import org.opensearch.core.rest.RestStatus; -import org.opensearch.security.securityconf.impl.CType; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -61,8 +57,6 @@ public void isNotAllowedNoRightsToChangeRoleEntity() throws Exception { @Test public void onConfigChangeShouldCheckRoles() throws Exception { when(restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(any(Object.class))).thenCallRealMethod(); - when(configurationRepository.getConfigurationsFromIndex(List.of(CType.ROLES), false)) - .thenReturn(Map.of(CType.ROLES, rolesConfiguration)); final var rolesApiActionEndpointValidator = new RolesMappingApiAction(clusterService, threadPool, securityApiDependencies).createEndpointValidator(); diff --git a/src/test/java/org/opensearch/security/dlic/rest/validation/EndpointValidatorTest.java b/src/test/java/org/opensearch/security/dlic/rest/validation/EndpointValidatorTest.java index 69bdb6a1a3..f3c10e3d9c 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/validation/EndpointValidatorTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/validation/EndpointValidatorTest.java @@ -38,6 +38,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -351,8 +352,7 @@ public void regularUserCanNotChangeObjectWithRestAdminPermissionsForNewRoles() t final var array = objectMapper.createArrayNode(); restAdminPermissions().forEach(array::add); - when(configuration.getCType()).thenReturn(CType.ROLES); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ROLES).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); when(configuration.exists("some_role")).thenReturn(false); when(restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(any(Object.class))).thenCallRealMethod(); @@ -378,8 +378,7 @@ public void regularUserCanNotChangeObjectWithRestAdminPermissionsForExitingActio @Test public void regularUserCanNotChangeObjectWithRestAdminPermissionsForMewActionGroups() throws Exception { - when(configuration.getCType()).thenReturn(CType.ACTIONGROUPS); - when(configuration.getVersion()).thenReturn(2); + doReturn(CType.ACTIONGROUPS).when(configuration).getCType(); when(configuration.getImplementingClass()).thenCallRealMethod(); when(configuration.exists("some_ag")).thenReturn(false); when(restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(any(Object.class))).thenCallRealMethod(); @@ -389,7 +388,7 @@ public void regularUserCanNotChangeObjectWithRestAdminPermissionsForMewActionGro restAdminPermissions().forEach(array::add); var agCheckResult = endpointValidator.isAllowedToChangeEntityWithRestAdminPermissions( - SecurityConfiguration.of(objectMapper.createObjectNode().set("allowed_actions", array), "some_ag", configuration) + SecurityConfiguration.of(objectMapper.createObjectNode().set("allowed_actions", array), "some_ag", configuration) ); assertFalse(agCheckResult.isValid()); assertThat(agCheckResult.status(), is(RestStatus.FORBIDDEN)); diff --git a/src/test/java/org/opensearch/security/securityconf/FlattenedActionGroupsTest.java b/src/test/java/org/opensearch/security/securityconf/FlattenedActionGroupsTest.java index 7cea117d25..cc0d635686 100644 --- a/src/test/java/org/opensearch/security/securityconf/FlattenedActionGroupsTest.java +++ b/src/test/java/org/opensearch/security/securityconf/FlattenedActionGroupsTest.java @@ -38,8 +38,7 @@ public void basicTest() throws Exception { ); SecurityDynamicConfiguration config = SecurityDynamicConfiguration.fromMap( testActionGroups.map, - CType.ACTIONGROUPS, - 2 + CType.ACTIONGROUPS ); FlattenedActionGroups actionGroups = new FlattenedActionGroups(config); @@ -66,8 +65,7 @@ public void cycleTest() throws Exception { SecurityDynamicConfiguration config = SecurityDynamicConfiguration.fromMap( testActionGroups.map, - CType.ACTIONGROUPS, - 2 + CType.ACTIONGROUPS ); FlattenedActionGroups actionGroups = new FlattenedActionGroups(config); diff --git a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java index f469d1989c..f21a3e98a2 100644 --- a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java +++ b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java @@ -48,6 +48,10 @@ import org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.PermissionBuilder; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; +import org.opensearch.security.securityconf.impl.v7.RoleMappingsV7; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.securityconf.impl.v7.TenantV7; import org.mockito.Mockito; @@ -243,7 +247,7 @@ static ObjectNode meta(final String type) { return DefaultObjectMapper.objectMapper.createObjectNode().put("type", type).put("config_version", 2); } - static SecurityDynamicConfiguration createRolesConfig() throws IOException { + static SecurityDynamicConfiguration createRolesConfig() throws IOException { final ObjectNode rolesNode = DefaultObjectMapper.objectMapper.createObjectNode(); rolesNode.set("_meta", meta("roles")); NO_REST_ADMIN_PERMISSIONS_ROLES.forEach(rolesNode::set); @@ -252,19 +256,19 @@ static SecurityDynamicConfiguration createRolesConfig() throws IOExceptio return SecurityDynamicConfiguration.fromNode(rolesNode, CType.ROLES, 2, 0, 0); } - static SecurityDynamicConfiguration createRoleMappingsConfig() throws IOException { + static SecurityDynamicConfiguration createRoleMappingsConfig() throws IOException { final ObjectNode metaNode = DefaultObjectMapper.objectMapper.createObjectNode(); metaNode.set("_meta", meta("rolesmapping")); return SecurityDynamicConfiguration.fromNode(metaNode, CType.ROLESMAPPING, 2, 0, 0); } - static SecurityDynamicConfiguration createActionGroupsConfig() throws IOException { + static SecurityDynamicConfiguration createActionGroupsConfig() throws IOException { final ObjectNode metaNode = DefaultObjectMapper.objectMapper.createObjectNode(); metaNode.set("_meta", meta("actiongroups")); return SecurityDynamicConfiguration.fromNode(metaNode, CType.ACTIONGROUPS, 2, 0, 0); } - static SecurityDynamicConfiguration createTenantsConfig() throws IOException { + static SecurityDynamicConfiguration createTenantsConfig() throws IOException { final ObjectNode metaNode = DefaultObjectMapper.objectMapper.createObjectNode(); metaNode.set("_meta", meta("tenants")); return SecurityDynamicConfiguration.fromNode(metaNode, CType.TENANTS, 2, 0, 0); diff --git a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java deleted file mode 100644 index 530db3211b..0000000000 --- a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java +++ /dev/null @@ -1,193 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.securityconf; - -import java.io.IOException; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.TreeMap; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import org.junit.Assert; -import org.junit.Test; - -import org.opensearch.action.support.IndicesOptions; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.IndexAbstraction; -import org.opensearch.cluster.metadata.IndexNameExpressionResolver; -import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.Settings; -import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.resolver.IndexResolverReplacer; -import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; -import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.user.User; - -import org.mockito.quality.Strictness; - -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.withSettings; - -public class SecurityRolesPermissionsV6Test { - static final String TEST_INDEX = ".test"; - - // a role with * permission but no system:admin/system_index permission - static final Map NO_EXPLICIT_SYSTEM_INDEX_PERMISSION = ImmutableMap.builder() - .put("all_access_without_system_index_permission", role(new String[] { "*" }, new String[] { TEST_INDEX }, new String[] { "*" })) - .build(); - - static final Map HAS_SYSTEM_INDEX_PERMISSION = ImmutableMap.builder() - .put( - "has_system_index_permission", - role(new String[] { "*" }, new String[] { TEST_INDEX }, new String[] { ConfigConstants.SYSTEM_INDEX_PERMISSION }) - ) - .build(); - - static ObjectNode role(final String[] clusterPermissions, final String[] indexPatterns, final String[] allowedActions) { - ObjectMapper objectMapper = DefaultObjectMapper.objectMapper; - // form cluster permissions - final ArrayNode clusterPermissionsArrayNode = objectMapper.createArrayNode(); - Arrays.stream(clusterPermissions).forEach(clusterPermissionsArrayNode::add); - - // form index_permissions - ArrayNode permissions = objectMapper.createArrayNode(); - Arrays.stream(allowedActions).forEach(permissions::add); // permission in v6 format - - ObjectNode permissionNode = objectMapper.createObjectNode(); - permissionNode.set("*", permissions); // type : "*" - - ObjectNode indexPermission = objectMapper.createObjectNode(); - indexPermission.set("*", permissionNode); // '*' -> all indices - - // add both to the role - ObjectNode role = objectMapper.createObjectNode(); - role.put("readonly", true); - role.set("cluster", clusterPermissionsArrayNode); - role.set("indices", indexPermission); - - return role; - } - - final ConfigModel configModel; - - public SecurityRolesPermissionsV6Test() throws IOException { - this.configModel = new ConfigModelV6( - createRolesConfig(), - createRoleMappingsConfig(), - createActionGroupsConfig(), - mock(DynamicConfigModel.class), - Settings.EMPTY - ); - } - - @Test - public void hasExplicitIndexPermission() { - IndexNameExpressionResolver resolver = mock(IndexNameExpressionResolver.class); - User user = new User("test"); - ClusterService cs = mock(ClusterService.class); - doReturn(createClusterState(new IndexShorthand(TEST_INDEX, IndexAbstraction.Type.ALIAS))).when(cs).state(); - IndexResolverReplacer.Resolved resolved = createResolved(TEST_INDEX); - - // test hasExplicitIndexPermission - final SecurityRoles securityRoleWithStarAccess = configModel.getSecurityRoles() - .filter(ImmutableSet.of("all_access_without_system_index_permission")); - user.addSecurityRoles(List.of("all_access_without_system_index_permission")); - - Assert.assertFalse( - "Should not allow system index access with * only", - securityRoleWithStarAccess.hasExplicitIndexPermission(resolved, user, new String[] {}, resolver, cs) - ); - - final SecurityRoles securityRoleWithExplicitAccess = configModel.getSecurityRoles() - .filter(ImmutableSet.of("has_system_index_permission")); - user.addSecurityRoles(List.of("has_system_index_permission")); - - Assert.assertTrue( - "Should allow system index access with explicit only", - securityRoleWithExplicitAccess.hasExplicitIndexPermission(resolved, user, new String[] {}, resolver, cs) - ); - } - - @Test - public void isPermittedOnSystemIndex() { - final SecurityRoles securityRoleWithExplicitAccess = configModel.getSecurityRoles() - .filter(ImmutableSet.of("has_system_index_permission")); - Assert.assertTrue(securityRoleWithExplicitAccess.isPermittedOnSystemIndex(TEST_INDEX)); - - final SecurityRoles securityRoleWithStarAccess = configModel.getSecurityRoles() - .filter(ImmutableSet.of("all_access_without_system_index_permission")); - Assert.assertFalse(securityRoleWithStarAccess.isPermittedOnSystemIndex(TEST_INDEX)); - } - - static SecurityDynamicConfiguration createRolesConfig() throws IOException { - final ObjectNode rolesNode = DefaultObjectMapper.objectMapper.createObjectNode(); - NO_EXPLICIT_SYSTEM_INDEX_PERMISSION.forEach(rolesNode::set); - HAS_SYSTEM_INDEX_PERMISSION.forEach(rolesNode::set); - return SecurityDynamicConfiguration.fromNode(rolesNode, CType.ROLES, 1, 0, 0); - } - - static SecurityDynamicConfiguration createRoleMappingsConfig() throws IOException { - final ObjectNode metaNode = DefaultObjectMapper.objectMapper.createObjectNode(); - return SecurityDynamicConfiguration.fromNode(metaNode, CType.ROLESMAPPING, 1, 0, 0); - } - - static SecurityDynamicConfiguration createActionGroupsConfig() throws IOException { - final ObjectNode metaNode = DefaultObjectMapper.objectMapper.createObjectNode(); - return SecurityDynamicConfiguration.fromNode(metaNode, CType.ACTIONGROUPS, 1, 0, 0); - } - - private IndexResolverReplacer.Resolved createResolved(final String... indexes) { - return new IndexResolverReplacer.Resolved( - ImmutableSet.of(), - ImmutableSet.copyOf(indexes), - ImmutableSet.copyOf(indexes), - ImmutableSet.of(), - IndicesOptions.STRICT_EXPAND_OPEN - ); - } - - private ClusterState createClusterState(final IndexShorthand... indices) { - final TreeMap indexMap = new TreeMap(); - Arrays.stream(indices).forEach(indexShorthand -> { - final IndexAbstraction indexAbstraction = mock(IndexAbstraction.class); - when(indexAbstraction.getType()).thenReturn(indexShorthand.type); - indexMap.put(indexShorthand.name, indexAbstraction); - }); - - final Metadata mockMetadata = mock(Metadata.class, withSettings().strictness(Strictness.LENIENT)); - when(mockMetadata.getIndicesLookup()).thenReturn(indexMap); - - final ClusterState mockClusterState = mock(ClusterState.class, withSettings().strictness(Strictness.LENIENT)); - when(mockClusterState.getMetadata()).thenReturn(mockMetadata); - - return mockClusterState; - } - - private class IndexShorthand { - public final String name; - public final IndexAbstraction.Type type; - - public IndexShorthand(final String name, final IndexAbstraction.Type type) { - this.name = name; - this.type = type; - } - } -} diff --git a/src/test/java/org/opensearch/security/support/ConfigReaderTest.java b/src/test/java/org/opensearch/security/support/ConfigReaderTest.java index b75d5a6e35..dfdfa76e75 100644 --- a/src/test/java/org/opensearch/security/support/ConfigReaderTest.java +++ b/src/test/java/org/opensearch/security/support/ConfigReaderTest.java @@ -41,7 +41,7 @@ public static void createConfigFile() throws IOException { @Test public void testThrowsIOExceptionForMandatoryCTypes() { - for (final var cType : CType.REQUIRED_CONFIG_FILES) { + for (final var cType : CType.requiredConfigTypes()) { assertThrows(IOException.class, () -> YamlConfigReader.newReader(cType, configDir.toPath())); } } @@ -49,7 +49,7 @@ public void testThrowsIOExceptionForMandatoryCTypes() { @Test public void testCreateReaderForNonMandatoryCTypes() throws IOException { final var yamlMapper = DefaultObjectMapper.YAML_MAPPER; - for (final var cType : CType.NOT_REQUIRED_CONFIG_FILES) { + for (final var cType : CType.notRequiredConfigTypes()) { try (final var reader = new BufferedReader(YamlConfigReader.newReader(cType, configDir.toPath()))) { final var emptyYaml = yamlMapper.readTree(reader); assertTrue(emptyYaml.has("_meta")); diff --git a/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java b/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java index e121218af4..f1d62fc5f4 100644 --- a/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/support/SecurityIndexHandlerTest.java @@ -47,8 +47,8 @@ import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.index.get.GetResult; import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.configuration.ConfigurationMap; import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.state.SecurityConfig; import org.opensearch.threadpool.ThreadPool; @@ -104,7 +104,7 @@ public class SecurityIndexHandlerTest { + "all_access: \n" + " reserved: false\n"; - static final Map> YAML = Map.of( + static final Map, CheckedSupplier> YAML = Map.of( CType.ACTIONGROUPS, () -> emptyYamlConfigFor(CType.ACTIONGROUPS), CType.ALLOWLIST, @@ -226,7 +226,7 @@ public void testUploadDefaultConfiguration_shouldFailIfBulkHasFailures() throws actionListener.onResponse(failedBulkResponse); return null; }).when(client).bulk(any(BulkRequest.class), any()); - for (final var c : CType.REQUIRED_CONFIG_FILES) { + for (final var c : CType.requiredConfigTypes()) { try (final var io = Files.newBufferedWriter(c.configFile(configFolder))) { io.write(YAML.get(c).get()); io.flush(); @@ -246,7 +246,7 @@ public void testUploadDefaultConfiguration_shouldCreateSetOfSecurityConfigs() th } }, e -> fail("Unexpected behave"))); - for (final var c : CType.REQUIRED_CONFIG_FILES) { + for (final var c : CType.requiredConfigTypes()) { try (final var io = Files.newBufferedWriter(c.configFile(configFolder))) { final var source = YAML.get(c).get(); io.write(source); @@ -283,7 +283,7 @@ public void testUploadDefaultConfiguration_shouldSkipAudit() throws IOException ) ); - for (final var c : CType.REQUIRED_CONFIG_FILES) { + for (final var c : CType.requiredConfigTypes()) { if (c == CType.AUDIT) continue; try (final var io = Files.newBufferedWriter(c.configFile(configFolder))) { final var source = YAML.get(c).get(); @@ -312,7 +312,7 @@ public void testUploadDefaultConfiguration_shouldSkipWhitelist() throws IOExcept ) ); - for (final var c : CType.REQUIRED_CONFIG_FILES) { + for (final var c : CType.requiredConfigTypes()) { if (c == CType.WHITELIST) continue; try (final var io = Files.newBufferedWriter(c.configFile(configFolder))) { final var source = YAML.get(c).get(); @@ -335,7 +335,7 @@ public void testUploadDefaultConfiguration_shouldSkipWhitelist() throws IOExcept @Test public void testLoadConfiguration_shouldFailIfResponseHasFailures() { final var listener = spy( - ActionListener.>>wrap( + ActionListener.wrap( r -> fail("Unexpected behave"), e -> assertThat(e.getClass(), is(SecurityException.class)) ) @@ -359,7 +359,7 @@ public void testLoadConfiguration_shouldFailIfResponseHasFailures() { @Test public void testLoadConfiguration_shouldFailIfNoRequiredConfigInResponse() { final var listener = spy( - ActionListener.>>wrap( + ActionListener.wrap( r -> fail("Unexpected behave"), e -> assertThat(e.getMessage(), is("Missing required configuration for type: CONFIG")) ) @@ -379,43 +379,10 @@ 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( - ActionListener.>>wrap( + ActionListener.wrap( r -> fail("Unexpected behave"), e -> assertThat(e.getMessage(), is("Couldn't parse content for CONFIG")) ) @@ -450,8 +417,8 @@ public void testLoadConfiguration_shouldFailForUnparseableConfig() { @Test public void testLoadConfiguration_shouldBuildSecurityConfig() { - final var listener = spy(ActionListener.>>wrap(config -> { - assertThat(config.keySet().size(), is(CType.values().length)); + final var listener = spy(ActionListener.wrap(config -> { + assertThat(config.keySet().size(), is(CType.values().size())); for (final var c : CType.values()) { assertTrue(c.toLCString(), config.containsKey(c)); } @@ -460,7 +427,7 @@ public void testLoadConfiguration_shouldBuildSecurityConfig() { final var objectMapper = DefaultObjectMapper.objectMapper; ActionListener actionListener = invocation.getArgument(1); - final var responses = new MultiGetItemResponse[CType.values().length]; + final var responses = new MultiGetItemResponse[CType.values().size()]; var counter = 0; for (final var c : CType.values()) { final var getResult = mock(GetResult.class); @@ -497,7 +464,7 @@ public void testLoadConfiguration_shouldBuildSecurityConfig() { verify(listener).onResponse(any()); } - private ObjectNode minimumRequiredConfig(final CType cType) { + private ObjectNode minimumRequiredConfig(final CType cType) { final var objectMapper = DefaultObjectMapper.objectMapper; return objectMapper.createObjectNode() .set("_meta", objectMapper.createObjectNode().put("type", cType.toLCString()).put("config_version", DEFAULT_CONFIG_VERSION));