From 7a3af328cd60cef98a133fb366caa3265a55849a Mon Sep 17 00:00:00 2001
From: Nicola <nicola.macoir@rightcrowd.com>
Date: Tue, 19 May 2026 11:41:03 +0200
Subject: [PATCH] fix: retain omitted name subattributes on PUT /Users
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

UsersController.updateUser wrote name.givenName and name.familyName
unconditionally whenever the parent "name" object was present in the
request body, so a payload with only one of the two subattributes
silently cleared the other.

Sibling top-level attributes (emails, additionalProperties like
displayName / preferredLanguage / externalId) are already gated and
correctly retained when omitted. Only the name.* subattributes were
written through with null. Per RFC 7644 §3.5.1, omitted readWrite
attributes MAY be assumed to be unchanged — applies to subattributes
too.

Guard each name.* write on a non-null subattribute value so the
behaviour matches the rest of the resource.

Adds a regression test that PUTs with only name.givenName and
asserts the existing name.familyName is retained.

Closes #105
---
 .../scim/server/users/UsersController.java    |  8 +++-
 .../functional/RealmUserUpdateTestsIT.java    | 43 +++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/main/java/fi/metatavu/keycloak/scim/server/users/UsersController.java b/src/main/java/fi/metatavu/keycloak/scim/server/users/UsersController.java
index 97aa439..6bfdc18 100644
--- a/src/main/java/fi/metatavu/keycloak/scim/server/users/UsersController.java
+++ b/src/main/java/fi/metatavu/keycloak/scim/server/users/UsersController.java
@@ -232,8 +232,12 @@ public fi.metatavu.keycloak.scim.server.model.User updateUser(
         ((BooleanUserAttribute) userAttributes.findByScimPath("active")).write(existing, scimUser.getActive() == null || Boolean.TRUE.equals(scimUser.getActive()));
 
         if (scimUser.getName() != null) {
-            ((StringUserAttribute) userAttributes.findByScimPath("name.givenName")).write(existing, scimUser.getName().getGivenName());
-            ((StringUserAttribute) userAttributes.findByScimPath("name.familyName")).write(existing, scimUser.getName().getFamilyName());
+            if (scimUser.getName().getGivenName() != null) {
+                ((StringUserAttribute) userAttributes.findByScimPath("name.givenName")).write(existing, scimUser.getName().getGivenName());
+            }
+            if (scimUser.getName().getFamilyName() != null) {
+                ((StringUserAttribute) userAttributes.findByScimPath("name.familyName")).write(existing, scimUser.getName().getFamilyName());
+            }
         }
 
         if (scimUser.getEmails() != null && !scimUser.getEmails().isEmpty()) {
diff --git a/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmUserUpdateTestsIT.java b/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmUserUpdateTestsIT.java
index 881f4f2..b01f48d 100644
--- a/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmUserUpdateTestsIT.java
+++ b/src/test/java/fi/metatavu/keycloak/scim/server/test/tests/functional/RealmUserUpdateTestsIT.java
@@ -5,6 +5,7 @@
 import fi.metatavu.keycloak.scim.server.test.TestConsts;
 import fi.metatavu.keycloak.scim.server.test.client.ApiException;
 import fi.metatavu.keycloak.scim.server.test.client.model.User;
+import fi.metatavu.keycloak.scim.server.test.client.model.UserName;
 import org.junit.jupiter.api.Test;
 import org.keycloak.events.admin.AdminEvent;
 import org.keycloak.events.admin.OperationType;
@@ -183,4 +184,46 @@ void testUpdateUserAdminEvents() throws ApiException, IOException {
         // Cleanup
         deleteRealmUser(TestConsts.TEST_REALM, created.getId());
     }
+
+    /**
+     * Regression: omitted "name.familyName" in a PUT body must retain the existing value.
+     * Previously the request unconditionally wrote null when the parent "name" object was
+     * present, clearing the existing family name. RFC 7644 §3.5.1 says omitted readWrite
+     * attributes MAY be assumed to be unchanged — applies to subattributes too.
+     */
+    @Test
+    void testReplaceUserRetainsOmittedNameSubattributes() throws ApiException {
+        ScimClient scimClient = getAuthenticatedScimClient();
+
+        User user = new User();
+        user.setUserName("omitted-name-sub");
+        user.setActive(true);
+        user.setSchemas(List.of("urn:ietf:params:scim:schemas:core:2.0:User"));
+        user.setName(getName("Original", "Lastname"));
+        user.setEmails(getEmails("omitted.name.sub@example.com"));
+
+        User created = scimClient.createUser(user);
+        String userId = created.getId();
+
+        // PUT with only givenName under name; familyName omitted
+        User replacement = new User();
+        replacement.setUserName(user.getUserName());
+        replacement.setActive(true);
+        replacement.setSchemas(List.of("urn:ietf:params:scim:schemas:core:2.0:User"));
+        UserName partialName = new UserName();
+        partialName.setGivenName("NewGiven");
+        replacement.setName(partialName);
+
+        User updated = scimClient.updateUser(userId, replacement);
+
+        assertNotNull(updated.getName());
+        assertEquals("NewGiven", updated.getName().getGivenName());
+        assertEquals("Lastname", updated.getName().getFamilyName(), "familyName must be retained when omitted in PUT");
+
+        UserRepresentation realmUser = findRealmUser(TestConsts.TEST_REALM, userId);
+        assertEquals("NewGiven", realmUser.getFirstName());
+        assertEquals("Lastname", realmUser.getLastName());
+
+        deleteRealmUser(TestConsts.TEST_REALM, userId);
+    }
 }
\ No newline at end of file