Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Add PatchOps with the same filter (i.e. address[type="work"]) generate multiple JsonAttributes instead of updating existing attribute #213

Closed
egorksv opened this issue Nov 12, 2023 · 3 comments · Fixed by #230
Assignees

Comments

@egorksv
Copy link

egorksv commented Nov 12, 2023

Describe the bug
When multiple Patch Add operations are issued for the same compound object in the scope of the same PatchRequest, AddOperation creates a separate record for each property.

Important node: We are building SCIM Server, so the PatchRequest is deserialised from external system (Entra ID in our case)

To Reproduce
Using the code from the documentation and standard User resource:

        ScimUserResource scimUserResource = patchUserMapper.userToScimUserResource(existingUser);
        GenericScimResource gsr = scimUserResource.asGenericScimResource();
        patchRequest.apply(gsr);
        ObjectNode objectNode = gsr.getObjectNode();
        scimUserResource = JsonUtils.nodeToValue(objectNode, ScimUserResource.class);

... with this PatchOp request:

{
  "schemas" : [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
  "Operations" : [ {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].streetAddress",
    "value" : "User Street Address"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"home\"].streetAddress",
    "value" : "User Street Address"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].locality",
    "value" : "User City"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].region",
    "value" : "User Country"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"home\"].region",
    "value" : "User Country"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].postalCode",
    "value" : "55555"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].country",
    "value" : "User Country"
  }, {
    "op" : "replace",
    "path" : "phoneNumbers[type eq \"work\"].value",
    "value" : "16176807200"
  }, {
    "op" : "replace",
    "path" : "phoneNumbers[type eq \"mobile\"].value",
    "value" : "16176809900"
  }, {
    "op" : "replace",
    "path" : "urn:ietf:params:scim:schemas:extension:acea:2.0:User:joinDate",
    "value" : "2023-06-30T20:00:00Z"
  }, {
    "op" : "add",
    "path" : "urn:ietf:params:scim:schemas:extension:acea:2.0:User:userDOB",
    "value" : "2023-06-30T20:00:00Z"
  } ]
}

The following result is achieved (skipped the irrelevant parts of User resource):

{
  "addresses": [
    {
      "formatted": null,
      "streetAddress": "User Street Address",
      "locality": null,
      "region": null,
      "postalCode": null,
      "country": null,
      "type": "work",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": "User Street Address",
      "locality": null,
      "region": null,
      "postalCode": null,
      "country": null,
      "type": "home",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": null,
      "locality": "User City",
      "region": null,
      "postalCode": null,
      "country": null,
      "type": "work",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": null,
      "locality": null,
      "region": "User Country",
      "postalCode": null,
      "country": null,
      "type": "work",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": null,
      "locality": null,
      "region": "User Country",
      "postalCode": null,
      "country": null,
      "type": "home",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": null,
      "locality": null,
      "region": null,
      "postalCode": "55555",
      "country": null,
      "type": "work",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": null,
      "locality": null,
      "region": null,
      "postalCode": null,
      "country": "User Country",
      "type": "work",
      "primary": null
    }
  ]
}

Expected behavior
The code is expected to create only two Address records, one [type="work"] and one [type="home"]:

{
  "addresses": [
    {
      "formatted": null,
      "streetAddress": "User Street Address",
      "locality": "User City",
      "region": "User Country",
      "postalCode": "55555",
      "country": "User Country",
      "type": "work",
      "primary": null
    },
    {
      "formatted": null,
      "streetAddress": "User Street Address",
      "locality": null,
      "region": "User Country",
      "postalCode": null,
      "country": null,
      "type": "home",
      "primary": null
    }
  ]
}

Additional context
Add any other context about the problem here. For example:

  • Java 11
  • SCIM 2 SDK version: 3.0.0

Proposed Solution
I have copied and patched PatchRequest/PatchOperation classes into our code base to provide immediate fix. Please advise if pull request is welcome.

        private void applyAddWithValueFilter(
                @NotNull final Path path,
                @NotNull final ObjectNode existingResource,
                @NotNull final JsonNode value)
                throws BadRequestException {
            Filter valueFilter = path.getElement(0).getValueFilter();
            String filterAttributeName = valueFilter.getAttributePath().toString();
            ValueNode filterValue = valueFilter.getComparisonValue();

            // For an attribute path of the form 'emails[...].value', fetch the
            // attribute (emails) and the sub-attribute (value).
            String attributeName = path.getElement(0).getAttribute();
            String subAttributeName = path.getElement(1).getAttribute();

            JsonNode jsonAttribute = existingResource.get(attributeName);
            if (jsonAttribute == null) {
                // There are no existing values for the attribute, so we should add this
                // value ourselves.
                jsonAttribute = JsonUtils.getJsonNodeFactory().arrayNode(1);
            }
            if (!jsonAttribute.isArray()) {
                throw BadRequestException.invalidSyntax(
                        "The patch operation could not be processed because a complex"
                                + " value selection filter was provided, but '" + attributeName
                                + "' is single-valued"
                );
            }
            ArrayNode attribute = (ArrayNode) jsonAttribute;

//*** Changes begin here
            ObjectNode valueToSet = null;
            for (JsonNode existingObject : attribute) {
                if (filterValue.equals(existingObject.get(filterAttributeName))) {
                    // Actually locate the existing attribute using the filter value.
                    valueToSet = (ObjectNode) existingObject;
                    break;
                }
            }
            if (valueToSet == null) {
                // Construct the new attribute value if it does not already exist.
                valueToSet = JsonUtils.getJsonNodeFactory().objectNode();
                attribute.add(valueToSet);
                existingResource.replace(attributeName, attribute);
            }
            valueToSet.set(subAttributeName, value);
            valueToSet.set(filterAttributeName, filterValue);
        }
@kqarryzada kqarryzada self-assigned this Apr 26, 2024
@kqarryzada
Copy link
Collaborator

I have some preliminary changes for this issue, but they will require more testing before we can release this fix.

@kqarryzada
Copy link
Collaborator

@egorksv, this behavior will be available in the next release of the SCIM SDK. To opt into this feature, set the following value in your code:

PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY = false;

@kqarryzada
Copy link
Collaborator

This feature is now available in the 3.2.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants