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/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_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

@@ -172,15 +172,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();
     }
 
     /**
@@ -189,7 +189,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,6 +47,7 @@
 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 java.time.Instant;
@@ -74,7 +76,8 @@
 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 String HANDLER_ROLE_MAPPINGS_NAME = "role_mappings";
     private static final Logger logger = LogManager.getLogger(SecurityIndexManager.class);
 
     /**
@@ -267,6 +270,17 @@ private static boolean isCreatedOnLatestVersion(IndexMetadata indexMetadata) {
         return indexVersionCreated != null && indexVersionCreated.onOrAfter(IndexVersion.current());
     }
 
+    private static boolean isReservedRoleMappingsSynced(ClusterState clusterState) {
+        ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FILE_SETTINGS_METADATA_NAMESPACE);
+        if (fileSettingsMetadata != null && fileSettingsMetadata.handlers().containsKey(HANDLER_ROLE_MAPPINGS_NAME)) {
+            int fileSettingsMetadataSize = fileSettingsMetadata.handlers().get(HANDLER_ROLE_MAPPINGS_NAME).keys().size();
+            if (fileSettingsMetadataSize > 0) {
+                return fileSettingsMetadataSize == RoleMappingMetadata.getFromClusterState(clusterState).getRoleMappings().size();
+            }
+        }
+        return true;
+    }
+
     @Override
     public void clusterChanged(ClusterChangedEvent event) {
         if (event.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) {
@@ -284,6 +298,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 reservedRoleMappingsSynced = isReservedRoleMappingsSynced(event.state());
         final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state());
         final int migrationsVersion = getMigrationVersionFromIndexMetadata(indexMetadata);
         final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(event.state());
@@ -314,6 +329,7 @@ public void clusterChanged(ClusterChangedEvent event) {
             indexAvailableForWrite,
             mappingIsUpToDate,
             createdOnLatestVersion,
+            reservedRoleMappingsSynced,
             migrationsVersion,
             minClusterMappingVersion,
             indexMappingVersion,
@@ -438,7 +454,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 +688,7 @@ public static class State {
             false,
             false,
             false,
+            false,
             null,
             null,
             null,
@@ -686,6 +704,7 @@ public static class State {
         public final boolean indexAvailableForWrite;
         public final boolean mappingUpToDate;
         public final boolean createdOnLatestVersion;
+        public final boolean reservedRoleMappingsSynced;
         public final Integer migrationsVersion;
         // Min mapping version supported by the descriptors in the cluster
         public final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion;
@@ -704,6 +723,7 @@ public State(
             boolean indexAvailableForWrite,
             boolean mappingUpToDate,
             boolean createdOnLatestVersion,
+            boolean reservedRoleMappingsSynced,
             Integer migrationsVersion,
             SystemIndexDescriptor.MappingsVersion minClusterMappingVersion,
             Integer indexMappingVersion,
@@ -720,6 +740,7 @@ public State(
             this.mappingUpToDate = mappingUpToDate;
             this.migrationsVersion = migrationsVersion;
             this.createdOnLatestVersion = createdOnLatestVersion;
+            this.reservedRoleMappingsSynced = reservedRoleMappingsSynced;
             this.minClusterMappingVersion = minClusterMappingVersion;
             this.indexMappingVersion = indexMappingVersion;
             this.concreteIndexName = concreteIndexName;
@@ -740,6 +761,7 @@ public boolean equals(Object o) {
                 && indexAvailableForWrite == state.indexAvailableForWrite
                 && mappingUpToDate == state.mappingUpToDate
                 && createdOnLatestVersion == state.createdOnLatestVersion
+                && reservedRoleMappingsSynced == state.reservedRoleMappingsSynced
                 && Objects.equals(indexMappingVersion, state.indexMappingVersion)
                 && Objects.equals(migrationsVersion, state.migrationsVersion)
                 && Objects.equals(minClusterMappingVersion, state.minClusterMappingVersion)
@@ -762,6 +784,7 @@ public int hashCode() {
                 indexAvailableForWrite,
                 mappingUpToDate,
                 createdOnLatestVersion,
+                reservedRoleMappingsSynced,
                 migrationsVersion,
                 minClusterMappingVersion,
                 indexMappingVersion,
@@ -786,6 +809,8 @@ public String toString() {
                 + mappingUpToDate
                 + ", createdOnLatestVersion="
                 + createdOnLatestVersion
+                + ", reservedRoleMappingsSynced="
+                + reservedRoleMappingsSynced
                 + ", migrationsVersion="
                 + migrationsVersion
                 + ", minClusterMappingVersion="