From 25ffea98d4e9cc53062df818207ff5b4de51df6b Mon Sep 17 00:00:00 2001 From: Nicola Date: Tue, 19 May 2026 11:29:17 +0200 Subject: [PATCH] fix: guard null email in group membership leave admin event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GroupsController.dispatchGroupMembershipLeaveEvent passed user.getEmail() directly into Map.of(), which rejects null values and threw NullPointerException whenever the removed member had no email set in Keycloak. PATCH /Groups remove-member then returned HTTP 500 to the client. The sibling join event already coalesces null to "" — apply the same guard to the leave event so the two paths stay symmetric. Adds a regression test that creates a user, clears their email via the Keycloak admin client, then removes them from a group via SCIM PATCH and asserts success. Closes #103 --- .../scim/server/groups/GroupsController.java | 2 +- .../functional/RealmGroupPatchTestsIT.java | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/groups/GroupsController.java b/src/main/java/fi/metatavu/keycloak/scim/server/groups/GroupsController.java index 96adb9c..424c58c 100644 --- a/src/main/java/fi/metatavu/keycloak/scim/server/groups/GroupsController.java +++ b/src/main/java/fi/metatavu/keycloak/scim/server/groups/GroupsController.java @@ -411,7 +411,7 @@ protected void dispatchGroupMembershipLeaveEvent( groupRepresentation, Map.of( UserModel.USERNAME, user.getUsername(), - UserModel.EMAIL, user.getEmail() + UserModel.EMAIL, user.getEmail() == null ? "" : user.getEmail() ) ); } diff --git a/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmGroupPatchTestsIT.java b/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmGroupPatchTestsIT.java index 671533e..093b04e 100644 --- a/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmGroupPatchTestsIT.java +++ b/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmGroupPatchTestsIT.java @@ -9,6 +9,7 @@ import org.keycloak.events.admin.AdminEvent; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; +import org.keycloak.representations.idm.UserRepresentation; import org.testcontainers.junit.jupiter.Testcontainers; import java.io.IOException; @@ -208,4 +209,51 @@ void testRemoveGroupMemberAdminEvents() throws ApiException, IOException { deleteRealmUser(TestConsts.TEST_REALM, user.getId()); deleteRealmGroup(TestConsts.TEST_REALM, group.getId()); } + + /** + * Regression: removing a group member whose email is null must not 500. + * Map.of() rejects null values, so the admin event dispatch previously + * NPE'd when the user had no email set. + */ + @Test + void testRemoveGroupMemberWithoutEmail() throws ApiException { + ScimClient scimClient = getAuthenticatedScimClient(); + + User user = createUser(scimClient, "no-email-user", "No", "Email"); + Group group = createGroup(scimClient, "no-email-group"); + + UserRepresentation userRep = findRealmUser(TestConsts.TEST_REALM, user.getId()); + userRep.setEmail(null); + getKeycloakContainer().getKeycloakAdminClient() + .realms() + .realm(TestConsts.TEST_REALM) + .users() + .get(user.getId()) + .update(userRep); + + PatchRequest addRequest = new PatchRequest(); + addRequest.setSchemas(List.of("urn:ietf:params:scim:api:messages:2.0:PatchOp")); + PatchRequestOperationsInner addOperation = new PatchRequestOperationsInner(); + addOperation.setOp("add"); + addOperation.setPath("members"); + GroupMembersInner member = new GroupMembersInner(); + member.setValue(user.getId()); + addOperation.setValue(Collections.singletonList(member)); + addRequest.setOperations(List.of(addOperation)); + scimClient.patchGroup(group.getId(), addRequest); + + PatchRequest removeRequest = new PatchRequest(); + removeRequest.setSchemas(List.of("urn:ietf:params:scim:api:messages:2.0:PatchOp")); + PatchRequestOperationsInner removeOperation = new PatchRequestOperationsInner(); + removeOperation.setOp("remove"); + removeOperation.setPath("members[value eq \"" + user.getId() + "\"]"); + removeRequest.setOperations(List.of(removeOperation)); + + Group patched = scimClient.patchGroup(group.getId(), removeRequest); + + assertTrue(patched.getMembers() == null || patched.getMembers().isEmpty()); + + deleteRealmUser(TestConsts.TEST_REALM, user.getId()); + deleteRealmGroup(TestConsts.TEST_REALM, group.getId()); + } }