fix: retain omitted name subattributes on PUT /Users#106
Open
nicolamacoir wants to merge 2 commits into
Open
Conversation
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 Metatavu#105
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PUT /Users/{id}with a partialnameobject (e.g. onlygivenName) silently cleared the other subattribute (familyName), inconsistent with how other omitted readWrite attributes are handled.UsersController.updateUsernow guards eachname.*write on a non-null subattribute value, matching the existing handling foremailsandadditionalProperties.Changes
UsersController.java:234-241— only writename.givenName/name.familyNamewhen the corresponding value is non-null.RealmUserUpdateTestsIT.testReplaceUserRetainsOmittedNameSubattributes— regression test that creates a user with bothgivenNameandfamilyNameset, PUTs with onlygivenName, and asserts the existingfamilyNameis retained both in the SCIM response and in the underlying Keycloak user.Test plan
./gradlew test --tests "fi.metatavu.keycloak.scim.server.test.tests.functional.RealmUserUpdateTestsIT"passestestReplaceUser(full-payload PUT) still passes — partial-name handling does not regress the full-replace casenamePUT no longer clears the omitted subattributeNote on clearing values
This PR makes omitted
name.*subattributes "no change" rather than "clear", which is the consistent interpretation of the RFC. Clients that need to clear a name subattribute should use PATCHop: remove(the normative SCIM way to clear attributes), or send an empty string if the deployment allows it.