From 65c2624c66df85d38d853e46bdd347243cb60597 Mon Sep 17 00:00:00 2001 From: Nir Gofman Date: Thu, 21 May 2026 11:25:27 +0300 Subject: [PATCH 1/2] fix(groups): reconcile members on PUT and invalidate user cache Two related fixes to GroupsController: 1. updateGroup (PUT /Groups/{id}) previously only set displayName and silently dropped the `members` array. Okta's Group Push uses PUT (not PATCH) with the desired final member list, so this no-op meant membership changes from Okta never propagated to Keycloak. The fix reconciles the request's members against the current members: removes users no longer in the desired set (with leave events) and adds new ones (with join events). 2. After mutating membership via user.leaveGroup() / user.joinGroup(), Keycloak's Infinispan user cache holds a stale view of the user's group list. Subsequent reads of /users/{id}/groups return outdated entries until the cache expires. Explicitly evict the affected user from the UserCache to ensure immediate consistency. Verified against Okta SCIM provisioning + Keycloak 26.6: - PUT /Groups/{id} with added/removed members updates both the group's members list and each user's groups list. - No manual cache clear required after membership changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scim/server/groups/GroupsController.java | 57 ++++++++++++++++++- 1 file changed, 56 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..9be9fe7 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 @@ -19,6 +19,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.cache.UserCache; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.GroupRepresentation; @@ -26,6 +27,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; /** @@ -141,7 +144,59 @@ public GroupsList listGroups( * @return updated group */ public Group updateGroup(ScimContext scimContext, GroupModel existing, fi.metatavu.keycloak.scim.server.model.Group group) { - existing.setName(group.getDisplayName()); + KeycloakSession session = scimContext.getSession(); + RealmModel realm = scimContext.getRealm(); + + if (group.getDisplayName() != null) { + existing.setName(group.getDisplayName()); + } + + // SCIM PUT is a full replace of the resource — reconcile members against the request. + // Okta's Group Push uses PUT (not PATCH) with the desired final member list. + List requestedMembers = group.getMembers(); + if (requestedMembers != null) { + Set desiredIds = requestedMembers.stream() + .map(GroupMembersInner::getValue) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + List currentMembers = session.users() + .getGroupMembersStream(realm, existing) + .collect(Collectors.toList()); + + Set currentIds = currentMembers.stream() + .map(UserModel::getId) + .collect(Collectors.toSet()); + + UserCache userCache = session.getProvider(UserCache.class); + + // Remove members no longer in the desired set + for (UserModel user : currentMembers) { + if (!desiredIds.contains(user.getId())) { + user.leaveGroup(existing); + if (userCache != null) { + userCache.evict(realm, user); + } + dispatchGroupMembershipLeaveEvent(scimContext, existing, user); + } + } + + // Add members that are new + for (String id : desiredIds) { + if (currentIds.contains(id)) { + continue; + } + UserModel user = session.users().getUserById(realm, id); + if (user != null) { + user.joinGroup(existing); + if (userCache != null) { + userCache.evict(realm, user); + } + dispatchGroupMembershipJoinEvent(scimContext, existing, user); + } + } + } + return translateGroup(scimContext, existing); } From f90eacc3bbc18715450fe6ca514c5d9f377e73f4 Mon Sep 17 00:00:00 2001 From: zivisaiah Date: Mon, 25 May 2026 18:28:36 +0300 Subject: [PATCH 2/2] SCIM error responses: RFC 7644 JSON + idempotent CREATE on conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Add ScimErrorResponse utility for RFC 7644 §3.12 compliant JSON error bodies. Replace all plain-text .entity("string") error responses so SCIM clients can parse error details. 2. Idempotent POST /Users: when the username already exists, update the existing user and return 200 OK instead of 409 Conflict. Okta treats 409 as a permanent provisioning failure. Note: SonarCloud duplication flag is a false positive — the flagged blocks are the pre-existing structural duplication between RealmScimServer and OrganizationScimServer (identical method bodies with different context types). This PR did not introduce that duplication; it was inherited from the base branch. --- .../scim/server/ScimErrorResponse.java | 18 ++++++++++ .../keycloak/scim/server/ScimResources.java | 8 ++--- .../organization/OrganizationScimServer.java | 21 +++++------ .../scim/server/realm/RealmScimServer.java | 36 ++++++++++--------- 4 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 src/main/java/fi/metatavu/keycloak/scim/server/ScimErrorResponse.java diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/ScimErrorResponse.java b/src/main/java/fi/metatavu/keycloak/scim/server/ScimErrorResponse.java new file mode 100644 index 0000000..b96e834 --- /dev/null +++ b/src/main/java/fi/metatavu/keycloak/scim/server/ScimErrorResponse.java @@ -0,0 +1,18 @@ +package fi.metatavu.keycloak.scim.server; + +import jakarta.ws.rs.core.Response; + +public final class ScimErrorResponse { + + private static final String SCHEMA = "urn:ietf:params:scim:api:messages:2.0:Error"; + + private ScimErrorResponse() {} + + public static Response scimError(Response.Status status, String scimType, String detail) { + String json = "{\"schemas\":[\"" + SCHEMA + "\"]," + + "\"status\":\"" + status.getStatusCode() + "\"" + + (scimType != null ? ",\"scimType\":\"" + scimType + "\"" : "") + + ",\"detail\":\"" + detail.replace("\\", "\\\\").replace("\"", "\\\"") + "\"}"; + return Response.status(status).entity(json).type("application/scim+json").build(); + } +} diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/ScimResources.java b/src/main/java/fi/metatavu/keycloak/scim/server/ScimResources.java index fffa0f5..acc497c 100644 --- a/src/main/java/fi/metatavu/keycloak/scim/server/ScimResources.java +++ b/src/main/java/fi/metatavu/keycloak/scim/server/ScimResources.java @@ -89,7 +89,7 @@ public Response listRealmUsers( scimFilter = parseFilter(filter); } catch (Exception e) { logger.warn(String.format("Failed to parse filter: '%s'", filter), e); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid filter").build(); + return scimError(Response.Status.BAD_REQUEST, "invalidFilter", "Invalid filter"); } return realmScimServer.listUsers( @@ -213,7 +213,7 @@ public Response listRealmGroups( scimFilter = parseFilter(filter); } catch (Exception e) { logger.warn(String.format("Failed to parse filter: '%s'", filter), e); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid filter").build(); + return scimError(Response.Status.BAD_REQUEST, "invalidFilter", "Invalid filter"); } return realmScimServer.listGroups( @@ -424,7 +424,7 @@ public Response listOrganizationUsers( scimFilter = parseFilter(filter); } catch (Exception e) { logger.warn(String.format("Failed to parse filter: '%s'", filter), e); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid filter").build(); + return scimError(Response.Status.BAD_REQUEST, "invalidFilter", "Invalid filter"); } return getOrganizationScimServer().listUsers( @@ -554,7 +554,7 @@ public Response listOrganizationGroups( scimFilter = parseFilter(filter); } catch (Exception e) { logger.warn(String.format("Failed to parse filter: '%s'", filter), e); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid filter").build(); + return scimError(Response.Status.BAD_REQUEST, "invalidFilter", "Invalid filter"); } return getOrganizationScimServer().listGroups( diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/organization/OrganizationScimServer.java b/src/main/java/fi/metatavu/keycloak/scim/server/organization/OrganizationScimServer.java index 6865fa0..ad18af1 100644 --- a/src/main/java/fi/metatavu/keycloak/scim/server/organization/OrganizationScimServer.java +++ b/src/main/java/fi/metatavu/keycloak/scim/server/organization/OrganizationScimServer.java @@ -11,6 +11,7 @@ import fi.metatavu.keycloak.scim.server.patch.UnsupportedPatchOperation; import jakarta.ws.rs.InternalServerErrorException; import jakarta.ws.rs.NotFoundException; +import fi.metatavu.keycloak.scim.server.ScimErrorResponse; import jakarta.ws.rs.core.Response; import org.jboss.logging.Logger; import org.keycloak.models.*; @@ -36,12 +37,12 @@ public Response createUser(OrganizationScimContext scimContext, User createReque if (isBlank(createRequest.getUserName())) { logger.warn("Cannot create user: Missing userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Missing userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Missing userName"); } if (emailAsUsername && !isValidEmail(createRequest.getUserName())) { logger.warn("Cannot create user: Invalid email format for userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid email format for userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Invalid email format for userName"); } UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); @@ -68,19 +69,19 @@ public Response updateUser(OrganizationScimContext scimContext, String userId, f if (isBlank(username)) { logger.warn("Missing userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Missing userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Missing userName"); } if (emailAsUsername && !isValidEmail(updateRequest.getUserName())) { logger.warn("Cannot update user: Invalid email format for userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid email format for userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Invalid email format for userName"); } if (emailAsUsername && updateRequest.getEmails() != null) { for (fi.metatavu.keycloak.scim.server.model.UserEmailsInner email : updateRequest.getEmails()) { if (!Objects.equals(email.getValue(), updateRequest.getUserName())) { logger.warn("Conflicting email and userName when emailAsUsername is enabled"); - return Response.status(Response.Status.BAD_REQUEST).entity("Username and email must match when emailAsUsername is enabled").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Username and email must match when emailAsUsername is enabled"); } } } @@ -89,7 +90,7 @@ public Response updateUser(OrganizationScimContext scimContext, String userId, f UserModel user = session.users().getUserById(realm, userId); if (user == null) { logger.warn(String.format("User not found: %s", userId)); - return Response.status(Response.Status.NOT_FOUND).entity("User not found").build(); + return ScimErrorResponse.scimError(Response.Status.NOT_FOUND, null, "User not found"); } // Check if username is being changed to an already existing one @@ -102,7 +103,7 @@ public Response updateUser(OrganizationScimContext scimContext, String userId, f if (existing != null && !existing.getId().equals(userId)) { logger.warn(String.format("User name already taken: %s", updateRequest.getUserName())); - return Response.status(Response.Status.CONFLICT).entity("User name already taken").build(); + return ScimErrorResponse.scimError(Response.Status.CONFLICT, "uniqueness", "User name already taken"); } UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); @@ -119,7 +120,7 @@ public Response patchUser(OrganizationScimContext scimContext, String userId, fi UserModel existing = session.users().getUserById(realm, userId); if (existing == null) { logger.warn(String.format("User not found: %s", userId)); - return Response.status(Response.Status.NOT_FOUND).entity("User not found").build(); + return ScimErrorResponse.scimError(Response.Status.NOT_FOUND, null, "User not found"); } UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); @@ -128,7 +129,7 @@ public Response patchUser(OrganizationScimContext scimContext, String userId, fi fi.metatavu.keycloak.scim.server.model.User result = organizationUserController.patchOrganizationUser(scimContext, userAttributes, existing, patchRequest); return Response.ok(result).build(); } catch (UnsupportedPatchOperation e) { - return Response.status(Response.Status.BAD_REQUEST).entity("Unsupported patch operation").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Unsupported patch operation"); } } @@ -173,7 +174,7 @@ public Response deleteUser(OrganizationScimContext scimContext, String userId) { RoleModel scimManagedRole = realm.getRole("scim-managed"); if (scimManagedRole != null && !user.hasRole(scimManagedRole)) { logger.warn(String.format("User is not SCIM-managed: %s", userId)); - return Response.status(Response.Status.FORBIDDEN).entity("User is not managed by SCIM").build(); + return ScimErrorResponse.scimError(Response.Status.FORBIDDEN, null, "User is not managed by SCIM"); } organizationUserController.deleteOrganizationUser(scimContext, user); diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/realm/RealmScimServer.java b/src/main/java/fi/metatavu/keycloak/scim/server/realm/RealmScimServer.java index 19d087c..2e2c9c4 100644 --- a/src/main/java/fi/metatavu/keycloak/scim/server/realm/RealmScimServer.java +++ b/src/main/java/fi/metatavu/keycloak/scim/server/realm/RealmScimServer.java @@ -9,6 +9,7 @@ import fi.metatavu.keycloak.scim.server.patch.UnsupportedPatchOperation; import jakarta.ws.rs.InternalServerErrorException; import jakarta.ws.rs.NotFoundException; +import fi.metatavu.keycloak.scim.server.ScimErrorResponse; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; import java.net.URI; @@ -37,16 +38,17 @@ public Response createUser( if (isBlank(createRequest.getUserName())) { logger.warn("Cannot create user: Missing userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Missing userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Missing userName"); } + UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); + UserModel existing = session.users().getUserByUsername(realm, createRequest.getUserName()); if (existing != null) { - return Response.status(Response.Status.CONFLICT).entity("User already exists").build(); + User updated = usersController.updateUser(scimContext, userAttributes, existing, createRequest); + return Response.ok(updated).build(); } - UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); - User user = usersController.createUser( scimContext, userAttributes, @@ -68,19 +70,19 @@ public Response updateUser(RealmScimContext scimContext, String userId, fi.metat if (isBlank(updateRequest.getUserName())) { logger.warn("Missing userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Missing userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Missing userName"); } if (emailAsUsername && !isValidEmail(updateRequest.getUserName())) { logger.warn("Cannot update user: Invalid email format for userName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Invalid email format for userName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Invalid email format for userName"); } if (emailAsUsername && updateRequest.getEmails() != null) { for (fi.metatavu.keycloak.scim.server.model.UserEmailsInner email : updateRequest.getEmails()) { if (!Objects.equals(email.getValue(), updateRequest.getUserName())) { logger.warn("Conflicting email and userName when emailAsUsername is enabled"); - return Response.status(Response.Status.BAD_REQUEST).entity("Username and email must match when emailAsUsername is enabled").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Username and email must match when emailAsUsername is enabled"); } } } @@ -89,7 +91,7 @@ public Response updateUser(RealmScimContext scimContext, String userId, fi.metat UserModel user = session.users().getUserById(realm, userId); if (user == null) { logger.warn(String.format("User not found: %s", userId)); - return Response.status(Response.Status.NOT_FOUND).entity("User not found").build(); + return ScimErrorResponse.scimError(Response.Status.NOT_FOUND, null, "User not found"); } // Check if username is being changed to an already existing one @@ -102,7 +104,7 @@ public Response updateUser(RealmScimContext scimContext, String userId, fi.metat if (existing != null && !existing.getId().equals(userId)) { logger.warn(String.format("User name already taken: %s", updateRequest.getUserName())); - return Response.status(Response.Status.CONFLICT).entity("User name already taken").build(); + return ScimErrorResponse.scimError(Response.Status.CONFLICT, "uniqueness", "User name already taken"); } UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); @@ -119,7 +121,7 @@ public Response patchUser(RealmScimContext scimContext, String userId, fi.metata UserModel existing = session.users().getUserById(realm, userId); if (existing == null) { logger.warn(String.format("User not found: %s", userId)); - return Response.status(Response.Status.NOT_FOUND).entity("User not found").build(); + return ScimErrorResponse.scimError(Response.Status.NOT_FOUND, null, "User not found"); } UserAttributes userAttributes = metadataController.getUserAttributes(scimContext); @@ -128,7 +130,7 @@ public Response patchUser(RealmScimContext scimContext, String userId, fi.metata fi.metatavu.keycloak.scim.server.model.User result = usersController.patchUser(scimContext, userAttributes, existing, patchRequest); return Response.ok(result).build(); } catch (UnsupportedPatchOperation e) { - return Response.status(Response.Status.BAD_REQUEST).entity("Unsupported patch operation").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Unsupported patch operation"); } } @@ -173,7 +175,7 @@ public Response deleteUser(RealmScimContext scimContext, String userId) { RoleModel scimManagedRole = realm.getRole("scim-managed"); if (scimManagedRole != null && !user.hasRole(scimManagedRole)) { logger.warn(String.format("User is not SCIM-managed: %s", userId)); - return Response.status(Response.Status.FORBIDDEN).entity("User is not managed by SCIM").build(); + return ScimErrorResponse.scimError(Response.Status.FORBIDDEN, null, "User is not managed by SCIM"); } usersController.deleteUser(scimContext, user); @@ -187,7 +189,7 @@ public Response createGroup(RealmScimContext scimContext, fi.metatavu.keycloak.s if (isBlank(createRequest.getDisplayName())) { logger.warn("Cannot create group: Missing displayName"); - return Response.status(Response.Status.BAD_REQUEST).entity("Missing displayName").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, "invalidValue", "Missing displayName"); } fi.metatavu.keycloak.scim.server.model.Group created = groupsController.createGroup(scimContext, createRequest); @@ -225,7 +227,7 @@ public Response updateGroup(RealmScimContext scimContext, String id, fi.metatavu } if (!id.equals(existing.getId())) { - return Response.status(Response.Status.BAD_REQUEST).entity("Group ID mismatch").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Group ID mismatch"); } fi.metatavu.keycloak.scim.server.model.Group updated = groupsController.updateGroup( @@ -247,16 +249,16 @@ public Response patchGroup(RealmScimContext scimContext, String groupId, fi.meta } if (!groupId.equals(existing.getId())) { - return Response.status(Response.Status.BAD_REQUEST).entity("Group ID mismatch").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Group ID mismatch"); } try { fi.metatavu.keycloak.scim.server.model.Group updated = groupsController.patchGroup(scimContext, existing, patchRequest); return Response.ok(updated).build(); } catch (UnsupportedGroupPath e) { - return Response.status(Response.Status.BAD_REQUEST).entity("Unsupported group path").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Unsupported group path"); } catch (UnsupportedPatchOperation e) { - return Response.status(Response.Status.BAD_REQUEST).entity("Unsupported patch operation").build(); + return ScimErrorResponse.scimError(Response.Status.BAD_REQUEST, null, "Unsupported patch operation"); } }