fix(groups): accept path-less PATCH Operations per RFC 7644 §3.5.2

RFC 7644 §3.5.2.1 / §3.5.2.3 allow Operations with no "path" — in that
case "value" is a partial resource that should be merged into the
target. UsersController.patchUser already handles this branch; the
Group variant did not, returning "Unsupported group path" (400) for any
path-less Operation.

Okta's SCIM client uses this shape for group metadata reconciliation
immediately after each create:

    {"Operations":[{"op":"replace",
                    "value":{"id":"...","displayName":"..."}}]}

The 400 caused Okta to mark the entire group push as Error and stop
pushing group memberships, breaking provisioning end-to-end against
the OAuth Bearer Token variant of Okta's SCIM 2.0 Test App.

Mirror the path-less branch from UsersController:
- if path is null and value is a Map, iterate entries and apply each
- read-only / unknown attributes (id, schemas, meta, externalId, …)
  are skipped with a debug log rather than failing, matching the
  RFC's "extend gracefully" tone and matching Okta's real payloads
- the existing path-based switch is refactored into a private
  applyGroupPatchValue helper so both branches share it

Adds two regression tests in RealmGroupPatchTestsIT exercising the
happy path (displayName update) and the read-only-only edge case
(only id/schemas in value).

Validated end-to-end against a live Okta SCIM 2.0 Test App
(OAuth Bearer Token); after this fix, Okta proceeds to push group
memberships via PATCH op=add path=members value=[...] which the
existing handler accepts.

Author: zivisaiah

src/main/java/fi/metatavu/keycloak/scim/server/groups/GroupsController.java

@@ -171,8 +171,31 @@ public fi.metatavu.keycloak.scim.server.model.Group patchGroup(
                 throw new UnsupportedPatchOperation("Unsupported patch operation: " + operation.getOp());
             }
 
+            // RFC 7644 §3.5.2: when "path" is omitted, "value" carries a
+            // partial resource (map of attribute -> value) to apply to the
+            // target. Okta emits this for group metadata reconciliation:
+            // {"op":"replace","value":{"id":"...","displayName":"..."}}.
+            // Iterate the map and apply each known attribute; ignore
+            // read-only / unknown keys so spurious metadata fields like "id"
+            // don't fail the request.
+            if (path == null) {
+                if (!(value instanceof Map<?, ?> valueMap)) {
+                    throw new UnsupportedGroupPath("PatchOp without 'path' requires a map-valued 'value'");
+                }
+                for (Map.Entry<?, ?> entry : valueMap.entrySet()) {
+                    String attrName = String.valueOf(entry.getKey());
+                    GroupAttribute attr = GroupAttribute.findByScimPath(attrName);
+                    if (attr == null) {
+                        logger.debugf("Ignoring unsupported group attribute in path-less PATCH: %s", attrName);
+                        continue;
+                    }
+                    applyGroupPatchValue(scimContext, op, attr, null, existing, entry.getValue());
+                }
+                continue;
+            }
+
             // Extract base attribute path (e.g., "members" from "members[value eq \"id\"]")
-            String attributePath = path != null && path.contains("[")
+            String attributePath = path.contains("[")
                 ? path.substring(0, path.indexOf("["))
                 : path;
 
@@ -187,64 +210,78 @@ public fi.metatavu.keycloak.scim.server.model.Group patchGroup(
                 break;
             }
 
-            switch (op) {
-                case REPLACE, ADD -> {
-                    switch (groupAttribute) {
-                        case DISPLAY_NAME -> existing.setName((String) value);
-                        case MEMBERS -> {
-                            // Clear current members if REPLACE, just add if ADD
-                            if (op == PatchOperation.REPLACE) {
-                                session.users().getGroupMembersStream(realm, existing)
-                                    .forEach(user -> user.leaveGroup(existing));
-                            }
+            applyGroupPatchValue(scimContext, op, groupAttribute, path, existing, value);
+        }
 
-                            for (Object obj : (List<?>) value) {
-                                if (!(obj instanceof Map<?, ?> memberMap)) {
-                                    logger.warn("Invalid member object: " + obj);
-                                    continue;
-                                }
+        return translateGroup(scimContext, existing);
+    }
 
-                                String memberId = (String) memberMap.get("value");
-                                if (memberId == null) {
-                                    logger.warn("Member value missing: " + obj);
-                                    continue;
-                                }
+    /**
+     * Applies a single PATCH operation to a group attribute. Shared by the
+     * path-based and path-less PATCH branches so the membership and
+     * displayName mutation logic stays in one place.
+     */
+    private void applyGroupPatchValue(
+        ScimContext scimContext,
+        PatchOperation op,
+        GroupAttribute groupAttribute,
+        String path,
+        GroupModel existing,
+        Object value
+    ) {
+        KeycloakSession session = scimContext.getSession();
+        RealmModel realm = scimContext.getRealm();
 
-                                UserModel user = scimContext.getSession().users().getUserById(scimContext.getRealm(), memberId);
-                                if (user != null) {
-                                    user.joinGroup(existing);
-                                    dispatchGroupMembershipJoinEvent(scimContext, existing, user);
-                                }
+        switch (op) {
+            case REPLACE, ADD -> {
+                switch (groupAttribute) {
+                    case DISPLAY_NAME -> existing.setName((String) value);
+                    case MEMBERS -> {
+                        if (op == PatchOperation.REPLACE) {
+                            session.users().getGroupMembersStream(realm, existing)
+                                .forEach(user -> user.leaveGroup(existing));
+                        }
+                        for (Object obj : (List<?>) value) {
+                            if (!(obj instanceof Map<?, ?> memberMap)) {
+                                logger.warn("Invalid member object: " + obj);
+                                continue;
+                            }
+                            String memberId = (String) memberMap.get("value");
+                            if (memberId == null) {
+                                logger.warn("Member value missing: " + obj);
+                                continue;
+                            }
+                            UserModel user = session.users().getUserById(realm, memberId);
+                            if (user != null) {
+                                user.joinGroup(existing);
+                                dispatchGroupMembershipJoinEvent(scimContext, existing, user);
                             }
                         }
                     }
                 }
-
-                case REMOVE -> {
-                    switch (groupAttribute) {
-                        case DISPLAY_NAME -> existing.setName(null);
-                        case MEMBERS -> {
-                            // Handle path filter (e.g., "members[value eq \"user-id\"]")
-                            if (path != null && path.contains("[")) {
-                                String memberId = extractValueFromFilter(path);
-                                if (memberId != null) {
-                                    UserModel user = session.users().getUserById(realm, memberId);
-                                    if (user != null) {
-                                        user.leaveGroup(existing);
-                                        dispatchGroupMembershipLeaveEvent(scimContext, existing, user);
-                                    }
+            }
+            case REMOVE -> {
+                switch (groupAttribute) {
+                    case DISPLAY_NAME -> existing.setName(null);
+                    case MEMBERS -> {
+                        if (path != null && path.contains("[")) {
+                            String memberId = extractValueFromFilter(path);
+                            if (memberId != null) {
+                                UserModel user = session.users().getUserById(realm, memberId);
+                                if (user != null) {
+                                    user.leaveGroup(existing);
+                                    dispatchGroupMembershipLeaveEvent(scimContext, existing, user);
                                 }
-                            } else if (value instanceof List<?> list) {
-                                // Handle direct value list
-                                for (Object obj : list) {
-                                    if (obj instanceof Map<?, ?> memberMap) {
-                                        String memberId = (String) memberMap.get("value");
-                                        if (memberId != null) {
-                                            UserModel user = session.users().getUserById(realm, memberId);
-                                            if (user != null) {
-                                                user.leaveGroup(existing);
-                                                dispatchGroupMembershipLeaveEvent(scimContext, existing, user);
-                                            }
+                            }
+                        } else if (value instanceof List<?> list) {
+                            for (Object obj : list) {
+                                if (obj instanceof Map<?, ?> memberMap) {
+                                    String memberId = (String) memberMap.get("value");
+                                    if (memberId != null) {
+                                        UserModel user = session.users().getUserById(realm, memberId);
+                                        if (user != null) {
+                                            user.leaveGroup(existing);
+                                            dispatchGroupMembershipLeaveEvent(scimContext, existing, user);
                                         }
                                     }
                                 }
@@ -254,8 +291,6 @@ public fi.metatavu.keycloak.scim.server.model.Group patchGroup(
                 }
             }
         }
-
-        return translateGroup(scimContext, existing);
     }
 
     /**

src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmGroupPatchTestsIT.java

@@ -14,7 +14,9 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -256,4 +258,81 @@ void testRemoveGroupMemberWithoutEmail() throws ApiException {
         deleteRealmUser(TestConsts.TEST_REALM, user.getId());
         deleteRealmGroup(TestConsts.TEST_REALM, group.getId());
     }
+
+    /**
+     * Regression test for path-less PATCH on Groups per RFC 7644 §3.5.2.
+     *
+     * Okta's SCIM client refreshes group metadata after each push by sending
+     * a PATCH where the Operation omits "path" and supplies a partial
+     * resource as "value":
+     *
+     *   {"op":"replace","value":{"id":"...","displayName":"..."}}
+     *
+     * Before this fix, GroupsController#patchGroup rejected such Operations
+     * with `UnsupportedGroupPath`, which Okta interpreted as a group-push
+     * failure and stopped pushing memberships. The fix mirrors the
+     * path-less branch already present in UsersController#patchUser:
+     * iterate the value map, apply each known attribute, and ignore
+     * unknown / read-only keys (id, schemas, meta, externalId) without
+     * failing.
+     */
+    @Test
+    void testPatchGroupWithoutPathReplacesDisplayName() throws ApiException {
+        ScimClient scimClient = getAuthenticatedScimClient();
+        Group group = createGroup(scimClient, "pathless-original-name");
+
+        PatchRequest patchRequest = new PatchRequest();
+        patchRequest.setSchemas(List.of("urn:ietf:params:scim:api:messages:2.0:PatchOp"));
+
+        PatchRequestOperationsInner operation = new PatchRequestOperationsInner();
+        operation.setOp("replace");
+        // path intentionally omitted to exercise the path-less branch.
+
+        Map<String, Object> partialResource = new HashMap<>();
+        partialResource.put("id", group.getId());                   // read-only, must be ignored
+        partialResource.put("displayName", "pathless-updated-name"); // must be applied
+        operation.setValue(partialResource);
+
+        patchRequest.setOperations(List.of(operation));
+
+        Group patched = scimClient.patchGroup(group.getId(), patchRequest);
+
+        assertEquals("pathless-updated-name", patched.getDisplayName());
+
+        deleteRealmGroup(TestConsts.TEST_REALM, group.getId());
+    }
+
+    /**
+     * Path-less PATCH where the value map contains only attributes the
+     * server treats as read-only / unknown (id, schemas) must succeed
+     * silently — applying nothing — rather than throwing
+     * UnsupportedGroupPath. This mirrors Okta's idempotent metadata
+     * reconciliation calls where the server returns the same data Okta
+     * just pushed.
+     */
+    @Test
+    void testPatchGroupWithoutPathIgnoresReadOnlyAttributes() throws ApiException {
+        ScimClient scimClient = getAuthenticatedScimClient();
+        Group group = createGroup(scimClient, "pathless-readonly-name");
+        String originalDisplayName = group.getDisplayName();
+
+        PatchRequest patchRequest = new PatchRequest();
+        patchRequest.setSchemas(List.of("urn:ietf:params:scim:api:messages:2.0:PatchOp"));
+
+        PatchRequestOperationsInner operation = new PatchRequestOperationsInner();
+        operation.setOp("replace");
+
+        Map<String, Object> partialResource = new HashMap<>();
+        partialResource.put("id", group.getId());
+        partialResource.put("schemas", List.of("urn:ietf:params:scim:schemas:core:2.0:Group"));
+        operation.setValue(partialResource);
+
+        patchRequest.setOperations(List.of(operation));
+
+        Group patched = scimClient.patchGroup(group.getId(), patchRequest);
+
+        assertEquals(originalDisplayName, patched.getDisplayName());
+
+        deleteRealmGroup(TestConsts.TEST_REALM, group.getId());
+    }
 }