Add name to role mappings in cluster state

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -239,6 +239,7 @@ static TransportVersion def(int id) {
     public static final TransportVersion TEXT_SIMILARITY_RERANKER_QUERY_REWRITE = def(8_763_00_0);
     public static final TransportVersion SIMULATE_INDEX_TEMPLATES_SUBSTITUTIONS = def(8_764_00_0);
     public static final TransportVersion RETRIEVERS_TELEMETRY_ADDED = def(8_765_00_0);
+    public static final TransportVersion ADD_VERSION_TO_ROLE_MAPPING_METADATA = def(8_766_00_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

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

@@ -16,6 +16,7 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.xcontent.ObjectParser;
 import org.elasticsearch.xcontent.ObjectParser.ValueType;
@@ -68,6 +69,7 @@ public class ExpressionRoleMapping implements ToXContentObject, Writeable {
         PARSER.declareField(Builder::rules, ExpressionParser::parseObject, Fields.RULES, ValueType.OBJECT);
         PARSER.declareField(Builder::metadata, XContentParser::map, Fields.METADATA, ValueType.OBJECT);
         PARSER.declareBoolean(Builder::enabled, Fields.ENABLED);
+        PARSER.declareString(Builder::name, Fields.NAME);
         BiConsumer<Builder, String> ignored = (b, v) -> {};
         // skip the doc_type and type fields in case we're parsing directly from the index
         PARSER.declareString(ignored, new ParseField(NativeRoleMappingStoreField.DOC_TYPE_FIELD));
@@ -241,13 +243,16 @@ public static ExpressionRoleMapping parse(String name, BytesReference source, XC
         }
     }
 
+    public static ExpressionRoleMapping parse(XContentParser parser) throws IOException {
+        return parse(null, parser);
+    }
+
     /**
      * Parse an {@link ExpressionRoleMapping} from the provided <em>XContent</em>
      */
     public static ExpressionRoleMapping parse(String name, XContentParser parser) throws IOException {
         try {
-            final Builder builder = PARSER.parse(parser, name);
-            return builder.build(name);
+            return PARSER.parse(parser, name).build();
         } catch (IllegalArgumentException | IllegalStateException e) {
             throw new ParsingException(parser.getTokenLocation(), e.getMessage(), e);
         }
@@ -264,6 +269,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
 
     public XContentBuilder toXContent(XContentBuilder builder, Params params, boolean indexFormat) throws IOException {
         builder.startObject();
+        return toXContent(builder, params, indexFormat, null).endObject();
+    }
+
+    public XContentBuilder toXContent(XContentBuilder builder, Params params, boolean indexFormat, @Nullable String name)
+        throws IOException {
+        if (name != null) {
+            builder.field(Fields.NAME.getPreferredName(), name);
+        }
         builder.field(Fields.ENABLED.getPreferredName(), enabled);
         if (roles.isEmpty() == false) {
             builder.startArray(Fields.ROLES.getPreferredName());
@@ -287,7 +300,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params, boolea
         if (indexFormat) {
             builder.field(NativeRoleMappingStoreField.DOC_TYPE_FIELD, NativeRoleMappingStoreField.DOC_TYPE_ROLE_MAPPING);
         }
-        return builder.endObject();
+        return builder;
     }
 
     public Set<String> getRoleNames(ScriptService scriptService, ExpressionModel model) {
@@ -299,12 +312,18 @@ public Set<String> getRoleNames(ScriptService scriptService, ExpressionModel mod
      * Used to facilitate the use of {@link ObjectParser} (via {@link #PARSER}).
      */
     private static class Builder {
+        private String name;
         private RoleMapperExpression rules;
         private List<String> roles;
         private List<TemplateRoleName> roleTemplates;
         private Map<String, Object> metadata = Collections.emptyMap();
         private Boolean enabled;
 
+        Builder name(String name) {
+            this.name = name;
+            return this;
+        }
+
         Builder rules(RoleMapperExpression expression) {
             this.rules = expression;
             return this;
@@ -330,6 +349,10 @@ Builder enabled(boolean enabled) {
             return this;
         }
 
+        private ExpressionRoleMapping build() {
+            return build(name);
+        }
+
         private ExpressionRoleMapping build(String name) {
             if (roles == null && roleTemplates == null) {
                 throw missingField(name, Fields.ROLES);
@@ -349,6 +372,7 @@ private static IllegalStateException missingField(String id, ParseField field) {
     }
 
     public interface Fields {
+        ParseField NAME = new ParseField("name");
         ParseField ROLES = new ParseField("roles");
         ParseField ROLE_TEMPLATES = new ParseField("role_templates");
         ParseField ENABLED = new ParseField("enabled");

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleMappingMetadata.java

@@ -31,43 +31,45 @@
 import java.util.Objects;
 import java.util.Set;
 
+import static org.elasticsearch.TransportVersions.ADD_VERSION_TO_ROLE_MAPPING_METADATA;
 import static org.elasticsearch.cluster.metadata.Metadata.ALL_CONTEXTS;
 import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
 
 public final class RoleMappingMetadata extends AbstractNamedDiffable<Metadata.Custom> implements Metadata.Custom {
 
     public static final String TYPE = "role_mappings";
+    public static final String VERSION_FIELD = "version";
+    private static final int ROLE_MAPPING_METADATA_VERSION_SERIALIZE_NAME = 1;
 
     @SuppressWarnings("unchecked")
     private static final ConstructingObjectParser<RoleMappingMetadata, Void> PARSER = new ConstructingObjectParser<>(
         TYPE,
         // serialization tests rely on the order of the ExpressionRoleMapping
-        args -> new RoleMappingMetadata(new LinkedHashSet<>((Collection<ExpressionRoleMapping>) args[0]))
+        args -> new RoleMappingMetadata(new LinkedHashSet<>((Collection<ExpressionRoleMapping>) args[0]), (int) args[1])
     );
 
     static {
-        PARSER.declareObjectArray(
-            constructorArg(),
-            // role mapping names are lost when the role mapping metadata is serialized
-            (p, c) -> ExpressionRoleMapping.parse("name_not_available_after_deserialization", p),
-            new ParseField(TYPE)
-        );
+        PARSER.declareObjectArray(constructorArg(), (p, c) -> ExpressionRoleMapping.parse(p), new ParseField(TYPE));
+        PARSER.declareIntOrNull(constructorArg(), 0, new ParseField(VERSION_FIELD));
     }
 
-    private static final RoleMappingMetadata EMPTY = new RoleMappingMetadata(Set.of());
+    private static final RoleMappingMetadata EMPTY = new RoleMappingMetadata(Set.of(), 0);
 
     public static RoleMappingMetadata getFromClusterState(ClusterState clusterState) {
         return clusterState.metadata().custom(RoleMappingMetadata.TYPE, RoleMappingMetadata.EMPTY);
     }
 
     private final Set<ExpressionRoleMapping> roleMappings;
+    private final Integer version;
 
-    public RoleMappingMetadata(Set<ExpressionRoleMapping> roleMappings) {
+    public RoleMappingMetadata(Set<ExpressionRoleMapping> roleMappings, int version) {
         this.roleMappings = roleMappings;
+        this.version = version;
     }
 
     public RoleMappingMetadata(StreamInput input) throws IOException {
         this.roleMappings = input.readCollectionAsSet(ExpressionRoleMapping::new);
+        this.version = input.readOptionalInt();
     }
 
     public Set<ExpressionRoleMapping> getRoleMappings() {
@@ -94,7 +96,24 @@ public static NamedDiff<Metadata.Custom> readDiffFrom(StreamInput streamInput) t
     @Override
     public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params) {
         // role mappings are serialized without their names
-        return Iterators.concat(ChunkedToXContentHelper.startArray(TYPE), roleMappings.iterator(), ChunkedToXContentHelper.endArray());
+        return Iterators.concat(ChunkedToXContentHelper.startArray(TYPE), innerToXContent(), ChunkedToXContentHelper.endArray());
+    }
+
+    private Iterator<? extends ToXContent> innerToXContent() {
+        return roleMappings.stream()
+            .map(
+                roleMapping -> (ToXContent) (builder, params) -> roleMapping.toXContent(
+                    builder,
+                    params,
+                    false,
+                    shouldSerializeName() ? roleMapping.getName() : null
+                )
+            )
+            .iterator();
+    }
+
+    private boolean shouldSerializeName() {
+        return this.version >= ROLE_MAPPING_METADATA_VERSION_SERIALIZE_NAME;
     }
 
     public static RoleMappingMetadata fromXContent(XContentParser parser) throws IOException {
@@ -114,19 +133,22 @@ public TransportVersion getMinimalSupportedVersion() {
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeCollection(roleMappings);
+        if (out.getTransportVersion().onOrAfter(ADD_VERSION_TO_ROLE_MAPPING_METADATA)) {
+            out.writeInt(version);
+        }
     }
 
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         final var other = (RoleMappingMetadata) o;
-        return Objects.equals(roleMappings, other.roleMappings);
+        return Objects.equals(roleMappings, other.roleMappings) && Objects.equals(version, other.version);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(roleMappings);
+        return Objects.hash(roleMappings, version);
     }
 
     @Override
@@ -141,7 +163,7 @@ public String toString() {
             builder.append(entryList.next().toString());
             firstEntry = false;
         }
-        return builder.append("]]").toString();
+        return builder.append("], version=[").append(version).append("]]").toString();
     }
 
     @Override

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java

@@ -125,15 +125,15 @@ public void testReservedStatePersistsOnRestart() throws Exception {
             roleMappings,
             containsInAnyOrder(
                 new ExpressionRoleMapping(
-                    "name_not_available_after_deserialization",
+                    "everyone_kibana_alone",
                     new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
                     List.of("kibana_user"),
                     List.of(),
                     Map.of("uuid", "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", "_foo", "something"),
                     true
                 ),
                 new ExpressionRoleMapping(
-                    "name_not_available_after_deserialization",
+                    "everyone_fleet_alone",
                     new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
                     List.of("fleet_user"),
                     List.of(),

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -1203,7 +1203,7 @@ Collection<Object> createComponents(
             new SecurityUsageServices(realms, allRolesStore, nativeRoleMappingStore, ipFilter.get(), profileService, apiKeyService)
         );
 
-        reservedRoleMappingAction.set(new ReservedRoleMappingAction());
+        reservedRoleMappingAction.set(new ReservedRoleMappingAction(featureService));
 
         cacheInvalidatorRegistry.validate();
 

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_NAME_FIELD;
 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_NAME_FIELD, SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK);
     }
 
     @Override

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java

@@ -8,6 +8,7 @@
 package org.elasticsearch.xpack.security.action.rolemapping;
 
 import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.features.FeatureService;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
 import org.elasticsearch.xcontent.XContentParser;
@@ -25,6 +26,7 @@
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.XContentHelper.mapToXContentParser;
+import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLE_MAPPING_NAME_FIELD;
 
 /**
  * This Action is the reserved state save version of RestPutRoleMappingAction/RestDeleteRoleMappingAction
@@ -34,6 +36,11 @@
  */
 public class ReservedRoleMappingAction implements ReservedClusterStateHandler<List<PutRoleMappingRequest>> {
     public static final String NAME = "role_mappings";
+    private final FeatureService featureService;
+
+    public ReservedRoleMappingAction(FeatureService featureService) {
+        this.featureService = featureService;
+    }
 
     @Override
     public String name() {
@@ -44,7 +51,12 @@ public String name() {
     public TransformState transform(Object source, TransformState prevState) throws Exception {
         @SuppressWarnings("unchecked")
         Set<ExpressionRoleMapping> roleMappings = validate((List<PutRoleMappingRequest>) source);
-        RoleMappingMetadata newRoleMappingMetadata = new RoleMappingMetadata(roleMappings);
+        // RoleMappingMetadata is written to cluster state, so make sure all nodes can parse the new format (with name), if they
+        // can't, write the old format (exclude name)
+        RoleMappingMetadata newRoleMappingMetadata = featureService.clusterHasFeature(prevState.state(), SECURITY_ROLE_MAPPING_NAME_FIELD)
+            ? new RoleMappingMetadata(roleMappings, )
+            : new RoleMappingMetadata(roleMappings);
+
         if (newRoleMappingMetadata.equals(RoleMappingMetadata.getFromClusterState(prevState.state()))) {
             return prevState;
         } else {

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_NAME_FIELD = new NodeFeature("security.role_mapping_name_field");
 
     /**
      * Security managed index mappings used to be updated based on the product version. They are now updated based on per-index mappings

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.features.FeatureService;
 import org.elasticsearch.reservedstate.TransformState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.XContentParser;
@@ -21,6 +22,7 @@
 
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
+import static org.mockito.Mockito.mock;
 
 /**
  * Tests that the ReservedRoleMappingAction does validation, can add and remove role mappings
@@ -37,7 +39,7 @@ private TransformState processJSON(ReservedRoleMappingAction action, TransformSt
     public void testValidation() {
         ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build();
         TransformState prevState = new TransformState(state, Collections.emptySet());
-        ReservedRoleMappingAction action = new ReservedRoleMappingAction();
+        ReservedRoleMappingAction action = new ReservedRoleMappingAction(mock(FeatureService.class));
         String badPolicyJSON = """
             {
                "everyone_kibana": {
@@ -66,7 +68,7 @@ public void testValidation() {
     public void testAddRemoveRoleMapping() throws Exception {
         ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build();
         TransformState prevState = new TransformState(state, Collections.emptySet());
-        ReservedRoleMappingAction action = new ReservedRoleMappingAction();
+        ReservedRoleMappingAction action = new ReservedRoleMappingAction(mock(FeatureService.class));
         String emptyJSON = "";
 
         TransformState updatedState = processJSON(action, prevState, emptyJSON);