Drop role cache entries when project is deleted

server/src/main/java/org/elasticsearch/cluster/project/ProjectDeletedListener.java

@@ -0,0 +1,36 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.cluster.project;
+
+import org.elasticsearch.cluster.ClusterChangedEvent;
+import org.elasticsearch.cluster.metadata.ProjectId;
+import org.elasticsearch.cluster.service.ClusterService;
+
+import java.util.function.Consumer;
+
+/**
+ * Utility class to make it easy to run a block of code whenever a project is deleted (e.g. to cleanup cache entries)
+ */
+public class ProjectDeletedListener {
+
+    private final Consumer<ProjectId> consumer;
+
+    public ProjectDeletedListener(Consumer<ProjectId> consumer) {
+        this.consumer = consumer;
+    }
+
+    public void attach(ClusterService clusterService) {
+        clusterService.addListener(event -> {
+            final ClusterChangedEvent.ProjectsDelta delta = event.projectDelta();
+            delta.removed().forEach(consumer);
+        });
+    }
+
+}

server/src/test/java/org/elasticsearch/cluster/project/ProjectDeletedListenerTests.java

@@ -0,0 +1,65 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.cluster.project;
+
+import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.cluster.metadata.ProjectId;
+import org.elasticsearch.cluster.metadata.ProjectMetadata;
+import org.elasticsearch.cluster.routing.GlobalRoutingTableTestHelper;
+import org.elasticsearch.cluster.routing.RoutingTable;
+import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
+import org.elasticsearch.test.ClusterServiceUtils;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class ProjectDeletedListenerTests extends ESTestCase {
+
+    public void testInvocation() {
+        final List<ProjectId> existingProjects = randomList(5, 15, ESTestCase::randomUniqueProjectId);
+
+        try (ClusterService clusterService = ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool())) {
+            final ClusterState.Builder csBuilder = ClusterState.builder(ClusterName.DEFAULT);
+            existingProjects.forEach(p -> csBuilder.putProjectMetadata(ProjectMetadata.builder(p).build()));
+            final ClusterState cs0 = csBuilder.build();
+
+            ClusterServiceUtils.setState(clusterService, cs0);
+
+            final Set<ProjectId> notifiedProjects = new HashSet<>();
+            var pdl = new ProjectDeletedListener(notifiedProjects::add);
+            pdl.attach(clusterService);
+
+            final Set<ProjectId> projectsToDelete = Set.copyOf(
+                randomSubsetOf(randomIntBetween(1, existingProjects.size() / 2), existingProjects)
+            );
+            final List<ProjectId> projectsToCreate = randomList(0, 3, ESTestCase::randomUniqueProjectId);
+
+            final var mdBuilder = Metadata.builder(cs0.metadata());
+            projectsToDelete.forEach(mdBuilder::removeProject);
+            projectsToCreate.forEach(p -> mdBuilder.put(ProjectMetadata.builder(p).build()));
+            var md = mdBuilder.build();
+            var cs1 = ClusterState.builder(cs0)
+                .metadata(md)
+                .routingTable(GlobalRoutingTableTestHelper.buildRoutingTable(md, RoutingTable.Builder::addAsNew))
+                .build();
+            ClusterServiceUtils.setState(clusterService, cs1);
+
+            assertThat(notifiedProjects, equalTo(projectsToDelete));
+        }
+    }
+
+}

test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java

@@ -19,13 +19,14 @@
 
 import java.util.Collection;
 import java.util.Objects;
+import java.util.function.Supplier;
 
 /**
  * An implementation of {@link ProjectResolver} that handles multiple projects for testing purposes. Not usable in production
  */
 public final class TestProjectResolvers {
 
-    public static final ProjectResolver DEFAULT_PROJECT_ONLY = singleProject(Metadata.DEFAULT_PROJECT_ID, true);
+    public static final ProjectResolver DEFAULT_PROJECT_ONLY = singleProject(() -> Metadata.DEFAULT_PROJECT_ID, true);
 
     /**
      * @return a ProjectResolver that must only be used in a cluster context. It throws in single project related methods.
@@ -131,6 +132,14 @@ public static ProjectResolver alwaysThrow() {
      * The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
      */
     public static ProjectResolver singleProject(ProjectId projectId) {
+        return singleProject(() -> projectId, false);
+    }
+
+    /**
+     * This method returns a ProjectResolver that gives back the specified project-id when its getProjectId method is called.
+     * The ProjectResolver can work with cluster state containing multiple projects and its supportsMultipleProjects returns true.
+     */
+    public static ProjectResolver singleProject(Supplier<ProjectId> projectId) {
         return singleProject(projectId, false);
     }
 
@@ -140,11 +149,11 @@ public static ProjectResolver singleProject(ProjectId projectId) {
      * In addition, the ProjectResolvers returns false for supportsMultipleProjects.
      */
     public static ProjectResolver singleProjectOnly(ProjectId projectId) {
-        return singleProject(projectId, true);
+        return singleProject(() -> projectId, true);
     }
 
-    private static ProjectResolver singleProject(ProjectId projectId, boolean only) {
-        Objects.requireNonNull(projectId);
+    private static ProjectResolver singleProject(Supplier<ProjectId> projectIdSupplier, boolean only) {
+        Objects.requireNonNull(projectIdSupplier);
         return new ProjectResolver() {
 
             @Override
@@ -157,7 +166,7 @@ public ProjectMetadata getProjectMetadata(Metadata metadata) {
 
             @Override
             public ProjectId getProjectId() {
-                return projectId;
+                return projectIdSupplier.get();
             }
 
             @Override
@@ -170,6 +179,7 @@ public Collection<ProjectId> getProjectIds(ClusterState clusterState) {
 
             @Override
             public <E extends Exception> void executeOnProject(ProjectId otherProjectId, CheckedRunnable<E> body) throws E {
+                final ProjectId projectId = projectIdSupplier.get();
                 if (projectId.equals(otherProjectId)) {
                     body.run();
                 } else {

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

@@ -1037,6 +1037,7 @@ Collection<Object> createComponents(
         );
         final CompositeRolesStore allRolesStore = new CompositeRolesStore(
             settings,
+            clusterService,
             roleProviders,
             privilegeStore,
             threadPool.getThreadContext(),

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

@@ -12,7 +12,9 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRunnable;
 import org.elasticsearch.cluster.metadata.ProjectId;
+import org.elasticsearch.cluster.project.ProjectDeletedListener;
 import org.elasticsearch.cluster.project.ProjectResolver;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.cache.Cache;
@@ -23,7 +25,6 @@
 import org.elasticsearch.common.util.concurrent.ReleasableLock;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.set.Sets;
-import org.elasticsearch.core.FixForMultiProject;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.license.XPackLicenseState;
@@ -113,7 +114,6 @@ public class CompositeRolesStore {
     private final DocumentSubsetBitsetCache dlsBitsetCache;
     private final AnonymousUser anonymousUser;
 
-    @FixForMultiProject(description = "Deleted projects are never cleared from this map")
     private final Map<ProjectId, Long> numInvalidation = new ConcurrentHashMap<>();
     private final RoleDescriptorStore roleReferenceResolver;
     private final Role superuserRole;
@@ -124,6 +124,7 @@ public class CompositeRolesStore {
 
     public CompositeRolesStore(
         Settings settings,
+        ClusterService clusterService,
         RoleProviders roleProviders,
         NativePrivilegeStore privilegeStore,
         ThreadContext threadContext,
@@ -137,6 +138,8 @@ public CompositeRolesStore(
         Executor roleBuildingExecutor,
         Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer
     ) {
+        new ProjectDeletedListener(this::removeProject).attach(clusterService);
+
         this.roleProviders = roleProviders;
         roleProviders.addChangeListener(new RoleProviders.ChangeListener() {
             @Override
@@ -633,6 +636,12 @@ public void invalidateProject(ProjectId projectId) {
         }
     }
 
+    final void removeProject(ProjectId projectId) {
+        numInvalidation.remove(projectId);
+        negativeLookupCacheHelper.removeKeysIf(key -> key.projectId().equals(projectId));
+        roleCacheHelper.removeKeysIf(key -> key.projectId().equals(projectId));
+    }
+
     public void invalidateAll() {
         numInvalidation.replaceAll((p, num) -> num + 1);
         negativeLookupCache.invalidateAll();
@@ -655,6 +664,11 @@ public void invalidateClusterScopedRoles(Set<String> roles) {
         negativeLookupCacheHelper.removeKeysIf(key -> roles.contains(key.value()));
     }
 
+    // for testing
+    Iterable<ProjectScoped<RoleKey>> cachedRoles() {
+        return this.roleCache.keys();
+    }
+
     public void usageStats(ActionListener<Map<String, Object>> listener) {
         final Map<String, Object> usage = new HashMap<>();
         usage.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java

@@ -247,6 +247,7 @@ public void setup() {
         rolesStore = Mockito.spy(
             new CompositeRolesStore(
                 settings,
+                mock(ClusterService.class),
                 mock(RoleProviders.class),
                 mock(NativePrivilegeStore.class),
                 new ThreadContext(settings),