filter reserved roles in other cases and add tests

Author: slobodanadamovic

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()));
+        }
+    }
+}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java

@@ -177,6 +177,19 @@ public void accept(Set<String> names, ActionListener<RoleRetrievalResult> listen
         getRoleDescriptors(names, listener);
     }
 
+    private Set<String> filterReservedRoles(Set<String> names) {
+        if (names == null || names.isEmpty()) {
+            return names;
+        }
+        Set<String> filtered = new HashSet<>(names.size());
+        for (String name : names) {
+            if (false == reservedRoleNameChecker.isReserved(name)) {
+                filtered.add(name);
+            }
+        }
+        return filtered;
+    }
+
     /**
      * Retrieve a list of roles, if rolesToGet is null or empty, fetch all roles
      */
@@ -186,13 +199,21 @@ public void getRoleDescriptors(Set<String> names, final ActionListener<RoleRetri
             return;
         }
 
+        final Set<String> rolesToGet = filterReservedRoles(names);
+        if ((names != null && names.size() > 0) && (rolesToGet == null || rolesToGet.isEmpty())) {
+            // if we have no roles to get, it means all requested roles were reserved.
+            // we can short-circuit and return an empty set here.
+            listener.onResponse(RoleRetrievalResult.success(Set.of()));
+            return;
+        }
+
         final SecurityIndexManager frozenSecurityIndex = this.securityIndex.defensiveCopy();
         if (frozenSecurityIndex.indexExists() == false) {
             // TODO remove this short circuiting and fix tests that fail without this!
             listener.onResponse(RoleRetrievalResult.success(Collections.emptySet()));
         } else if (frozenSecurityIndex.isAvailable(SEARCH_SHARDS) == false) {
             listener.onResponse(RoleRetrievalResult.failure(frozenSecurityIndex.getUnavailableReason(SEARCH_SHARDS)));
-        } else if (names == null || names.isEmpty()) {
+        } else if (rolesToGet == null || rolesToGet.isEmpty()) {
             securityIndex.checkIndexVersionThenExecute(listener::onFailure, () -> {
                 QueryBuilder query = QueryBuilders.boolQuery()
                     .must(QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE))
@@ -220,11 +241,11 @@ public void getRoleDescriptors(Set<String> names, final ActionListener<RoleRetri
                     );
                 }
             });
-        } else if (names.size() == 1) {
-            getRoleDescriptor(Objects.requireNonNull(names.iterator().next()), listener);
+        } else if (rolesToGet.size() == 1) {
+            getRoleDescriptor(Objects.requireNonNull(rolesToGet.iterator().next()), listener);
         } else {
             securityIndex.checkIndexVersionThenExecute(listener::onFailure, () -> {
-                final String[] roleIds = names.stream().map(NativeRolesStore::getIdForRole).toArray(String[]::new);
+                final String[] roleIds = rolesToGet.stream().map(NativeRolesStore::getIdForRole).toArray(String[]::new);
                 MultiGetRequest multiGetRequest = client.prepareMultiGet().addIds(SECURITY_MAIN_ALIAS, roleIds).request();
                 executeAsyncWithOrigin(
                     client.threadPool().getThreadContext(),