Add Role Mapping Cleanup Security Migration

docs/changelog/114830.yaml

@@ -0,0 +1,5 @@
+pr: 114830
+summary: Add Role Mapping Cleanup Security Migration
+area: Security
+type: bug
+issues: []

server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java

@@ -91,6 +91,21 @@ public Set<String> conflicts(String handlerName, Set<String> 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<String> 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}
      *

server/src/main/java/org/elasticsearch/index/IndexVersions.java

@@ -117,6 +117,7 @@ private static IndexVersion def(int id, Version luceneVersion) {
     public static final IndexVersion ENABLE_IGNORE_MALFORMED_LOGSDB = def(8_514_00_0, Version.LUCENE_9_11_1);
     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 ADD_ROLE_MAPPING_CLEANUP_MIGRATION = def(8_517_00_0, Version.LUCENE_9_12_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/ExpressionRoleMapping.java

@@ -206,15 +206,15 @@ public RoleMapperExpression getExpression() {
      * that match the {@link #getExpression() expression} in this mapping.
      */
     public List<String> getRoles() {
-        return Collections.unmodifiableList(roles);
+        return roles != null ? Collections.unmodifiableList(roles) : Collections.emptyList();
     }
 
     /**
      * The list of {@link RoleDescriptor roles} (specified by a {@link TemplateRoleName template} that evaluates to one or more names)
      * that should be assigned to users that match the {@link #getExpression() expression} in this mapping.
      */
     public List<TemplateRoleName> getRoleTemplates() {
-        return Collections.unmodifiableList(roleTemplates);
+        return roleTemplates != null ? Collections.unmodifiableList(roleTemplates) : Collections.emptyList();
     }
 
     /**
@@ -223,7 +223,7 @@ public List<TemplateRoleName> getRoleTemplates() {
      * This is not used within the mapping process, and does not affect whether the expression matches, nor which roles are assigned.
      */
     public Map<String, Object> getMetadata() {
-        return Collections.unmodifiableMap(metadata);
+        return metadata != null ? Collections.unmodifiableMap(metadata) : Collections.emptyMap();
     }
 
     /**

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<NodeFeature> 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

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;
@@ -74,7 +77,7 @@
 public class SecurityIndexManager implements ClusterStateListener {
 
     public static final String SECURITY_VERSION_STRING = "security-version";
-
+    private static final String FILE_SETTINGS_METADATA_NAMESPACE = "file_settings";
     private static final Logger logger = LogManager.getLogger(SecurityIndexManager.class);
 
     /**
@@ -267,6 +270,21 @@ private static boolean isCreatedOnLatestVersion(IndexMetadata indexMetadata) {
         return indexVersionCreated != null && indexVersionCreated.onOrAfter(IndexVersion.current());
     }
 
+    /**
+     * Check to see if there are any file based role mappings and if they have been loaded into cluster state. If the
+     * {@link ReservedStateMetadata} file_settings namespace contains role mapping names, it means that there should be the same number of
+     * cluster state role mappings available. If they're not available yet using {@code RoleMappingMetadata.getFromClusterState()}, they
+     * have not yet been synchronized.
+     */
+    private static boolean isReadyForRoleMappingCleanupMigration(ClusterState clusterState) {
+        ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FILE_SETTINGS_METADATA_NAMESPACE);
+        assert fileSettingsMetadata != null || clusterState.metadata().reservedStateMetadata().isEmpty();
+        return fileSettingsMetadata == null
+            || RoleMappingMetadata.getFromClusterState(clusterState).getRoleMappings().size() == fileSettingsMetadata.keys(
+                ReservedRoleMappingAction.NAME
+            ).size();
+    }
+
     @Override
     public void clusterChanged(ClusterChangedEvent event) {
         if (event.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) {
@@ -284,6 +302,7 @@ public void clusterChanged(ClusterChangedEvent event) {
         Tuple<Boolean, Boolean> available = checkIndexAvailable(event.state());
         final boolean indexAvailableForWrite = available.v1();
         final boolean indexAvailableForSearch = available.v2();
+        final boolean readyForRoleMappingCleanupMigration = isReadyForRoleMappingCleanupMigration(event.state());
         final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state());
         final int migrationsVersion = getMigrationVersionFromIndexMetadata(indexMetadata);
         final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(event.state());
@@ -314,6 +333,7 @@ public void clusterChanged(ClusterChangedEvent event) {
             indexAvailableForWrite,
             mappingIsUpToDate,
             createdOnLatestVersion,
+            readyForRoleMappingCleanupMigration,
             migrationsVersion,
             minClusterMappingVersion,
             indexMappingVersion,
@@ -438,7 +458,8 @@ private Tuple<Boolean, Boolean> 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) {
@@ -671,6 +692,7 @@ public static class State {
             false,
             false,
             false,
+            false,
             null,
             null,
             null,
@@ -686,6 +708,7 @@ public static class State {
         public final boolean indexAvailableForWrite;
         public final boolean mappingUpToDate;
         public final boolean createdOnLatestVersion;
+        public final boolean readyForRoleMappingCleanupMigration;
         public final Integer migrationsVersion;
         // Min mapping version supported by the descriptors in the cluster
         public final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion;
@@ -704,6 +727,7 @@ public State(
             boolean indexAvailableForWrite,
             boolean mappingUpToDate,
             boolean createdOnLatestVersion,
+            boolean readyForRoleMappingCleanupMigration,
             Integer migrationsVersion,
             SystemIndexDescriptor.MappingsVersion minClusterMappingVersion,
             Integer indexMappingVersion,
@@ -720,6 +744,7 @@ public State(
             this.mappingUpToDate = mappingUpToDate;
             this.migrationsVersion = migrationsVersion;
             this.createdOnLatestVersion = createdOnLatestVersion;
+            this.readyForRoleMappingCleanupMigration = readyForRoleMappingCleanupMigration;
             this.minClusterMappingVersion = minClusterMappingVersion;
             this.indexMappingVersion = indexMappingVersion;
             this.concreteIndexName = concreteIndexName;
@@ -740,6 +765,7 @@ public boolean equals(Object o) {
                 && indexAvailableForWrite == state.indexAvailableForWrite
                 && mappingUpToDate == state.mappingUpToDate
                 && createdOnLatestVersion == state.createdOnLatestVersion
+                && readyForRoleMappingCleanupMigration == state.readyForRoleMappingCleanupMigration
                 && Objects.equals(indexMappingVersion, state.indexMappingVersion)
                 && Objects.equals(migrationsVersion, state.migrationsVersion)
                 && Objects.equals(minClusterMappingVersion, state.minClusterMappingVersion)
@@ -762,6 +788,7 @@ public int hashCode() {
                 indexAvailableForWrite,
                 mappingUpToDate,
                 createdOnLatestVersion,
+                readyForRoleMappingCleanupMigration,
                 migrationsVersion,
                 minClusterMappingVersion,
                 indexMappingVersion,
@@ -786,6 +813,8 @@ public String toString() {
                 + mappingUpToDate
                 + ", createdOnLatestVersion="
                 + createdOnLatestVersion
+                + ", readyForRoleMappingCleanupMigration="
+                + readyForRoleMappingCleanupMigration
                 + ", migrationsVersion="
                 + migrationsVersion
                 + ", minClusterMappingVersion="