diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java index 35fd179019..4311cd268c 100644 --- a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java +++ b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java @@ -10,70 +10,98 @@ package org.opensearch.security.legacy; 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; +import java.util.Map; + +@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: - dynamic: - authc: - basic_internal_auth_domain: - http_enabled: true - order: 4 - http_authenticator: - type: basic - challenge: true - authentication_backend: - type: intern - """)// - .rawConfigurationDocumentYaml("internalusers", """ - admin: - readonly: true - hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG - roles: - - admin - attributes: - attribute1: value1 - """)// - .rawConfigurationDocumentYaml("roles", """ - all_access: - readonly: true - cluster: - - UNLIMITED - indices: - '*': - '*': - - UNLIMITED - tenants: - admin_tenant: RW - """)// - .rawConfigurationDocumentYaml("rolesmapping", """ - all_access: - readonly: true - backendroles: - - admin - """); + .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 checkAuthc() { + public void authc() { try (TestRestClient client = cluster.getRestClient("admin", "admin")) { TestRestClient.HttpResponse response = client.get("_opendistro/_security/authinfo"); - System.out.println(response.getBody()); + 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/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java b/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java index 5aa6e389d3..da193f1212 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java @@ -59,6 +59,7 @@ 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; @@ -122,17 +123,6 @@ public void success(SecurityDynamicConfiguration dConf) { result.with(dConf); - if (dConf.getCType() == CType.ROLES && dConf.getAutoConvertedFrom() != null) { - // 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) dConf.getAutoConvertedFrom(); - SecurityDynamicConfiguration tenants = Migration.createTenants(roleV6config); - result.with(tenants); - } - latch.countDown(); if (isDebugEnabled) { log.debug( @@ -158,28 +148,11 @@ public void singleFailure(Failure failure) { 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; - } - } - // 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, @@ -235,6 +208,20 @@ public void failure(Throwable t) { ); } + 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(); } diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java b/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java index bd023fc4ef..ef349e20fc 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationMap.java @@ -11,6 +11,8 @@ package org.opensearch.security.configuration; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import com.google.common.collect.ImmutableMap; @@ -72,7 +74,7 @@ public static ConfigurationMap of(SecurityDynamicConfiguration... configs) { } public static class Builder { - private ImmutableMap.Builder, SecurityDynamicConfiguration> map = new ImmutableMap.Builder<>(); + private Map, SecurityDynamicConfiguration> map = new HashMap<>(); public Builder() {} @@ -86,8 +88,23 @@ public Builder with(ConfigurationMap configurationMap) { 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(this.map.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 a53f87ac3f..f8fe70ca1a 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -635,6 +635,16 @@ public ConfigurationMap getConfigurationsFromIndex( 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 ConfigurationMap validate(ConfigurationMap conf, int expectedSize) throws InvalidConfigException { if (conf == null || conf.size() < expectedSize) { 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 160a7f708d..903c601143 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 @@ -455,16 +455,14 @@ public RequestContentValidator createRequestContentValidator(Object... params) { 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) { SecurityDynamicConfiguration loaded = securityApiDependencies.configurationRepository() - .getConfigurationsFromIndex(List.of(config), logComplianceEvent) - .get(config) + .getUnconvertedConfigurationFromIndex(config, logComplianceEvent) .deepCloneWithRedaction(); return DynamicConfigFactory.addStatics(loaded); } diff --git a/src/main/java/org/opensearch/security/securityconf/Migration.java b/src/main/java/org/opensearch/security/securityconf/Migration.java index 57df2c2ac1..8153360270 100644 --- a/src/main/java/org/opensearch/security/securityconf/Migration.java +++ b/src/main/java/org/opensearch/security/securityconf/Migration.java @@ -116,7 +116,7 @@ public static SecurityDynamicConfiguration createTenants(SecurityDynam } } - SecurityDynamicConfiguration tenantConfig = SecurityDynamicConfiguration.empty(CType.TENANTS); + SecurityDynamicConfiguration tenantConfig = SecurityDynamicConfiguration.empty(CType.TENANTS, 2); tenantConfig.set_meta(new Meta()); tenantConfig.get_meta().setConfig_version(2); tenantConfig.get_meta().setType("tenants"); 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 292104e6be..b987e265d1 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/CType.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/CType.java @@ -297,6 +297,7 @@ public SecurityDynamicConfiguration parseJson(CType ctype, Str public SecurityDynamicConfiguration convert(SecurityDynamicConfiguration oldConfig, CType ctype) { SecurityDynamicConfiguration newConfig = SecurityDynamicConfiguration.empty(ctype); newConfig.setAutoConvertedFrom(oldConfig); +// newConfig.setSeqNoPrimaryTerm(); for (Map.Entry oldEntry : oldConfig.getCEntries().entrySet()) { newConfig.putCEntry(mapKeyConversionFunction.apply(oldEntry.getKey()), entryConversionFunction.apply(oldEntry.getValue())); 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 bc03d59500..c925bdac7a 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -63,7 +63,15 @@ public class SecurityDynamicConfiguration implements ToXContent { private final Object modificationLock = new Object(); private long seqNo = -1; private long primaryTerm = -1; + /** + * 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; private SecurityDynamicConfiguration autoConvertedFrom; @@ -72,6 +80,13 @@ 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 public boolean notEmpty() { return !centries.isEmpty(); @@ -105,6 +120,9 @@ public static SecurityDynamicConfiguration fromJson( 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( @@ -120,13 +138,34 @@ 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. @@ -213,6 +252,7 @@ private SecurityDynamicConfiguration() { private SecurityDynamicConfiguration(CType ctype) { super(); this.ctype = ctype; + this.ctypeUnsafe = ctype; } private Meta _meta; @@ -345,6 +385,12 @@ public long getPrimaryTerm() { return primaryTerm; } + @JsonIgnore + public void setSeqNoPrimaryTerm(long seqNo, long primaryTerm) { + this.seqNo = seqNo; + this.primaryTerm = primaryTerm; + } + @JsonIgnore public CType getCType() { return ctype; @@ -374,7 +420,15 @@ public Class getImplementingClass() { @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); } @@ -384,7 +438,15 @@ public SecurityDynamicConfiguration deepClone() { @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); }