Do not fetch reserved roles from native store when Get Role API is called

docs/changelog/121971.yaml

@@ -0,0 +1,5 @@
+pr: 121971
+summary: Do not fetch reserved roles from native store when Get Role API is called
+area: Authorization
+type: enhancement
+issues: []

test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java

@@ -46,6 +46,7 @@
 import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -567,6 +568,37 @@ private void writeSecureSecretsFile() {
             }
         }
 
+        private void updateRolesFileAtomically() throws IOException {
+            final Path targetRolesFile = workingDir.resolve("config").resolve("roles.yml");
+            final Path tempFile = Files.createTempFile(workingDir.resolve("config"), null, null);
+
+            // collect all roles.yml files that should be combined into a single roles file
+            final List<Resource> rolesFiles = new ArrayList<>(spec.getRolesFiles().size() + 1);
+            rolesFiles.add(Resource.fromFile(distributionDir.resolve("config").resolve("roles.yml")));
+            rolesFiles.addAll(spec.getRolesFiles());
+
+            // append all roles files to the temp file
+            rolesFiles.forEach(rolesFile -> {
+                try (
+                    Writer writer = Files.newBufferedWriter(tempFile, StandardOpenOption.APPEND);
+                    Reader reader = new BufferedReader(new InputStreamReader(rolesFile.asStream()))
+                ) {
+                    reader.transferTo(writer);
+                } catch (IOException e) {
+                    throw new UncheckedIOException("Failed to append roles file " + rolesFile + " to " + tempFile, e);
+                }
+            });
+
+            // move the temp file to the target roles file atomically
+            try {
+                Files.move(tempFile, targetRolesFile, StandardCopyOption.ATOMIC_MOVE);
+            } catch (IOException e) {
+                throw new UncheckedIOException("Failed to move tmp roles file [" + tempFile + "] to [" + targetRolesFile + "]", e);
+            } finally {
+                Files.deleteIfExists(tempFile);
+            }
+        }
+
         private void configureSecurity() {
             if (spec.isSecurityEnabled()) {
                 if (spec.getUsers().isEmpty() == false) {
@@ -576,13 +608,11 @@ private void configureSecurity() {
                         if (resource instanceof MutableResource && roleFileListeners.add(resource)) {
                             ((MutableResource) resource).addUpdateListener(updated -> {
                                 LOGGER.info("Updating roles.yml for node '{}'", name);
-                                Path rolesFile = workingDir.resolve("config").resolve("roles.yml");
                                 try {
-                                    Files.delete(rolesFile);
-                                    Files.copy(distributionDir.resolve("config").resolve("roles.yml"), rolesFile);
-                                    writeRolesFile();
+                                    updateRolesFileAtomically();
+                                    LOGGER.info("Successfully updated roles.yml for node '{}'", name);
                                 } catch (IOException e) {
-                                    throw new UncheckedIOException(e);
+                                    throw new UncheckedIOException("Failed to update roles.yml file for node [" + name + "]", e);
                                 }
                             });
                         }

x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/GetRolesIT.java

@@ -0,0 +1,166 @@
+/*
+ * 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;
+
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
+import org.elasticsearch.client.RestClient;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.elasticsearch.test.cluster.local.distribution.DistributionType;
+import org.elasticsearch.test.cluster.local.model.User;
+import org.elasticsearch.test.cluster.util.resource.Resource;
+import org.elasticsearch.xcontent.ObjectPath;
+import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
+import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
+import org.junit.Before;
+import org.junit.ClassRule;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
+import static org.hamcrest.Matchers.equalTo;
+
+public class GetRolesIT extends SecurityInBasicRestTestCase {
+
+    private static final String ADMIN_USER = "admin_user";
+    private static final SecureString ADMIN_PASSWORD = new SecureString("admin-password".toCharArray());
+    protected static final String READ_SECURITY_USER = "read_security_user";
+    private static final SecureString READ_SECURITY_PASSWORD = new SecureString("read-security-password".toCharArray());
+
+    @Before
+    public void initialize() {
+        new ReservedRolesStore();
+    }
+
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
+        .distribution(DistributionType.DEFAULT)
+        .nodes(2)
+        .setting("xpack.security.enabled", "true")
+        .setting("xpack.license.self_generated.type", "basic")
+        .rolesFile(Resource.fromClasspath("roles.yml"))
+        .user(ADMIN_USER, ADMIN_PASSWORD.toString(), User.ROOT_USER_ROLE, true)
+        .user(READ_SECURITY_USER, READ_SECURITY_PASSWORD.toString(), "read_security_user_role", false)
+        .build();
+
+    @Override
+    protected Settings restAdminSettings() {
+        String token = basicAuthHeaderValue(ADMIN_USER, ADMIN_PASSWORD);
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build();
+    }
+
+    @Override
+    protected Settings restClientSettings() {
+        String token = basicAuthHeaderValue(READ_SECURITY_USER, READ_SECURITY_PASSWORD);
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build();
+    }
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
+    public void testGetAllRolesNoNative() throws Exception {
+        // Test get roles API with operator admin_user
+        getAllRolesAndAssert(adminClient(), ReservedRolesStore.names());
+        // Test get roles API with read_security_user
+        getAllRolesAndAssert(client(), ReservedRolesStore.names());
+    }
+
+    public void testGetAllRolesWithNative() throws Exception {
+        createRole("custom_role", "Test custom native role.", Map.of("owner", "test"));
+
+        Set<String> expectedRoles = new HashSet<>(ReservedRolesStore.names());
+        expectedRoles.add("custom_role");
+
+        // Test get roles API with operator admin_user
+        getAllRolesAndAssert(adminClient(), expectedRoles);
+        // Test get roles API with read_security_user
+        getAllRolesAndAssert(client(), expectedRoles);
+    }
+
+    public void testGetReservedOnly() throws Exception {
+        createRole("custom_role", "Test custom native role.", Map.of("owner", "test"));
+
+        Set<String> rolesToGet = new HashSet<>();
+        rolesToGet.add("custom_role");
+        rolesToGet.addAll(randomSet(1, 5, () -> randomFrom(ReservedRolesStore.names())));
+
+        getRolesAndAssert(adminClient(), rolesToGet);
+        getRolesAndAssert(client(), rolesToGet);
+    }
+
+    public void testGetNativeOnly() throws Exception {
+        createRole("custom_role1", "Test custom native role.", Map.of("owner", "test1"));
+        createRole("custom_role2", "Test custom native role.", Map.of("owner", "test2"));
+
+        Set<String> rolesToGet = Set.of("custom_role1", "custom_role2");
+
+        getRolesAndAssert(adminClient(), rolesToGet);
+        getRolesAndAssert(client(), rolesToGet);
+    }
+
+    public void testGetMixedRoles() throws Exception {
+        createRole("custom_role", "Test custom native role.", Map.of("owner", "test"));
+
+        Set<String> rolesToGet = new HashSet<>();
+        rolesToGet.add("custom_role");
+        rolesToGet.addAll(randomSet(1, 5, () -> randomFrom(ReservedRolesStore.names())));
+
+        getRolesAndAssert(adminClient(), rolesToGet);
+        getRolesAndAssert(client(), rolesToGet);
+    }
+
+    public void testNonExistentRole() {
+        var e = expectThrows(
+            ResponseException.class,
+            () -> client().performRequest(new Request("GET", "/_security/role/non_existent_role"))
+        );
+        assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(404));
+    }
+
+    private void createRole(String roleName, String description, Map<String, Object> metadata) throws IOException {
+        Request request = new Request("POST", "/_security/role/" + roleName);
+        Map<String, Object> requestMap = new HashMap<>();
+        if (description != null) {
+            requestMap.put(RoleDescriptor.Fields.DESCRIPTION.getPreferredName(), description);
+        }
+        if (metadata != null) {
+            requestMap.put(RoleDescriptor.Fields.METADATA.getPreferredName(), metadata);
+        }
+        BytesReference source = BytesReference.bytes(jsonBuilder().map(requestMap));
+        request.setJsonEntity(source.utf8ToString());
+        Response response = adminClient().performRequest(request);
+        assertOK(response);
+        Map<String, Object> responseMap = responseAsMap(response);
+        assertTrue(ObjectPath.eval("role.created", responseMap));
+    }
+
+    private void getAllRolesAndAssert(RestClient client, Set<String> expectedRoles) throws IOException {
+        final Response response = client.performRequest(new Request("GET", "/_security/role"));
+        assertOK(response);
+        final Map<String, Object> responseMap = responseAsMap(response);
+        assertThat(responseMap.keySet(), equalTo(expectedRoles));
+    }
+
+    private void getRolesAndAssert(RestClient client, Set<String> rolesToGet) throws IOException {
+        final Response response = client.performRequest(new Request("GET", "/_security/role/" + String.join(",", rolesToGet)));
+        assertOK(response);
+        final Map<String, Object> responseMap = responseAsMap(response);
+        assertThat(responseMap.keySet(), equalTo(rolesToGet));
+    }
+}

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/GeRoleDescriptorsTests.java

@@ -0,0 +1,103 @@
+/*
+ * 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.integration;
+
+import org.elasticsearch.action.support.PlainActionFuture;
+import org.elasticsearch.test.NativeRealmIntegTestCase;
+import org.elasticsearch.test.TestSecurityClient;
+import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
+import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
+import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult;
+import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
+import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
+import org.junit.Before;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.elasticsearch.test.SecuritySettingsSource.SECURITY_REQUEST_OPTIONS;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+/**
+ * Test for the {@link NativeRolesStore#getRoleDescriptors} method.
+ */
+public class GeRoleDescriptorsTests extends NativeRealmIntegTestCase {
+
+    private static Set<String> customRoles;
+
+    @BeforeClass
+    public static void init() throws Exception {
+        new ReservedRolesStore();
+
+        final int numOfRoles = randomIntBetween(5, 10);
+        customRoles = new HashSet<>(numOfRoles);
+        for (int i = 0; i < numOfRoles; i++) {
+            customRoles.add("custom_role_" + randomAlphaOfLength(10) + "_" + i);
+        }
+    }
+
+    @Before
+    public void setup() throws IOException {
+        final TestSecurityClient securityClient = new TestSecurityClient(getRestClient(), SECURITY_REQUEST_OPTIONS);
+        for (String role : customRoles) {
+            final RoleDescriptor descriptor = new RoleDescriptor(
+                role,
+                new String[0],
+                new RoleDescriptor.IndicesPrivileges[] {
+                    RoleDescriptor.IndicesPrivileges.builder()
+                        .indices("*")
+                        .privileges("ALL")
+                        .allowRestrictedIndices(randomBoolean())
+                        .build() },
+                new String[0]
+            );
+            securityClient.putRole(descriptor);
+            logger.info("--> created role [{}]", role);
+        }
+
+        ensureGreen(SecuritySystemIndices.SECURITY_MAIN_ALIAS);
+    }
+
+    public void testGetCustomRoles() {
+        for (NativeRolesStore rolesStore : internalCluster().getInstances(NativeRolesStore.class)) {
+            PlainActionFuture<RoleRetrievalResult> future = new PlainActionFuture<>();
+            rolesStore.getRoleDescriptors(customRoles, future);
+            RoleRetrievalResult result = future.actionGet();
+            assertThat(result, notNullValue());
+            assertTrue(result.isSuccess());
+            assertThat(result.getDescriptors().stream().map(RoleDescriptor::getName).toList(), containsInAnyOrder(customRoles.toArray()));
+        }
+    }
+
+    public void testGetReservedRoles() {
+        for (NativeRolesStore rolesStore : internalCluster().getInstances(NativeRolesStore.class)) {
+            PlainActionFuture<RoleRetrievalResult> future = new PlainActionFuture<>();
+            Set<String> reservedRoles = randomUnique(() -> randomFrom(ReservedRolesStore.names()), randomIntBetween(1, 5));
+            rolesStore.getRoleDescriptors(reservedRoles, future);
+            RoleRetrievalResult result = future.actionGet();
+            assertThat(result, notNullValue());
+            assertTrue(result.isSuccess());
+            assertThat(result.getDescriptors(), is(empty()));
+        }
+    }
+
+    public void testGetAllRoles() {
+        for (NativeRolesStore rolesStore : internalCluster().getInstances(NativeRolesStore.class)) {
+            PlainActionFuture<RoleRetrievalResult> future = new PlainActionFuture<>();
+            rolesStore.getRoleDescriptors(randomBoolean() ? null : Set.of(), future);
+            RoleRetrievalResult result = future.actionGet();
+            assertThat(result, notNullValue());
+            assertTrue(result.isSuccess());
+            assertThat(result.getDescriptors().stream().map(RoleDescriptor::getName).toList(), containsInAnyOrder(customRoles.toArray()));
+        }
+    }
+}