Skip to content

Commit 332c9b6

Browse files
vramikpedroigor
authored andcommitted
Fix NPE when accessing group concurrently
Closes #40368 Signed-off-by: vramik <[email protected]>
1 parent e92b825 commit 332c9b6

File tree

4 files changed

+51
-11
lines changed

4 files changed

+51
-11
lines changed

model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ public Stream<GroupModel> getSubGroupsStream(String search, Boolean exact, Integ
271271
@Override
272272
public Long getSubGroupsCount() {
273273
if (isUpdated()) return updated.getSubGroupsCount();
274-
return getGroupModel().getSubGroupsCount();
274+
GroupModel model = modelSupplier.get();
275+
return model == null ? null : model.getSubGroupsCount();
275276
}
276277

277278
@Override

model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public CachedGroup(Long revision, RealmModel realm, GroupModel group) {
5858
this.type = group.getType();
5959
}
6060

61+
@Override
6162
public String getRealm() {
6263
return realm;
6364
}

services/src/main/java/org/keycloak/utils/GroupUtils.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,4 @@ public static GroupRepresentation toRepresentation(GroupPermissionEvaluator grou
9898
rep.setAccess(groupsEvaluator.getAccess(groupTree));
9999
return rep;
100100
}
101-
102-
private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search) {
103-
if (StringUtil.isBlank(search)) {
104-
return true;
105-
}
106-
if (group.getName().contains(search)) {
107-
return true;
108-
}
109-
return group.getSubGroupsStream().findAny().isPresent();
110-
}
111101
}

tests/base/src/test/java/org/keycloak/tests/admin/group/GroupTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.http.client.methods.CloseableHttpResponse;
2626
import org.apache.http.client.methods.HttpGet;
2727
import org.apache.http.impl.client.CloseableHttpClient;
28+
import org.hamcrest.Matchers;
2829
import org.junit.jupiter.api.Assertions;
2930
import org.junit.jupiter.api.Test;
3031
import org.keycloak.admin.client.Keycloak;
@@ -76,6 +77,9 @@
7677
import java.util.List;
7778
import java.util.Map;
7879
import java.util.UUID;
80+
import java.util.concurrent.CopyOnWriteArrayList;
81+
import java.util.concurrent.atomic.AtomicBoolean;
82+
import java.util.stream.IntStream;
7983

8084
import static org.hamcrest.MatcherAssert.assertThat;
8185
import static org.hamcrest.Matchers.anEmptyMap;
@@ -90,6 +94,7 @@
9094
import static org.junit.jupiter.api.Assertions.assertNotNull;
9195
import static org.junit.jupiter.api.Assertions.assertNull;
9296
import static org.junit.jupiter.api.Assertions.assertTrue;
97+
import static org.junit.jupiter.api.Assertions.fail;
9398

9499
/**
95100
* @author <a href="mailto:[email protected]">Marko Strukelj</a>
@@ -109,6 +114,49 @@ public class GroupTest extends AbstractGroupTest {
109114
@InjectHttpClient
110115
CloseableHttpClient httpClient;
111116

117+
118+
@Test
119+
public void createMultiDeleteMultiReadMulti() {
120+
// create multiple groups
121+
List<String> groupUuuids = new ArrayList<>();
122+
IntStream.range(0, 100).forEach(groupIndex -> {
123+
GroupRepresentation group = new GroupRepresentation();
124+
group.setName("Test Group " + groupIndex);
125+
try (Response response = managedRealm.admin().groups().add(group)) {
126+
boolean created = response.getStatusInfo().getFamily() == Response.Status.Family.SUCCESSFUL;
127+
if (created) {
128+
final String groupUuid = ApiUtil.getCreatedId(response);
129+
groupUuuids.add(groupUuid);
130+
} else {
131+
fail("Failed to create group: " + response.getStatusInfo().getReasonPhrase());
132+
}
133+
}
134+
});
135+
136+
AtomicBoolean deletedAll = new AtomicBoolean(false);
137+
List<Exception> caughtExceptions = new CopyOnWriteArrayList<>();
138+
// read groups in a separate thread
139+
new Thread(() -> {
140+
while (!deletedAll.get()) {
141+
try {
142+
// just loading briefs
143+
managedRealm.admin().groups().groups(null, 0, Integer.MAX_VALUE, true);
144+
} catch (Exception e) {
145+
146+
caughtExceptions.add(e);
147+
}
148+
}
149+
}).start();
150+
151+
// delete groups
152+
groupUuuids.forEach(groupUuid -> {
153+
managedRealm.admin().groups().group(groupUuid).remove();
154+
});
155+
deletedAll.set(true);
156+
157+
assertThat(caughtExceptions, Matchers.empty());
158+
}
159+
112160
// KEYCLOAK-2716 Can't delete client if its role is assigned to a group
113161
@Test
114162
public void testClientRemoveWithClientRoleGroupMapping() {

0 commit comments

Comments
 (0)