Clearer error on modifying read-only role mappings (#115951) (#115962)

Copy of: #115509 also due
to temporary repo unavailability.

That PR is already approved.

(cherry picked from commit e543e82)

Co-authored-by: Elastic Machine <[email protected]>

Author: n1v0lg

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

@@ -26,6 +26,7 @@
 import java.util.Objects;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
+import static org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG;
 
 /**
  * Request object for adding/updating a role-mapping to the native store
@@ -77,10 +78,19 @@ public ActionRequestValidationException validate(boolean validateMetadata) {
             validationException = addValidationError("role-mapping rules are missing", validationException);
         }
         if (validateMetadata && MetadataUtils.containsReservedMetadata(metadata)) {
-            validationException = addValidationError(
-                "metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]",
-                validationException
-            );
+            if (metadata.containsKey(READ_ONLY_ROLE_MAPPING_METADATA_FLAG)) {
+                validationException = addValidationError(
+                    "metadata contains ["
+                        + READ_ONLY_ROLE_MAPPING_METADATA_FLAG
+                        + "] flag. You cannot create or update role-mappings with a read-only flag",
+                    validationException
+                );
+            } else {
+                validationException = addValidationError(
+                    "metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]",
+                    validationException
+                );
+            }
         }
         return validationException;
     }

x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/rolemapping/RoleMappingRestIT.java

@@ -150,6 +150,32 @@ public void testPutAndDeleteRoleMappings() throws IOException {
             );
         }
 
+        // simulate attempt to update a CS role mapping (the request will include a _read_only metadata flag
+        {
+            var ex = expectThrows(
+                ResponseException.class,
+                () -> putMapping(expressionRoleMapping("role-mapping-1-read-only-operator-mapping", Map.of("_read_only", true)))
+            );
+            assertThat(
+                ex.getMessage(),
+                containsString("metadata contains [_read_only] flag. You cannot create or update role-mappings with a read-only flag")
+            );
+        }
+
+        {
+            var ex = expectThrows(
+                ResponseException.class,
+                () -> putMapping(expressionRoleMapping("role-mapping-1-read-only-operator-mapping"))
+            );
+            assertThat(
+                ex.getMessage(),
+                containsString(
+                    "Invalid mapping name [role-mapping-1-read-only-operator-mapping]. "
+                        + "[-read-only-operator-mapping] is not an allowed suffix"
+                )
+            );
+        }
+
         // Also fails even if a CS role mapping with that name does not exist
         {
             var ex = expectThrows(
@@ -209,12 +235,16 @@ private static Response deleteMapping(String name, @Nullable String warning) thr
     }
 
     private static ExpressionRoleMapping expressionRoleMapping(String name) {
+        return expressionRoleMapping(name, Map.of());
+    }
+
+    private static ExpressionRoleMapping expressionRoleMapping(String name, Map<String, Object> metadata) {
         return new ExpressionRoleMapping(
             name,
             new FieldExpression("username", List.of(new FieldExpression.FieldValue(randomAlphaOfLength(10)))),
             List.of(randomAlphaOfLength(5)),
             null,
-            Map.of(),
+            metadata,
             true
         );
     }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/PutRoleMappingRequestTests.java

@@ -11,14 +11,19 @@
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest;
 import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequestBuilder;
+import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping;
 import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.RoleMapperExpression;
 import org.junit.Before;
 import org.mockito.Mockito;
 
 import java.util.Collections;
+import java.util.Map;
 
+import static org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 
 public class PutRoleMappingRequestTests extends ESTestCase {
 
@@ -54,6 +59,62 @@ public void testValidateMetadataKeys() throws Exception {
         assertValidationFailure(request, "metadata key");
     }
 
+    public void testValidateReadyOnlyMetadataKey() {
+        assertValidationFailure(
+            builder.name("test")
+                .roles("superuser")
+                .expression(Mockito.mock(RoleMapperExpression.class))
+                .metadata(Map.of("_secret", false, ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG, true))
+                .request(),
+            "metadata contains ["
+                + READ_ONLY_ROLE_MAPPING_METADATA_FLAG
+                + "] flag. You cannot create or update role-mappings with a read-only flag"
+        );
+
+        assertValidationFailure(
+            builder.name("test")
+                .roles("superuser")
+                .expression(Mockito.mock(RoleMapperExpression.class))
+                .metadata(Map.of(ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG, true))
+                .request(),
+            "metadata contains ["
+                + READ_ONLY_ROLE_MAPPING_METADATA_FLAG
+                + "] flag. You cannot create or update role-mappings with a read-only flag"
+        );
+    }
+
+    public void testValidateMetadataKeySkipped() {
+        assertThat(
+            builder.name("test")
+                .roles("superuser")
+                .expression(Mockito.mock(RoleMapperExpression.class))
+                .metadata(Map.of("_secret", false, ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG, true))
+                .request()
+                .validate(false),
+            is(nullValue())
+        );
+
+        assertThat(
+            builder.name("test")
+                .roles("superuser")
+                .expression(Mockito.mock(RoleMapperExpression.class))
+                .metadata(Map.of(ExpressionRoleMapping.READ_ONLY_ROLE_MAPPING_METADATA_FLAG, true))
+                .request()
+                .validate(false),
+            is(nullValue())
+        );
+
+        assertThat(
+            builder.name("test")
+                .roles("superuser")
+                .expression(Mockito.mock(RoleMapperExpression.class))
+                .metadata(Map.of("_secret", false))
+                .request()
+                .validate(false),
+            is(nullValue())
+        );
+    }
+
     private void assertValidationFailure(PutRoleMappingRequest request, String expectedMessage) {
         final ValidationException ve = request.validate();
         assertThat(ve, notNullValue());