diff --git a/docs/changelog/115823.yaml b/docs/changelog/115823.yaml new file mode 100644 index 0000000000000..a6119e0fa56e4 --- /dev/null +++ b/docs/changelog/115823.yaml @@ -0,0 +1,5 @@ +pr: 115823 +summary: Add ECK Role Mapping Cleanup +area: Security +type: bug +issues: [] diff --git a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FileSettingsRoleMappingUpgradeIT.java b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FileSettingsRoleMappingUpgradeIT.java index 834d97f755dfb..4caf33feeeebb 100644 --- a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FileSettingsRoleMappingUpgradeIT.java +++ b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FileSettingsRoleMappingUpgradeIT.java @@ -23,19 +23,20 @@ import org.junit.rules.TemporaryFolder; import org.junit.rules.TestRule; -import java.io.IOException; import java.util.List; +import java.util.Map; import java.util.function.Supplier; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; public class FileSettingsRoleMappingUpgradeIT extends ParameterizedRollingUpgradeTestCase { - private static final String settingsJSON = """ + private static final int ROLE_MAPPINGS_CLEANUP_MIGRATION_VERSION = 2; + private static final String SETTING_JSON = """ { "metadata": { "version": "1", @@ -53,7 +54,6 @@ public class FileSettingsRoleMappingUpgradeIT extends ParameterizedRollingUpgrad }"""; private static final TemporaryFolder repoDirectory = new TemporaryFolder(); - private static final ElasticsearchCluster cluster = ElasticsearchCluster.local() .distribution(DistributionType.DEFAULT) .version(getOldClusterTestVersion()) @@ -68,7 +68,7 @@ public String get() { .setting("xpack.security.enabled", "true") // workaround to avoid having to set up clients and authorization headers .setting("xpack.security.authc.anonymous.roles", "superuser") - .configFile("operator/settings.json", Resource.fromString(settingsJSON)) + .configFile("operator/settings.json", Resource.fromString(SETTING_JSON)) .build(); @ClassRule @@ -91,7 +91,30 @@ public void checkVersions() { ); } - public void testRoleMappingsAppliedOnUpgrade() throws IOException { + private static void waitForSecurityMigrationCompletionIfIndexExists() throws Exception { + final Request request = new Request("GET", "_cluster/state/metadata/.security-7"); + assertBusy(() -> { + Map indices = new XContentTestUtils.JsonMapView(entityAsMap(client().performRequest(request))).get( + "metadata.indices" + ); + assertNotNull(indices); + // If the security index exists, migration needs to happen. There is a bug in pre cluster state role mappings code that tries + // to write file based role mappings before security index manager state is recovered, this makes it look like the security + // index is outdated (isIndexUpToDate == false). Because we can't rely on the index being there for old versions, this check + // is needed. + if (indices.containsKey(".security-7")) { + // JsonMapView doesn't support . prefixed indices (splits on .) + @SuppressWarnings("unchecked") + String responseVersion = new XContentTestUtils.JsonMapView((Map) indices.get(".security-7")).get( + "migration_version.version" + ); + assertNotNull(responseVersion); + assertTrue(Integer.parseInt(responseVersion) >= ROLE_MAPPINGS_CLEANUP_MIGRATION_VERSION); + } + }); + } + + public void testRoleMappingsAppliedOnUpgrade() throws Exception { if (isOldCluster()) { Request clusterStateRequest = new Request("GET", "/_cluster/state/metadata"); List roleMappings = new XContentTestUtils.JsonMapView(entityAsMap(client().performRequest(clusterStateRequest))).get( @@ -107,11 +130,10 @@ public void testRoleMappingsAppliedOnUpgrade() throws IOException { ).get("metadata.role_mappings.role_mappings"); assertThat(clusterStateRoleMappings, is(not(nullValue()))); assertThat(clusterStateRoleMappings.size(), equalTo(1)); - + waitForSecurityMigrationCompletionIfIndexExists(); assertThat( entityAsMap(client().performRequest(new Request("GET", "/_security/role_mapping"))).keySet(), - // TODO change this to `contains` once the clean-up migration work is merged - hasItem("everyone_kibana-read-only-operator-mapping") + contains("everyone_kibana-read-only-operator-mapping") ); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java index 2390c96664057..a0b35f7cfc3eb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java @@ -91,6 +91,21 @@ public Set conflicts(String handlerName, Set modified) { return Collections.unmodifiableSet(intersect); } + /** + * Get the reserved keys for the handler name + * + * @param handlerName handler name to get keys for + * @return set of keys for that handler + */ + public Set keys(String handlerName) { + ReservedStateHandlerMetadata handlerMetadata = handlers.get(handlerName); + if (handlerMetadata == null || handlerMetadata.keys().isEmpty()) { + return Collections.emptySet(); + } + + return Collections.unmodifiableSet(handlerMetadata.keys()); + } + /** * Reads an {@link ReservedStateMetadata} from a {@link StreamInput} * diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersions.java b/server/src/main/java/org/elasticsearch/index/IndexVersions.java index efb1facc79b3a..2919f98ee200e 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexVersions.java +++ b/server/src/main/java/org/elasticsearch/index/IndexVersions.java @@ -128,7 +128,7 @@ private static Version parseUnchecked(String version) { public static final IndexVersion MERGE_ON_RECOVERY_VERSION = def(8_515_00_0, Version.LUCENE_9_11_1); public static final IndexVersion UPGRADE_TO_LUCENE_9_12 = def(8_516_00_0, Version.LUCENE_9_12_0); public static final IndexVersion ENABLE_IGNORE_ABOVE_LOGSDB = def(8_517_00_0, Version.LUCENE_9_12_0); - + public static final IndexVersion ADD_ROLE_MAPPING_CLEANUP_MIGRATION = def(8_518_00_0, Version.LUCENE_9_12_0); public static final IndexVersion UPGRADE_TO_LUCENE_10_0_0 = def(9_000_00_0, Version.LUCENE_10_0_0); /* diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/ExpressionRoleMapping.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/ExpressionRoleMapping.java index c504ebe56ed45..41fd3c6938dfc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/ExpressionRoleMapping.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/ExpressionRoleMapping.java @@ -206,7 +206,7 @@ public RoleMapperExpression getExpression() { * that match the {@link #getExpression() expression} in this mapping. */ public List getRoles() { - return Collections.unmodifiableList(roles); + return roles != null ? Collections.unmodifiableList(roles) : Collections.emptyList(); } /** @@ -214,7 +214,7 @@ public List getRoles() { * that should be assigned to users that match the {@link #getExpression() expression} in this mapping. */ public List getRoleTemplates() { - return Collections.unmodifiableList(roleTemplates); + return roleTemplates != null ? Collections.unmodifiableList(roleTemplates) : Collections.emptyList(); } /** @@ -223,7 +223,7 @@ public List getRoleTemplates() { * This is not used within the mapping process, and does not affect whether the expression matches, nor which roles are assigned. */ public Map getMetadata() { - return Collections.unmodifiableMap(metadata); + return metadata != null ? Collections.unmodifiableMap(metadata) : Collections.emptyMap(); } /** @@ -233,6 +233,15 @@ public boolean isEnabled() { return enabled; } + /** + * Whether this mapping is an operator defined/read only role mapping + */ + public boolean isReadOnly() { + return metadata != null && metadata.get(ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG) instanceof Boolean readOnly + ? readOnly + : false; + } + @Override public String toString() { return getClass().getSimpleName() + "<" + name + " ; " + roles + "/" + roleTemplates + " = " + Strings.toString(expression) + ">"; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java index 74c6223b1ebdd..31fe86ca77edd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java @@ -191,6 +191,14 @@ public static boolean hasFallbackName(ExpressionRoleMapping expressionRoleMappin return expressionRoleMapping.getName().equals(FALLBACK_NAME); } + /** + * Check if any of the role mappings have a fallback name + * @return true if any role mappings have the fallback name + */ + public boolean hasAnyMappingWithFallbackName() { + return roleMappings.stream().anyMatch(RoleMappingMetadata::hasFallbackName); + } + /** * Parse a role mapping from XContent, restoring the name from a reserved metadata field. * Used to parse a role mapping annotated with its name in metadata via @see {@link #copyWithNameInMetadata(ExpressionRoleMapping)}. diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index fdd854e7a9673..9e36055e917a6 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -204,6 +204,29 @@ public void clusterChanged(ClusterChangedEvent event) { return new Tuple<>(savedClusterState, metadataVersion); } + // Wait for any file metadata + public static Tuple setupClusterStateListener(String node) { + ClusterService clusterService = internalCluster().clusterService(node); + CountDownLatch savedClusterState = new CountDownLatch(1); + AtomicLong metadataVersion = new AtomicLong(-1); + clusterService.addListener(new ClusterStateListener() { + @Override + public void clusterChanged(ClusterChangedEvent event) { + ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE); + if (reservedState != null) { + ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedRoleMappingAction.NAME); + if (handlerMetadata != null) { + clusterService.removeListener(this); + metadataVersion.set(event.state().metadata().version()); + savedClusterState.countDown(); + } + } + } + }); + + return new Tuple<>(savedClusterState, metadataVersion); + } + public static Tuple setupClusterStateListenerForCleanup(String node) { ClusterService clusterService = internalCluster().clusterService(node); CountDownLatch savedClusterState = new CountDownLatch(1); diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java new file mode 100644 index 0000000000000..63c510062bdad --- /dev/null +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java @@ -0,0 +1,417 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security.support; + +import org.elasticsearch.action.ActionFuture; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateListener; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.reservedstate.service.FileSettingsService; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.SecurityIntegTestCase; +import org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingRequest; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingResponse; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; +import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; +import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingResponse; +import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.FieldExpression; +import org.junit.Before; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; + +import static org.elasticsearch.integration.RoleMappingFileSettingsIT.setupClusterStateListener; +import static org.elasticsearch.integration.RoleMappingFileSettingsIT.writeJSONFile; +import static org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction.MIGRATION_VERSION_CUSTOM_DATA_KEY; +import static org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction.MIGRATION_VERSION_CUSTOM_KEY; +import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) +public class CleanupRoleMappingDuplicatesMigrationIT extends SecurityIntegTestCase { + + private final AtomicLong versionCounter = new AtomicLong(1); + + @Before + public void resetVersion() { + versionCounter.set(1); + } + + private static final String TEST_JSON_WITH_ROLE_MAPPINGS = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "everyone_kibana_alone": { + "enabled": true, + "roles": [ "kibana_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", + "_foo": "something" + } + }, + "everyone_fleet_alone": { + "enabled": false, + "roles": [ "fleet_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7", + "_foo": "something_else" + } + } + } + } + }"""; + + private static final String TEST_JSON_WITH_FALLBACK_NAME = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "name_not_available_after_deserialization": { + "enabled": true, + "roles": [ "kibana_user", "kibana_admin" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", + "_foo": "something" + } + } + } + } + }"""; + + private static final String TEST_JSON_WITH_EMPTY_ROLE_MAPPINGS = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": {} + } + }"""; + + public void testMigrationSuccessful() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + + // Create a native role mapping to create security index and trigger migration (skipped initially) + createNativeRoleMapping("everyone_kibana_alone"); + createNativeRoleMapping("everyone_fleet_alone"); + createNativeRoleMapping("dont_clean_this_up"); + assertAllRoleMappings("everyone_kibana_alone", "everyone_fleet_alone", "dont_clean_this_up"); + + // Wait for file watcher to start + awaitFileSettingsWatcher(); + // Setup listener to wait for role mapping + var fileBasedRoleMappingsWrittenListener = setupClusterStateListener(masterNode, "everyone_kibana_alone"); + // Write role mappings + writeJSONFile(masterNode, TEST_JSON_WITH_ROLE_MAPPINGS, logger, versionCounter); + assertTrue(fileBasedRoleMappingsWrittenListener.v1().await(20, TimeUnit.SECONDS)); + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + assertAllRoleMappings( + "everyone_kibana_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "everyone_fleet_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "dont_clean_this_up" + ); + } + + public void testMigrationSuccessfulNoOverlap() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + + // Create a native role mapping to create security index and trigger migration (skipped initially) + createNativeRoleMapping("some_native_mapping"); + createNativeRoleMapping("some_other_native_mapping"); + assertAllRoleMappings("some_native_mapping", "some_other_native_mapping"); + + // Wait for file watcher to start + awaitFileSettingsWatcher(); + // Setup listener to wait for role mapping + var fileBasedRoleMappingsWrittenListener = setupClusterStateListener(masterNode, "everyone_kibana_alone"); + // Write role mappings with fallback name, this should block any security migration + writeJSONFile(masterNode, TEST_JSON_WITH_ROLE_MAPPINGS, logger, versionCounter); + assertTrue(fileBasedRoleMappingsWrittenListener.v1().await(20, TimeUnit.SECONDS)); + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + assertAllRoleMappings( + "everyone_kibana_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "everyone_fleet_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "some_native_mapping", + "some_other_native_mapping" + ); + } + + public void testMigrationSuccessfulNoNative() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + + // Create a native role mapping to create security index and trigger migration (skipped initially) + // Then delete it to test an empty role mapping store + createNativeRoleMapping("some_native_mapping"); + deleteNativeRoleMapping("some_native_mapping"); + // Wait for file watcher to start + awaitFileSettingsWatcher(); + // Setup listener to wait for role mapping + var fileBasedRoleMappingsWrittenListener = setupClusterStateListener(masterNode, "everyone_kibana_alone"); + // Write role mappings with fallback name, this should block any security migration + writeJSONFile(masterNode, TEST_JSON_WITH_ROLE_MAPPINGS, logger, versionCounter); + assertTrue(fileBasedRoleMappingsWrittenListener.v1().await(20, TimeUnit.SECONDS)); + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + assertAllRoleMappings( + "everyone_kibana_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "everyone_fleet_alone" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX + ); + } + + public void testMigrationFallbackNamePreCondition() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + // Wait for file watcher to start + awaitFileSettingsWatcher(); + + // Setup listener to wait for role mapping + var nameNotAvailableListener = setupClusterStateListener(masterNode, "name_not_available_after_deserialization"); + // Write role mappings with fallback name, this should block any security migration + writeJSONFile(masterNode, TEST_JSON_WITH_FALLBACK_NAME, logger, versionCounter); + assertTrue(nameNotAvailableListener.v1().await(20, TimeUnit.SECONDS)); + + // Create a native role mapping to create security index and trigger migration + createNativeRoleMapping("everyone_fleet_alone"); + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION - 1); + + // Make sure migration didn't run yet (blocked by the fallback name) + assertMigrationLessThan(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + ClusterService clusterService = internalCluster().getInstance(ClusterService.class); + SecurityIndexManager.RoleMappingsCleanupMigrationStatus status = SecurityIndexManager.getRoleMappingsCleanupMigrationStatus( + clusterService.state(), + SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION - 1 + ); + assertThat(status, equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.NOT_READY)); + + // Write file without fallback name in it to unblock migration + writeJSONFile(masterNode, TEST_JSON_WITH_ROLE_MAPPINGS, logger, versionCounter); + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + } + + public void testSkipMigrationNoFileBasedMappings() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + // Create a native role mapping to create security index and trigger migration (skipped initially) + createNativeRoleMapping("everyone_kibana_alone"); + createNativeRoleMapping("everyone_fleet_alone"); + assertAllRoleMappings("everyone_kibana_alone", "everyone_fleet_alone"); + + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + assertAllRoleMappings("everyone_kibana_alone", "everyone_fleet_alone"); + } + + public void testSkipMigrationEmptyFileBasedMappings() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + + // Wait for file watcher to start + awaitFileSettingsWatcher(); + // Setup listener to wait for any role mapping + var fileBasedRoleMappingsWrittenListener = setupClusterStateListener(masterNode); + // Write role mappings + writeJSONFile(masterNode, TEST_JSON_WITH_EMPTY_ROLE_MAPPINGS, logger, versionCounter); + assertTrue(fileBasedRoleMappingsWrittenListener.v1().await(20, TimeUnit.SECONDS)); + + // Create a native role mapping to create security index and trigger migration (skipped initially) + createNativeRoleMapping("everyone_kibana_alone"); + createNativeRoleMapping("everyone_fleet_alone"); + assertAllRoleMappings("everyone_kibana_alone", "everyone_fleet_alone"); + + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations + resetMigration(); + + // Wait for the first migration to finish + waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION); + + assertAllRoleMappings("everyone_kibana_alone", "everyone_fleet_alone"); + } + + public void testNewIndexSkipMigration() { + internalCluster().setBootstrapMasterNodeIndex(0); + final String masterNode = internalCluster().getMasterName(); + ensureGreen(); + CountDownLatch awaitMigrations = awaitMigrationVersionUpdates( + masterNode, + SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION + ); + // Create a native role mapping to create security index and trigger migration + createNativeRoleMapping("everyone_kibana_alone"); + // Make sure no migration ran (set to current version without applying prior migrations) + safeAwait(awaitMigrations); + } + + /** + * Make sure all versions are applied to cluster state sequentially + */ + private CountDownLatch awaitMigrationVersionUpdates(String node, final int... versions) { + final ClusterService clusterService = internalCluster().clusterService(node); + final CountDownLatch allVersionsCountDown = new CountDownLatch(1); + final AtomicInteger currentVersionIdx = new AtomicInteger(0); + clusterService.addListener(new ClusterStateListener() { + @Override + public void clusterChanged(ClusterChangedEvent event) { + int currentMigrationVersion = getCurrentMigrationVersion(event.state()); + if (currentMigrationVersion > 0) { + assertThat(versions[currentVersionIdx.get()], lessThanOrEqualTo(currentMigrationVersion)); + if (versions[currentVersionIdx.get()] == currentMigrationVersion) { + currentVersionIdx.incrementAndGet(); + } + + if (currentVersionIdx.get() >= versions.length) { + clusterService.removeListener(this); + allVersionsCountDown.countDown(); + } + } + } + }); + + return allVersionsCountDown; + } + + private void assertAllRoleMappings(String... roleMappingNames) { + GetRoleMappingsResponse response = client().execute(GetRoleMappingsAction.INSTANCE, new GetRoleMappingsRequest()).actionGet(); + + assertTrue(response.hasMappings()); + assertThat(response.mappings().length, equalTo(roleMappingNames.length)); + + assertThat( + Arrays.stream(response.mappings()).map(ExpressionRoleMapping::getName).toList(), + containsInAnyOrder( + roleMappingNames + + ) + ); + } + + private void awaitFileSettingsWatcher() throws Exception { + final String masterNode = internalCluster().getMasterName(); + FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode); + assertBusy(() -> assertTrue(masterFileSettingsService.watching())); + } + + private void resetMigration() { + client().execute( + UpdateIndexMigrationVersionAction.INSTANCE, + // -1 is a hack, since running a migration on version 0 on a new cluster will cause all migrations to be skipped (not needed) + new UpdateIndexMigrationVersionAction.Request(TimeValue.MAX_VALUE, -1, INTERNAL_SECURITY_MAIN_INDEX_7) + ).actionGet(); + } + + private void createNativeRoleMapping(String name) { + PutRoleMappingRequest request = new PutRoleMappingRequest(); + request.setName(name); + request.setRules(new FieldExpression("username", Collections.singletonList(new FieldExpression.FieldValue("*")))); + request.setRoles(List.of("superuser")); + + ActionFuture response = client().execute(PutRoleMappingAction.INSTANCE, request); + response.actionGet(); + } + + private void deleteNativeRoleMapping(String name) { + DeleteRoleMappingRequest request = new DeleteRoleMappingRequest(); + request.setName(name); + + ActionFuture response = client().execute(DeleteRoleMappingAction.INSTANCE, request); + response.actionGet(); + } + + private void assertMigrationVersionAtLeast(int expectedVersion) { + assertThat(getCurrentMigrationVersion(), greaterThanOrEqualTo(expectedVersion)); + } + + private void assertMigrationLessThan(int expectedVersion) { + assertThat(getCurrentMigrationVersion(), lessThan(expectedVersion)); + } + + private int getCurrentMigrationVersion(ClusterState state) { + IndexMetadata indexMetadata = state.metadata().getIndices().get(INTERNAL_SECURITY_MAIN_INDEX_7); + if (indexMetadata == null || indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY) == null) { + return 0; + } + return Integer.parseInt(indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY).get(MIGRATION_VERSION_CUSTOM_DATA_KEY)); + } + + private int getCurrentMigrationVersion() { + ClusterService clusterService = internalCluster().getInstance(ClusterService.class); + return getCurrentMigrationVersion(clusterService.state()); + } + + private void waitForMigrationCompletion(int version) throws Exception { + assertBusy(() -> assertMigrationVersionAtLeast(version)); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java index c1fe553f41334..d0292f32cd75f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java @@ -17,13 +17,14 @@ import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MIGRATION_FRAMEWORK; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_PROFILE_ORIGIN_FEATURE; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLES_METADATA_FLATTENED; +import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLE_MAPPING_CLEANUP; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.VERSION_SECURITY_PROFILE_ORIGIN; public class SecurityFeatures implements FeatureSpecification { @Override public Set getFeatures() { - return Set.of(SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK); + return Set.of(SECURITY_ROLE_MAPPING_CLEANUP, SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK); } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 6d9b0ef6aeebe..12ef800a7aae7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -31,6 +31,7 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.ReservedStateMetadata; import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.core.TimeValue; @@ -46,7 +47,9 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.threadpool.Scheduler; import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata; import org.elasticsearch.xpack.security.SecurityFeatures; +import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; import java.time.Instant; import java.util.List; @@ -75,7 +78,7 @@ public class SecurityIndexManager implements ClusterStateListener { public static final String SECURITY_VERSION_STRING = "security-version"; - + protected static final String FILE_SETTINGS_METADATA_NAMESPACE = "file_settings"; private static final Logger logger = LogManager.getLogger(SecurityIndexManager.class); /** @@ -86,6 +89,13 @@ public enum Availability { PRIMARY_SHARDS } + public enum RoleMappingsCleanupMigrationStatus { + READY, + NOT_READY, + SKIP, + DONE + } + private final Client client; private final SystemIndexDescriptor systemIndexDescriptor; @@ -195,10 +205,6 @@ public boolean isMigrationsVersionAtLeast(Integer expectedMigrationsVersion) { return indexExists() && this.state.migrationsVersion.compareTo(expectedMigrationsVersion) >= 0; } - public boolean isCreatedOnLatestVersion() { - return this.state.createdOnLatestVersion; - } - public ElasticsearchException getUnavailableReason(Availability availability) { // ensure usage of a local copy so all checks execute against the same state! if (defensiveCopy == false) { @@ -261,6 +267,7 @@ private SystemIndexDescriptor.MappingsVersion getMinSecurityIndexMappingVersion( /** * Check if the index was created on the latest index version available in the cluster */ + private static boolean isCreatedOnLatestVersion(IndexMetadata indexMetadata) { final IndexVersion indexVersionCreated = indexMetadata != null ? SETTING_INDEX_VERSION_CREATED.get(indexMetadata.getSettings()) @@ -268,6 +275,50 @@ private static boolean isCreatedOnLatestVersion(IndexMetadata indexMetadata) { return indexVersionCreated != null && indexVersionCreated.onOrAfter(IndexVersion.current()); } + /** + * Check if a role mappings cleanup migration is needed or has already been performed and if the cluster is ready for a cleanup + * migration + * + * @param clusterState current cluster state + * @param migrationsVersion current migration version + * + * @return RoleMappingsCleanupMigrationStatus + */ + static RoleMappingsCleanupMigrationStatus getRoleMappingsCleanupMigrationStatus(ClusterState clusterState, int migrationsVersion) { + // Migration already finished + if (migrationsVersion >= SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION) { + return RoleMappingsCleanupMigrationStatus.DONE; + } + + ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FILE_SETTINGS_METADATA_NAMESPACE); + boolean hasFileSettingsMetadata = fileSettingsMetadata != null; + // If there is no fileSettingsMetadata, there should be no reserved state (this is to catch bugs related to + // name changes to FILE_SETTINGS_METADATA_NAMESPACE) + assert hasFileSettingsMetadata || clusterState.metadata().reservedStateMetadata().isEmpty() + : "ReservedStateMetadata contains unknown namespace"; + + // If no file based role mappings available -> migration not needed + if (hasFileSettingsMetadata == false || fileSettingsMetadata.keys(ReservedRoleMappingAction.NAME).isEmpty()) { + return RoleMappingsCleanupMigrationStatus.SKIP; + } + + RoleMappingMetadata roleMappingMetadata = RoleMappingMetadata.getFromClusterState(clusterState); + + // If there are file based role mappings, make sure they have the latest format (name available) and that they have all been + // synced to cluster state (same size as the reserved state keys) + if (roleMappingMetadata.getRoleMappings().size() == fileSettingsMetadata.keys(ReservedRoleMappingAction.NAME).size() + && roleMappingMetadata.hasAnyMappingWithFallbackName() == false) { + return RoleMappingsCleanupMigrationStatus.READY; + } + + // If none of the above conditions are met, wait for a state change to re-evaluate if the cluster is ready for migration + return RoleMappingsCleanupMigrationStatus.NOT_READY; + } + + public RoleMappingsCleanupMigrationStatus getRoleMappingsCleanupMigrationStatus() { + return state.roleMappingsCleanupMigrationStatus; + } + @Override public void clusterChanged(ClusterChangedEvent event) { if (event.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) { @@ -285,8 +336,12 @@ public void clusterChanged(ClusterChangedEvent event) { Tuple available = checkIndexAvailable(event.state()); final boolean indexAvailableForWrite = available.v1(); final boolean indexAvailableForSearch = available.v2(); - final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state()); final int migrationsVersion = getMigrationVersionFromIndexMetadata(indexMetadata); + final RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus = getRoleMappingsCleanupMigrationStatus( + event.state(), + migrationsVersion + ); + final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state()); final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(event.state()); final int indexMappingVersion = loadIndexMappingVersion(systemIndexDescriptor.getAliasName(), event.state()); final String concreteIndexName = indexMetadata == null @@ -315,6 +370,7 @@ public void clusterChanged(ClusterChangedEvent event) { indexAvailableForWrite, mappingIsUpToDate, createdOnLatestVersion, + roleMappingsCleanupMigrationStatus, migrationsVersion, minClusterMappingVersion, indexMappingVersion, @@ -474,7 +530,8 @@ private Tuple checkIndexAvailable(ClusterState state) { public boolean isEligibleSecurityMigration(SecurityMigrations.SecurityMigration securityMigration) { return state.securityFeatures.containsAll(securityMigration.nodeFeaturesRequired()) - && state.indexMappingVersion >= securityMigration.minMappingVersion(); + && state.indexMappingVersion >= securityMigration.minMappingVersion() + && securityMigration.checkPreConditions(state); } public boolean isReadyForSecurityMigration(SecurityMigrations.SecurityMigration securityMigration) { @@ -680,6 +737,10 @@ public void onFailure(Exception e) { } } + public boolean isCreatedOnLatestVersion() { + return state.createdOnLatestVersion; + } + /** * Return true if the state moves from an unhealthy ("RED") index state to a healthy ("non-RED") state. */ @@ -714,6 +775,7 @@ public static class State { null, null, null, + null, Set.of() ); public final Instant creationTime; @@ -722,6 +784,7 @@ public static class State { public final boolean indexAvailableForWrite; public final boolean mappingUpToDate; public final boolean createdOnLatestVersion; + public final RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus; public final Integer migrationsVersion; // Min mapping version supported by the descriptors in the cluster public final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion; @@ -740,6 +803,7 @@ public State( boolean indexAvailableForWrite, boolean mappingUpToDate, boolean createdOnLatestVersion, + RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus, Integer migrationsVersion, SystemIndexDescriptor.MappingsVersion minClusterMappingVersion, Integer indexMappingVersion, @@ -756,6 +820,7 @@ public State( this.mappingUpToDate = mappingUpToDate; this.migrationsVersion = migrationsVersion; this.createdOnLatestVersion = createdOnLatestVersion; + this.roleMappingsCleanupMigrationStatus = roleMappingsCleanupMigrationStatus; this.minClusterMappingVersion = minClusterMappingVersion; this.indexMappingVersion = indexMappingVersion; this.concreteIndexName = concreteIndexName; @@ -776,6 +841,7 @@ public boolean equals(Object o) { && indexAvailableForWrite == state.indexAvailableForWrite && mappingUpToDate == state.mappingUpToDate && createdOnLatestVersion == state.createdOnLatestVersion + && roleMappingsCleanupMigrationStatus == state.roleMappingsCleanupMigrationStatus && Objects.equals(indexMappingVersion, state.indexMappingVersion) && Objects.equals(migrationsVersion, state.migrationsVersion) && Objects.equals(minClusterMappingVersion, state.minClusterMappingVersion) @@ -798,6 +864,7 @@ public int hashCode() { indexAvailableForWrite, mappingUpToDate, createdOnLatestVersion, + roleMappingsCleanupMigrationStatus, migrationsVersion, minClusterMappingVersion, indexMappingVersion, @@ -822,6 +889,8 @@ public String toString() { + mappingUpToDate + ", createdOnLatestVersion=" + createdOnLatestVersion + + ", roleMappingsCleanupMigrationStatus=" + + roleMappingsCleanupMigrationStatus + ", migrationsVersion=" + migrationsVersion + ", minClusterMappingVersion=" diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java index 5cd8cba763d3d..203dec9e25b91 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java @@ -11,6 +11,8 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.support.GroupedActionListener; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.internal.Client; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.query.BoolQueryBuilder; @@ -20,20 +22,35 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingRequestBuilder; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingResponse; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequestBuilder; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; +import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.stream.Collectors; +import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN; +import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; +import static org.elasticsearch.xpack.security.support.SecurityIndexManager.RoleMappingsCleanupMigrationStatus.READY; +import static org.elasticsearch.xpack.security.support.SecurityIndexManager.RoleMappingsCleanupMigrationStatus.SKIP; +import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SecurityMainIndexMappingVersion.ADD_MANAGE_ROLES_PRIVILEGE; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SecurityMainIndexMappingVersion.ADD_REMOTE_CLUSTER_AND_DESCRIPTION_FIELDS; -/** - * Interface for creating SecurityMigrations that will be automatically applied once to existing .security indices - * IMPORTANT: A new index version needs to be added to {@link org.elasticsearch.index.IndexVersions} for the migration to be triggered - */ public class SecurityMigrations { + /** + * Interface for creating SecurityMigrations that will be automatically applied once to existing .security indices + * IMPORTANT: A new index version needs to be added to {@link org.elasticsearch.index.IndexVersions} for the migration to be triggered + */ public interface SecurityMigration { /** * Method that will execute the actual migration - needs to be idempotent and non-blocking @@ -52,6 +69,16 @@ public interface SecurityMigration { */ Set nodeFeaturesRequired(); + /** + * Check that any pre-conditions are met before launching migration + * + * @param securityIndexManagerState current state of the security index + * @return true if pre-conditions met, otherwise false + */ + default boolean checkPreConditions(SecurityIndexManager.State securityIndexManagerState) { + return true; + } + /** * The min mapping version required to support this migration. This makes sure that the index has at least the min mapping that is * required to support the migration. @@ -62,63 +89,163 @@ public interface SecurityMigration { } public static final Integer ROLE_METADATA_FLATTENED_MIGRATION_VERSION = 1; + public static final Integer CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION = 2; + private static final Logger logger = LogManager.getLogger(SecurityMigration.class); public static final TreeMap MIGRATIONS_BY_VERSION = new TreeMap<>( - Map.of(ROLE_METADATA_FLATTENED_MIGRATION_VERSION, new SecurityMigration() { - private static final Logger logger = LogManager.getLogger(SecurityMigration.class); - - @Override - public void migrate(SecurityIndexManager indexManager, Client client, ActionListener listener) { - BoolQueryBuilder filterQuery = new BoolQueryBuilder().filter(QueryBuilders.termQuery("type", "role")) - .mustNot(QueryBuilders.existsQuery("metadata_flattened")); - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(filterQuery).size(0).trackTotalHits(true); - SearchRequest countRequest = new SearchRequest(indexManager.getConcreteIndexName()); - countRequest.source(searchSourceBuilder); - - client.search(countRequest, ActionListener.wrap(response -> { - // If there are no roles, skip migration - if (response.getHits().getTotalHits().value() > 0) { - logger.info("Preparing to migrate [" + response.getHits().getTotalHits().value() + "] roles"); - updateRolesByQuery(indexManager, client, filterQuery, listener); - } else { - listener.onResponse(null); - } + Map.of( + ROLE_METADATA_FLATTENED_MIGRATION_VERSION, + new RoleMetadataFlattenedMigration(), + CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION, + new CleanupRoleMappingDuplicatesMigration() + ) + ); + + public static class RoleMetadataFlattenedMigration implements SecurityMigration { + @Override + public void migrate(SecurityIndexManager indexManager, Client client, ActionListener listener) { + BoolQueryBuilder filterQuery = new BoolQueryBuilder().filter(QueryBuilders.termQuery("type", "role")) + .mustNot(QueryBuilders.existsQuery("metadata_flattened")); + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(filterQuery).size(0).trackTotalHits(true); + SearchRequest countRequest = new SearchRequest(indexManager.getConcreteIndexName()); + countRequest.source(searchSourceBuilder); + + client.search(countRequest, ActionListener.wrap(response -> { + // If there are no roles, skip migration + if (response.getHits().getTotalHits().value() > 0) { + logger.info("Preparing to migrate [" + response.getHits().getTotalHits().value() + "] roles"); + updateRolesByQuery(indexManager, client, filterQuery, listener); + } else { + listener.onResponse(null); + } + }, listener::onFailure)); + } + + private void updateRolesByQuery( + SecurityIndexManager indexManager, + Client client, + BoolQueryBuilder filterQuery, + ActionListener listener + ) { + UpdateByQueryRequest updateByQueryRequest = new UpdateByQueryRequest(indexManager.getConcreteIndexName()); + updateByQueryRequest.setQuery(filterQuery); + updateByQueryRequest.setScript( + new Script(ScriptType.INLINE, "painless", "ctx._source.metadata_flattened = ctx._source.metadata", Collections.emptyMap()) + ); + client.admin() + .cluster() + .execute(UpdateByQueryAction.INSTANCE, updateByQueryRequest, ActionListener.wrap(bulkByScrollResponse -> { + logger.info("Migrated [" + bulkByScrollResponse.getTotal() + "] roles"); + listener.onResponse(null); }, listener::onFailure)); + } + + @Override + public Set nodeFeaturesRequired() { + return Set.of(SecuritySystemIndices.SECURITY_ROLES_METADATA_FLATTENED); + } + + @Override + public int minMappingVersion() { + return ADD_REMOTE_CLUSTER_AND_DESCRIPTION_FIELDS.id(); + } + } + + public static class CleanupRoleMappingDuplicatesMigration implements SecurityMigration { + @Override + public void migrate(SecurityIndexManager indexManager, Client client, ActionListener listener) { + if (indexManager.getRoleMappingsCleanupMigrationStatus() == SKIP) { + listener.onResponse(null); + return; } + assert indexManager.getRoleMappingsCleanupMigrationStatus() == READY; - private void updateRolesByQuery( - SecurityIndexManager indexManager, - Client client, - BoolQueryBuilder filterQuery, - ActionListener listener - ) { - UpdateByQueryRequest updateByQueryRequest = new UpdateByQueryRequest(indexManager.getConcreteIndexName()); - updateByQueryRequest.setQuery(filterQuery); - updateByQueryRequest.setScript( - new Script( - ScriptType.INLINE, - "painless", - "ctx._source.metadata_flattened = ctx._source.metadata", - Collections.emptyMap() - ) + getRoleMappings(client, ActionListener.wrap(roleMappings -> { + List roleMappingsToDelete = getDuplicateRoleMappingNames(roleMappings.mappings()); + if (roleMappingsToDelete.isEmpty() == false) { + logger.info("Found [" + roleMappingsToDelete.size() + "] role mapping(s) to cleanup in .security index."); + deleteNativeRoleMappings(client, roleMappingsToDelete, listener); + } else { + listener.onResponse(null); + } + }, listener::onFailure)); + } + + private void getRoleMappings(Client client, ActionListener listener) { + executeAsyncWithOrigin( + client, + SECURITY_ORIGIN, + GetRoleMappingsAction.INSTANCE, + new GetRoleMappingsRequestBuilder(client).request(), + listener + ); + } + + private void deleteNativeRoleMappings(Client client, List names, ActionListener listener) { + assert names.isEmpty() == false; + ActionListener groupListener = new GroupedActionListener<>( + names.size(), + ActionListener.wrap(responses -> { + long foundRoleMappings = responses.stream().filter(DeleteRoleMappingResponse::isFound).count(); + if (responses.size() > foundRoleMappings) { + logger.warn( + "[" + (responses.size() - foundRoleMappings) + "] Role mapping(s) not found during role mapping clean up." + ); + } + if (foundRoleMappings > 0) { + logger.info("Deleted [" + foundRoleMappings + "] duplicated role mapping(s) from .security index"); + } + listener.onResponse(null); + }, listener::onFailure) + ); + + for (String name : names) { + executeAsyncWithOrigin( + client, + SECURITY_ORIGIN, + DeleteRoleMappingAction.INSTANCE, + new DeleteRoleMappingRequestBuilder(client).name(name).setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).request(), + groupListener ); - client.admin() - .cluster() - .execute(UpdateByQueryAction.INSTANCE, updateByQueryRequest, ActionListener.wrap(bulkByScrollResponse -> { - logger.info("Migrated [" + bulkByScrollResponse.getTotal() + "] roles"); - listener.onResponse(null); - }, listener::onFailure)); } - @Override - public Set nodeFeaturesRequired() { - return Set.of(SecuritySystemIndices.SECURITY_ROLES_METADATA_FLATTENED); - } + } - @Override - public int minMappingVersion() { - return ADD_REMOTE_CLUSTER_AND_DESCRIPTION_FIELDS.id(); - } - }) - ); + @Override + public boolean checkPreConditions(SecurityIndexManager.State securityIndexManagerState) { + // Block migration until expected role mappings are in cluster state and in the correct format or skip if no role mappings + // are expected + return securityIndexManagerState.roleMappingsCleanupMigrationStatus == READY + || securityIndexManagerState.roleMappingsCleanupMigrationStatus == SKIP; + } + + @Override + public Set nodeFeaturesRequired() { + return Set.of(SecuritySystemIndices.SECURITY_ROLE_MAPPING_CLEANUP); + } + + @Override + public int minMappingVersion() { + return ADD_MANAGE_ROLES_PRIVILEGE.id(); + } + + // Visible for testing + protected static List getDuplicateRoleMappingNames(ExpressionRoleMapping... roleMappings) { + // Partition role mappings on if they're cluster state role mappings (true) or native role mappings (false) + Map> partitionedRoleMappings = Arrays.stream(roleMappings) + .collect(Collectors.partitioningBy(ExpressionRoleMapping::isReadOnly)); + + Set clusterStateRoleMappings = partitionedRoleMappings.get(true) + .stream() + .map(ExpressionRoleMapping::getName) + .map(ExpressionRoleMapping::removeReadOnlySuffixIfPresent) + .collect(Collectors.toSet()); + + return partitionedRoleMappings.get(false) + .stream() + .map(ExpressionRoleMapping::getName) + .filter(clusterStateRoleMappings::contains) + .toList(); + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecuritySystemIndices.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecuritySystemIndices.java index 36ea14c6e101b..77c7d19e94a9b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecuritySystemIndices.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecuritySystemIndices.java @@ -61,6 +61,7 @@ public class SecuritySystemIndices { public static final NodeFeature SECURITY_PROFILE_ORIGIN_FEATURE = new NodeFeature("security.security_profile_origin"); public static final NodeFeature SECURITY_MIGRATION_FRAMEWORK = new NodeFeature("security.migration_framework"); public static final NodeFeature SECURITY_ROLES_METADATA_FLATTENED = new NodeFeature("security.roles_metadata_flattened"); + public static final NodeFeature SECURITY_ROLE_MAPPING_CLEANUP = new NodeFeature("security.role_mapping_cleanup"); /** * Security managed index mappings used to be updated based on the product version. They are now updated based on per-index mappings diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java index e1c3b936e5a32..cd6c88cf525af 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java @@ -2518,6 +2518,7 @@ private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) { null, null, null, + null, concreteSecurityIndexName, indexStatus, IndexMetadata.State.OPEN, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java index 2254c78a2910c..75d5959f351f0 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java @@ -43,6 +43,7 @@ private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) { null, null, null, + null, concreteSecurityIndexName, indexStatus, IndexMetadata.State.OPEN, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java index 38f01d4d18bc7..ca84a9189d90a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java @@ -415,6 +415,7 @@ private SecurityIndexManager.State indexState(boolean isUpToDate, ClusterHealthS null, null, null, + null, concreteSecurityIndexName, healthStatus, IndexMetadata.State.OPEN, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index 9587533d87d86..da903ff7f7177 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -1702,6 +1702,7 @@ public SecurityIndexManager.State dummyIndexState(boolean isIndexUpToDate, Clust null, null, null, + null, concreteSecurityIndexName, healthStatus, IndexMetadata.State.OPEN, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java index f91cb567ba689..73a45dc20ac42 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java @@ -904,6 +904,7 @@ private SecurityIndexManager.State dummyState( null, null, null, + null, concreteSecurityIndexName, healthStatus, IndexMetadata.State.OPEN, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java index e3b00dfbcc6b8..d551dded4e566 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java @@ -63,6 +63,7 @@ public void testSecurityIndexStateChangeWillInvalidateAllRegisteredInvalidators( true, true, null, + null, new SystemIndexDescriptor.MappingsVersion(SecurityMainIndexMappingVersion.latest().id(), 0), null, ".security", diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index 493483a5e4a1b..0b98a595a6ab9 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -23,6 +23,8 @@ import org.elasticsearch.cluster.metadata.AliasMetadata; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.ReservedStateHandlerMetadata; +import org.elasticsearch.cluster.metadata.ReservedStateMetadata; import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodeUtils; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -51,8 +53,11 @@ import org.elasticsearch.test.client.NoOpClient; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata; import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices; import org.elasticsearch.xpack.security.SecurityFeatures; +import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; import org.elasticsearch.xpack.security.support.SecuritySystemIndices.SecurityMainIndexMappingVersion; import org.elasticsearch.xpack.security.test.SecurityTestUtils; import org.hamcrest.Matchers; @@ -70,6 +75,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.xpack.security.support.SecurityIndexManager.FILE_SETTINGS_METADATA_NAMESPACE; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -654,6 +660,138 @@ public int minMappingVersion() { })); } + public void testNotReadyForMigrationBecauseOfPrecondition() { + final ClusterState.Builder clusterStateBuilder = createClusterState( + TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7, + SecuritySystemIndices.SECURITY_MAIN_ALIAS, + IndexMetadata.State.OPEN + ); + clusterStateBuilder.nodeFeatures( + Map.of("1", new SecurityFeatures().getFeatures().stream().map(NodeFeature::id).collect(Collectors.toSet())) + ); + manager.clusterChanged(event(markShardsAvailable(clusterStateBuilder))); + assertFalse(manager.isReadyForSecurityMigration(new SecurityMigrations.SecurityMigration() { + @Override + public void migrate(SecurityIndexManager indexManager, Client client, ActionListener listener) { + listener.onResponse(null); + } + + @Override + public Set nodeFeaturesRequired() { + return Set.of(); + } + + @Override + public int minMappingVersion() { + return 0; + } + + @Override + public boolean checkPreConditions(SecurityIndexManager.State securityIndexManagerState) { + return false; + } + })); + } + + private ClusterState.Builder clusterStateBuilderForMigrationTesting() { + return createClusterState( + TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7, + SecuritySystemIndices.SECURITY_MAIN_ALIAS, + IndexMetadata.State.OPEN + ); + } + + public void testGetRoleMappingsCleanupMigrationStatus() { + { + assertThat( + SecurityIndexManager.getRoleMappingsCleanupMigrationStatus( + clusterStateBuilderForMigrationTesting().build(), + SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION + ), + equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.DONE) + ); + } + { + // Migration should be skipped + ClusterState.Builder clusterStateBuilder = clusterStateBuilderForMigrationTesting(); + Metadata.Builder metadataBuilder = new Metadata.Builder(); + metadataBuilder.put(ReservedStateMetadata.builder(FILE_SETTINGS_METADATA_NAMESPACE).build()); + assertThat( + SecurityIndexManager.getRoleMappingsCleanupMigrationStatus(clusterStateBuilder.metadata(metadataBuilder).build(), 1), + equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.SKIP) + ); + } + { + // Not ready for migration + ClusterState.Builder clusterStateBuilder = clusterStateBuilderForMigrationTesting(); + Metadata.Builder metadataBuilder = new Metadata.Builder(); + ReservedStateMetadata.Builder builder = ReservedStateMetadata.builder(FILE_SETTINGS_METADATA_NAMESPACE); + // File settings role mappings exist + ReservedStateHandlerMetadata reservedStateHandlerMetadata = new ReservedStateHandlerMetadata( + ReservedRoleMappingAction.NAME, + Set.of("role_mapping_1") + ); + builder.putHandler(reservedStateHandlerMetadata); + metadataBuilder.put(builder.build()); + + // No role mappings in cluster state yet + metadataBuilder.putCustom(RoleMappingMetadata.TYPE, new RoleMappingMetadata(Set.of())); + + assertThat( + SecurityIndexManager.getRoleMappingsCleanupMigrationStatus(clusterStateBuilder.metadata(metadataBuilder).build(), 1), + equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.NOT_READY) + ); + } + { + // Old role mappings in cluster state + final ClusterState.Builder clusterStateBuilder = clusterStateBuilderForMigrationTesting(); + Metadata.Builder metadataBuilder = new Metadata.Builder(); + ReservedStateMetadata.Builder builder = ReservedStateMetadata.builder(FILE_SETTINGS_METADATA_NAMESPACE); + // File settings role mappings exist + ReservedStateHandlerMetadata reservedStateHandlerMetadata = new ReservedStateHandlerMetadata( + ReservedRoleMappingAction.NAME, + Set.of("role_mapping_1") + ); + builder.putHandler(reservedStateHandlerMetadata); + metadataBuilder.put(builder.build()); + + // Role mappings in cluster state with fallback name + metadataBuilder.putCustom( + RoleMappingMetadata.TYPE, + new RoleMappingMetadata(Set.of(new ExpressionRoleMapping(RoleMappingMetadata.FALLBACK_NAME, null, null, null, null, true))) + ); + + assertThat( + SecurityIndexManager.getRoleMappingsCleanupMigrationStatus(clusterStateBuilder.metadata(metadataBuilder).build(), 1), + equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.NOT_READY) + ); + } + { + // Ready for migration + final ClusterState.Builder clusterStateBuilder = clusterStateBuilderForMigrationTesting(); + Metadata.Builder metadataBuilder = new Metadata.Builder(); + ReservedStateMetadata.Builder builder = ReservedStateMetadata.builder(FILE_SETTINGS_METADATA_NAMESPACE); + // File settings role mappings exist + ReservedStateHandlerMetadata reservedStateHandlerMetadata = new ReservedStateHandlerMetadata( + ReservedRoleMappingAction.NAME, + Set.of("role_mapping_1") + ); + builder.putHandler(reservedStateHandlerMetadata); + metadataBuilder.put(builder.build()); + + // Role mappings in cluster state + metadataBuilder.putCustom( + RoleMappingMetadata.TYPE, + new RoleMappingMetadata(Set.of(new ExpressionRoleMapping("role_mapping_1", null, null, null, null, true))) + ); + + assertThat( + SecurityIndexManager.getRoleMappingsCleanupMigrationStatus(clusterStateBuilder.metadata(metadataBuilder).build(), 1), + equalTo(SecurityIndexManager.RoleMappingsCleanupMigrationStatus.READY) + ); + } + } + public void testProcessClosedIndexState() { // Index initially exists final ClusterState.Builder indexAvailable = createClusterState( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityMigrationsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityMigrationsTests.java new file mode 100644 index 0000000000000..3d3cc47b55cf6 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityMigrationsTests.java @@ -0,0 +1,174 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.xpack.security.support; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.client.internal.Client; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.TestThreadPool; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingRequest; +import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingResponse; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; +import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import org.junit.After; +import org.junit.Before; + +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SecurityMigrationsTests extends ESTestCase { + private ThreadPool threadPool; + private Client client; + + public void testGetDuplicateRoleMappingNames() { + assertThat(SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames(), empty()); + assertThat( + SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames( + nativeRoleMapping("roleMapping1"), + nativeRoleMapping("roleMapping2") + ), + empty() + ); + assertThat( + SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames( + nativeRoleMapping("roleMapping1"), + reservedRoleMapping("roleMapping1") + ), + equalTo(List.of("roleMapping1")) + ); + + { + List duplicates = SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames( + nativeRoleMapping("roleMapping1"), + nativeRoleMapping("roleMapping2"), + reservedRoleMapping("roleMapping1"), + reservedRoleMapping("roleMapping2") + ); + assertThat(duplicates, hasSize(2)); + assertThat(duplicates, containsInAnyOrder("roleMapping1", "roleMapping2")); + } + { + List duplicates = SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames( + nativeRoleMapping("roleMapping1"), + nativeRoleMapping("roleMapping2"), + nativeRoleMapping("roleMapping3"), + reservedRoleMapping("roleMapping1"), + reservedRoleMapping("roleMapping2"), + reservedRoleMapping("roleMapping4") + ); + assertThat(duplicates, hasSize(2)); + assertThat(duplicates, containsInAnyOrder("roleMapping1", "roleMapping2")); + } + { + List duplicates = SecurityMigrations.CleanupRoleMappingDuplicatesMigration.getDuplicateRoleMappingNames( + nativeRoleMapping("roleMapping1" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX), + nativeRoleMapping("roleMapping2" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX), + nativeRoleMapping("roleMapping3"), + reservedRoleMapping("roleMapping1"), + reservedRoleMapping("roleMapping2"), + reservedRoleMapping("roleMapping3") + ); + assertThat(duplicates, hasSize(1)); + assertThat(duplicates, containsInAnyOrder("roleMapping3")); + } + } + + private static ExpressionRoleMapping reservedRoleMapping(String name) { + return new ExpressionRoleMapping( + name + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + null, + null, + null, + Map.of(ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG, true), + true + ); + } + + private static ExpressionRoleMapping nativeRoleMapping(String name) { + return new ExpressionRoleMapping(name, null, null, null, randomBoolean() ? null : Map.of(), true); + } + + public void testCleanupRoleMappingDuplicatesMigrationPartialFailure() { + // Make sure migration continues even if a duplicate is not found + SecurityIndexManager securityIndexManager = mock(SecurityIndexManager.class); + when(securityIndexManager.getRoleMappingsCleanupMigrationStatus()).thenReturn( + SecurityIndexManager.RoleMappingsCleanupMigrationStatus.READY + ); + doAnswer(inv -> { + final Object[] args = inv.getArguments(); + @SuppressWarnings("unchecked") + ActionListener listener = (ActionListener) args[2]; + listener.onResponse( + new GetRoleMappingsResponse( + nativeRoleMapping("duplicate-0"), + reservedRoleMapping("duplicate-0"), + nativeRoleMapping("duplicate-1"), + reservedRoleMapping("duplicate-1"), + nativeRoleMapping("duplicate-2"), + reservedRoleMapping("duplicate-2") + ) + ); + return null; + }).when(client).execute(eq(GetRoleMappingsAction.INSTANCE), any(GetRoleMappingsRequest.class), any()); + + final boolean[] duplicatesDeleted = new boolean[3]; + doAnswer(inv -> { + final Object[] args = inv.getArguments(); + @SuppressWarnings("unchecked") + ActionListener listener = (ActionListener) args[2]; + DeleteRoleMappingRequest request = (DeleteRoleMappingRequest) args[1]; + if (request.getName().equals("duplicate-0")) { + duplicatesDeleted[0] = true; + } + if (request.getName().equals("duplicate-1")) { + if (randomBoolean()) { + listener.onResponse(new DeleteRoleMappingResponse(false)); + } else { + listener.onFailure(new IllegalStateException("bad state")); + } + } + if (request.getName().equals("duplicate-2")) { + duplicatesDeleted[2] = true; + } + return null; + }).when(client).execute(eq(DeleteRoleMappingAction.INSTANCE), any(DeleteRoleMappingRequest.class), any()); + + SecurityMigrations.SecurityMigration securityMigration = new SecurityMigrations.CleanupRoleMappingDuplicatesMigration(); + securityMigration.migrate(securityIndexManager, client, ActionListener.noop()); + + assertTrue(duplicatesDeleted[0]); + assertFalse(duplicatesDeleted[1]); + assertTrue(duplicatesDeleted[2]); + } + + @Before + public void createClientAndThreadPool() { + threadPool = new TestThreadPool("cleanup role mappings test pool"); + client = mock(Client.class); + when(client.threadPool()).thenReturn(threadPool); + } + + @After + public void stopThreadPool() { + terminate(threadPool); + } + +} diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index b9b0531fa5b68..38fbf99068a9b 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -88,6 +88,8 @@ BuildParams.bwcVersions.withWireCompatible { bwcVersion, baseName -> keystore 'xpack.watcher.encryption_key', file("${project.projectDir}/src/test/resources/system_key") setting 'xpack.watcher.encrypt_sensitive_data', 'true' + extraConfigFile 'operator/settings.json', file("${project.projectDir}/src/test/resources/operator_defined_role_mappings.json") + // Old versions of the code contain an invalid assertion that trips // during tests. Versions 5.6.9 and 6.2.4 have been fixed by removing // the assertion, but this is impossible for released versions. diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/AbstractUpgradeTestCase.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/AbstractUpgradeTestCase.java index 4324aed5fee18..b17644cd1c2a9 100644 --- a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/AbstractUpgradeTestCase.java +++ b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/AbstractUpgradeTestCase.java @@ -9,11 +9,13 @@ import org.elasticsearch.Build; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Booleans; +import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xpack.test.SecuritySettingsSourceField; import org.junit.Before; @@ -21,6 +23,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; public abstract class AbstractUpgradeTestCase extends ESRestTestCase { @@ -149,4 +152,22 @@ public void setupForTests() throws Exception { } }); } + + protected static void waitForSecurityMigrationCompletion(RestClient adminClient, int version) throws Exception { + final Request request = new Request("GET", "_cluster/state/metadata/.security-7"); + assertBusy(() -> { + Map indices = new XContentTestUtils.JsonMapView(entityAsMap(adminClient.performRequest(request))).get( + "metadata.indices" + ); + assertNotNull(indices); + assertTrue(indices.containsKey(".security-7")); + // JsonMapView doesn't support . prefixed indices (splits on .) + @SuppressWarnings("unchecked") + String responseVersion = new XContentTestUtils.JsonMapView((Map) indices.get(".security-7")).get( + "migration_version.version" + ); + assertNotNull(responseVersion); + assertTrue(Integer.parseInt(responseVersion) >= version); + }); + } } diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRoleMappingCleanupIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRoleMappingCleanupIT.java new file mode 100644 index 0000000000000..82d4050c044b1 --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRoleMappingCleanupIT.java @@ -0,0 +1,146 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.upgrades; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import static org.elasticsearch.TransportVersions.V_8_15_0; +import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.containsInAnyOrder; + +public class SecurityIndexRoleMappingCleanupIT extends AbstractUpgradeTestCase { + + public void testCleanupDuplicateMappings() throws Exception { + if (CLUSTER_TYPE == ClusterType.OLD) { + // If we're in a state where the same operator-defined role mappings can exist both in cluster state and the native store + // (V_8_15_0 transport added to security.role_mapping_cleanup feature added), create a state + // where the native store will need to be cleaned up + assumeTrue( + "Cleanup only needed before security.role_mapping_cleanup feature available in cluster", + clusterHasFeature("security.role_mapping_cleanup") == false + ); + assumeTrue( + "If role mappings are in cluster state but cleanup has not been performed yet, create duplicated role mappings", + minimumTransportVersion().onOrAfter(V_8_15_0) + ); + // Since the old cluster has role mappings in cluster state, but doesn't check duplicates, create duplicates + createNativeRoleMapping("operator_role_mapping_1", Map.of("meta", "test"), true); + createNativeRoleMapping("operator_role_mapping_2", Map.of("meta", "test"), true); + } else if (CLUSTER_TYPE == ClusterType.MIXED) { + // Create a native role mapping that doesn't conflict with anything before the migration run + createNativeRoleMapping("no_name_conflict", Map.of("meta", "test")); + } else if (CLUSTER_TYPE == ClusterType.UPGRADED) { + waitForSecurityMigrationCompletion(adminClient(), 2); + assertAllRoleMappings( + client(), + "operator_role_mapping_1" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "operator_role_mapping_2" + ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_SUFFIX, + "no_name_conflict" + ); + // In the old cluster we might have created these (depending on the node features), so make sure they were removed + assertFalse(roleMappingExistsInSecurityIndex("operator_role_mapping_1")); + assertFalse(roleMappingExistsInSecurityIndex("operator_role_mapping_2")); + assertTrue(roleMappingExistsInSecurityIndex("no_name_conflict")); + // Make sure we can create and delete a conflicting role mapping again + createNativeRoleMapping("operator_role_mapping_1", Map.of("meta", "test"), true); + deleteNativeRoleMapping("operator_role_mapping_1", true); + } + } + + @SuppressWarnings("unchecked") + private boolean roleMappingExistsInSecurityIndex(String mappingName) throws IOException { + final Request request = new Request("POST", "/.security/_search"); + request.setJsonEntity(String.format(Locale.ROOT, """ + {"query":{"bool":{"must":[{"term":{"_id":"%s_%s"}}]}}}""", "role-mapping", mappingName)); + + request.setOptions( + expectWarnings( + "this request accesses system indices: [.security-7]," + + " but in a future major version, direct access to system indices will be prevented by default" + ) + ); + + Response response = adminClient().performRequest(request); + assertOK(response); + final Map responseMap = responseAsMap(response); + + Map hits = ((Map) responseMap.get("hits")); + return ((List) hits.get("hits")).isEmpty() == false; + } + + private void createNativeRoleMapping(String roleMappingName, Map metadata) throws IOException { + createNativeRoleMapping(roleMappingName, metadata, false); + } + + private void createNativeRoleMapping(String roleMappingName, Map metadata, boolean expectWarning) throws IOException { + final Request request = new Request("POST", "/_security/role_mapping/" + roleMappingName); + if (expectWarning) { + request.setOptions( + expectWarnings( + "A read-only role mapping with the same name [" + + roleMappingName + + "] has been previously defined in a configuration file. " + + "Both role mappings will be used to determine role assignments." + ) + ); + } + + BytesReference source = BytesReference.bytes( + jsonBuilder().map( + Map.of( + ExpressionRoleMapping.Fields.ROLES.getPreferredName(), + List.of("superuser"), + ExpressionRoleMapping.Fields.ENABLED.getPreferredName(), + true, + ExpressionRoleMapping.Fields.RULES.getPreferredName(), + Map.of("field", Map.of("username", "role-mapping-test-user")), + RoleDescriptor.Fields.METADATA.getPreferredName(), + metadata + ) + ) + ); + request.setJsonEntity(source.utf8ToString()); + assertOK(client().performRequest(request)); + } + + private void deleteNativeRoleMapping(String roleMappingName, boolean expectWarning) throws IOException { + final Request request = new Request("DELETE", "/_security/role_mapping/" + roleMappingName); + if (expectWarning) { + request.setOptions( + expectWarnings( + "A read-only role mapping with the same name [" + + roleMappingName + + "] has previously been defined in a configuration file. " + + "The native role mapping was deleted, but the read-only mapping will remain active " + + "and will be used to determine role assignments." + ) + ); + } + assertOK(client().performRequest(request)); + } + + private void assertAllRoleMappings(RestClient client, String... roleNames) throws IOException { + Request request = new Request("GET", "/_security/role_mapping"); + Response response = client.performRequest(request); + assertOK(response); + Map responseMap = responseAsMap(response); + + assertThat(responseMap.keySet(), containsInAnyOrder(roleNames)); + assertThat(responseMap.size(), is(roleNames.length)); + } +} diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRolesMetadataMigrationIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRolesMetadataMigrationIT.java index d31130e970f03..6c34e68297aa0 100644 --- a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRolesMetadataMigrationIT.java +++ b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SecurityIndexRolesMetadataMigrationIT.java @@ -58,7 +58,7 @@ public void testRoleMigration() throws Exception { } else if (CLUSTER_TYPE == ClusterType.UPGRADED) { createRoleWithMetadata(upgradedTestRole, Map.of("meta", "test")); assertTrue(canRolesBeMigrated()); - waitForMigrationCompletion(adminClient()); + waitForSecurityMigrationCompletion(adminClient(), 1); assertMigratedDocInSecurityIndex(oldTestRole, "meta", "test"); assertMigratedDocInSecurityIndex(mixed1TestRole, "meta", "test"); assertMigratedDocInSecurityIndex(mixed2TestRole, "meta", "test"); @@ -136,23 +136,6 @@ private static void assertNoMigration(RestClient adminClient) throws Exception { ); } - @SuppressWarnings("unchecked") - private static void waitForMigrationCompletion(RestClient adminClient) throws Exception { - final Request request = new Request("GET", "_cluster/state/metadata/" + INTERNAL_SECURITY_MAIN_INDEX_7); - assertBusy(() -> { - Response response = adminClient.performRequest(request); - assertOK(response); - Map responseMap = responseAsMap(response); - Map indicesMetadataMap = (Map) ((Map) responseMap.get("metadata")).get( - "indices" - ); - assertTrue(indicesMetadataMap.containsKey(INTERNAL_SECURITY_MAIN_INDEX_7)); - assertTrue( - ((Map) indicesMetadataMap.get(INTERNAL_SECURITY_MAIN_INDEX_7)).containsKey(MIGRATION_VERSION_CUSTOM_KEY) - ); - }); - } - private void createRoleWithMetadata(String roleName, Map metadata) throws IOException { final Request request = new Request("POST", "/_security/role/" + roleName); BytesReference source = BytesReference.bytes( diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/operator_defined_role_mappings.json b/x-pack/qa/rolling-upgrade/src/test/resources/operator_defined_role_mappings.json new file mode 100644 index 0000000000000..d897cabb8ab01 --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/operator_defined_role_mappings.json @@ -0,0 +1,38 @@ +{ + "metadata": { + "version": "2", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "operator_role_mapping_1": { + "enabled": true, + "roles": [ + "kibana_user" + ], + "metadata": { + "from_file": true + }, + "rules": { + "field": { + "username": "role-mapping-test-user" + } + } + }, + "operator_role_mapping_2": { + "enabled": true, + "roles": [ + "fleet_user" + ], + "metadata": { + "from_file": true + }, + "rules": { + "field": { + "username": "role-mapping-test-user" + } + } + } + } + } +}