From 4f55b9b6bd6dadfdd04cfd09eb532dfdd26d5607 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 3 Oct 2025 14:25:57 -0300 Subject: [PATCH] Filter invalid resources and scopes when processing entries from the cache Closes #42907 Signed-off-by: Pedro Igor --- .../authorization/PolicyAdapter.java | 6 ++ .../StoreFactoryCacheSession.java | 4 +- .../testsuite/authz/EntitlementAPITest.java | 62 ++++++++++++++++++- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/PolicyAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/PolicyAdapter.java index fdd4266d7e4..0828e5b0544 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/PolicyAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/PolicyAdapter.java @@ -234,6 +234,9 @@ public class PolicyAdapter implements Policy, CachedModel { ResourceServer resourceServer = getResourceServer(); for (String resourceId : cached.getResourcesIds(session, modelSupplier)) { Resource resource = resourceStore.findById(resourceServer, resourceId); + if (resource == null) { + continue; + } cacheSession.cacheResource(resource); resources.add(resource); } @@ -305,6 +308,9 @@ public class PolicyAdapter implements Policy, CachedModel { ScopeStore scopeStore = cacheSession.getScopeStore(); for (String scopeId : cached.getScopesIds(session, modelSupplier)) { Scope scope = scopeStore.findById(resourceServer, scopeId); + if (scope == null) { + continue; + } cacheSession.cacheScope(scope); scopes.add(scope); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java index aabcd0d2c93..a10155af9f4 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java @@ -825,9 +825,9 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { Set resources = query.getResources(); if (consumer != null) { - resources.stream().map(resourceId -> (R) findById(resourceServer, resourceId)).forEach(consumer); + resources.stream().map(resourceId -> (R) findById(resourceServer, resourceId)).filter(Objects::nonNull).forEach(consumer); } else { - model = resources.stream().map(resourceId -> (R) findById(resourceServer, resourceId)).collect(Collectors.toList()); + model = resources.stream().map(resourceId -> (R) findById(resourceServer, resourceId)).filter(Objects::nonNull).collect(Collectors.toList()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java index 29ff122cbe9..39c2b358722 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java @@ -37,6 +37,9 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -59,13 +62,23 @@ import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.ResourceResource; import org.keycloak.admin.client.resource.ScopePermissionsResource; +import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.client.AuthorizationDeniedException; import org.keycloak.authorization.client.AuthzClient; import org.keycloak.authorization.client.Configuration; import org.keycloak.authorization.client.representation.TokenIntrospectionResponse; import org.keycloak.authorization.client.util.HttpResponseException; +import org.keycloak.authorization.model.ResourceServer; +import org.keycloak.authorization.store.ResourceServerStore; +import org.keycloak.authorization.store.StoreFactory; import org.keycloak.common.util.Base64Url; +import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.events.EventType; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.cache.infinispan.authorization.StoreFactoryCacheSession; +import org.keycloak.models.cache.infinispan.authorization.entities.ResourceQuery; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.HardcodedClaim; @@ -97,7 +110,6 @@ import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls; import org.keycloak.testsuite.util.ClientBuilder; -import org.keycloak.testsuite.util.oauth.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.RoleBuilder; import org.keycloak.testsuite.util.RolesBuilder; @@ -2649,6 +2661,54 @@ public class EntitlementAPITest extends AbstractAuthzTest { assertEquals("scope1", permissions.get(0).getScopes().iterator().next()); } + @Test + public void testDeleteConcurrency() throws Exception { + ClientResource client = getClient(getRealm(), RESOURCE_SERVER_TEST); + AuthorizationResource authorization = client.authorization(); + CountDownLatch successfulIterations = new CountDownLatch(200); + AtomicBoolean stop = new AtomicBoolean(false); + + // Thread that will be creating and deleting a resource + new Thread(() -> { + while (!stop.get()) { + String resourceName = "Test Resource"; + List test = authorization.resources().findByName(resourceName); + + if (test.isEmpty()) { + ResourceRepresentation resource = new ResourceRepresentation(); + resource.setName(resourceName); + authorization.resources().create(resource).close(); + } else { + authorization.resources().resource(test.get(0).getId()).remove(); + } + } + }).start(); + + AuthzClient authzClient = getAuthzClient(AUTHZ_CLIENT_CONFIG); + + for (int i = 0; i < 3; i++) { + // Thread that will be requesting permissions against the resource being created and deleted + new Thread(() -> { + while (!stop.get()) { + try { + authzClient.authorization().authorize(); + successfulIterations.countDown(); + } catch (Exception ignore) { + // unexpected failures will end execution and the latch will not be decremented + stop.set(true); + return; + } + } + }).start(); + } + + try { + assertTrue(successfulIterations.await(15, TimeUnit.SECONDS)); + } finally { + stop.set(true); + } + } + private void testRptRequestWithResourceName(String configFile) { Metadata metadata = new Metadata();