Skip to content

Filter stale role IDs in Infinispan client adapters #51616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PRAHLAD09-dev wants to merge 5 commits into
base: main
Choose a base branch
from
+136 −2
Open

Filter stale role IDs in Infinispan client adapters #51616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
73e2e89
Filter stale role IDs in Infinispan client adapters
PRAHLAD09-dev Aug 11, 2026
0b6e844
Add regression tests for stale cached role ID filtering
PRAHLAD09-dev Aug 12, 2026
fe12735
Add regression tests for unresolvable role mappings
PRAHLAD09-dev Aug 13, 2026
eb1957c
Keep JUnit 4 tests with Vintage engine
PRAHLAD09-dev Aug 13, 2026
d013fde
Add license headers to regression tests
PRAHLAD09-dev Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions model/infinispan/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,27 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
Comment on lines +104 to +107
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

<!-- Needed for infinispan statistics -->
<dependency>
<groupId>org.eclipse.microprofile.metrics</groupId>
Expand Down
3 changes: 2 additions & 1 deletion model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ public void setFullScopeAllowed(boolean value) {
public Stream<RoleModel> getScopeMappingsStream() {
if (isUpdated()) return updated.getScopeMappingsStream();
return cached.getScope().stream()
.map(id -> cacheSession.getRoleById(cachedRealm, id));
.map(id -> cacheSession.getRoleById(cachedRealm, id))
.filter(Objects::nonNull);

@PRAHLAD09-dev PRAHLAD09-dev Aug 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I checked the existing model/cache test infrastructure and confirmed that the existing tests cover the normal cache invalidation path.

For this specific regression, reproducing an unknown role ID in CachedClient deterministically requires bypassing the normal cache invalidation path. The available test infrastructure does not provide a supported API for injecting such a stale cached role ID, and Mockito is not an existing dependency of this module.

I therefore kept the fix limited to the null handling, matching the existing JPA implementation with Objects::nonNull, rather than introducing a new mocking dependency or relying on internal cache-state manipulation.

@PRAHLAD09-dev PRAHLAD09-dev Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: @ssilvert provided a concrete Mockito-based approach that instantiates ClientAdapter/ClientScopeAdapter directly with mocked RealmCacheSession/CachedClient/CachedClientScope collaborators — much simpler than I'd assumed, and it doesn't require touching internal cache state. Added junit-jupiter-api, junit-jupiter-engine, and mockito-core as test dependencies to model/infinispan (following the same coordinates already used in operator and ssf/transmitter), and added ClientAdapterTest/ClientScopeAdapterTest covering exactly this scenario. Both pass locally (Tests run: 2, Failures: 0, Errors: 0). Pushed in the latest commit.

}

public void addScopeMapping(RoleModel role) {
Expand Down
4 changes: 3 additions & 1 deletion model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Supplier;
import java.util.stream.Stream;

Expand Down Expand Up @@ -164,7 +165,8 @@ public void setProtocol(String protocol) {
public Stream<RoleModel> getScopeMappingsStream() {
if (isUpdated()) return updated.getScopeMappingsStream();
return cached.getScope().stream()
.map(id -> cacheSession.getRoleById(cachedRealm, id));
.map(id -> cacheSession.getRoleById(cachedRealm, id))
.filter(Objects::nonNull);

@PRAHLAD09-dev PRAHLAD09-dev Aug 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies to CachedClientScope. The existing test infrastructure covers normal cache invalidation, but does not provide a supported way to inject an arbitrary stale role ID into the cached scope mappings.

Adding a mocking dependency or manipulating internal cache state solely for this race-condition scenario would add unnecessary test infrastructure to the module. The production change mirrors the existing JPA implementation by filtering unresolved RoleModel instances with Objects::nonNull.

@PRAHLAD09-dev PRAHLAD09-dev Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: @ssilvert provided a concrete Mockito-based approach that instantiates ClientAdapter/ClientScopeAdapter directly with mocked RealmCacheSession/CachedClient/CachedClientScope collaborators — much simpler than I'd assumed, and it doesn't require touching internal cache state. Added junit-jupiter-api, junit-jupiter-engine, and mockito-core as test dependencies to model/infinispan (following the same coordinates already used in operator and ssf/transmitter), and added ClientAdapterTest/ClientScopeAdapterTest covering exactly this scenario. Both pass locally (Tests run: 2, Failures: 0, Errors: 0). Pushed in the latest commit.

}

@Override
Expand Down
55 changes: 55 additions & 0 deletions model/infinispan/src/test/java/org/keycloak/models/cache/infinispan/ClientAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2026 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.models.cache.infinispan;

import java.util.List;
import java.util.Set;

import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.cache.infinispan.entities.CachedClient;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class ClientAdapterTest {

@Test
void getScopeMappingsStreamFiltersUnresolvableRoleIds() {
RealmModel realm = mock(RealmModel.class);
RealmCacheSession cacheSession = mock(RealmCacheSession.class);
CachedClient cached = mock(CachedClient.class);

RoleModel liveRole = mock(RoleModel.class);
when(cached.getScope()).thenReturn(Set.of("stale-role-id", "live-role-id"));
// simulates a role deleted on another node whose cache invalidation
// hasn't reached this one yet
when(cacheSession.getRoleById(eq(realm), eq("stale-role-id"))).thenReturn(null);
when(cacheSession.getRoleById(eq(realm), eq("live-role-id"))).thenReturn(liveRole);

ClientAdapter adapter = new ClientAdapter(realm, cached, cacheSession);

List<RoleModel> result = adapter.getScopeMappingsStream().toList();

assertEquals(List.of(liveRole), result,
"a scope mapping pointing at an unresolvable role must be silently dropped, not surfaced as null");
}
}
55 changes: 55 additions & 0 deletions ...infinispan/src/test/java/org/keycloak/models/cache/infinispan/ClientScopeAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2026 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.models.cache.infinispan;

import java.util.List;
import java.util.Set;

import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.cache.infinispan.entities.CachedClientScope;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class ClientScopeAdapterTest {

@Test
void getScopeMappingsStreamFiltersUnresolvableRoleIds() {
RealmModel realm = mock(RealmModel.class);
RealmCacheSession cacheSession = mock(RealmCacheSession.class);
CachedClientScope cached = mock(CachedClientScope.class);

RoleModel liveRole = mock(RoleModel.class);
when(cached.getScope()).thenReturn(Set.of("stale-role-id", "live-role-id"));
// simulates a role deleted on another node whose cache invalidation
// hasn't reached this one yet
when(cacheSession.getRoleById(eq(realm), eq("stale-role-id"))).thenReturn(null);
when(cacheSession.getRoleById(eq(realm), eq("live-role-id"))).thenReturn(liveRole);

ClientScopeAdapter adapter = new ClientScopeAdapter(realm, cached, cacheSession);

List<RoleModel> result = adapter.getScopeMappingsStream().toList();

assertEquals(List.of(liveRole), result,
"a scope mapping pointing at an unresolvable role must be silently dropped, not surfaced as null");
}
}