scim2 icon indicating copy to clipboard operation
scim2 copied to clipboard

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

Open egorksv opened this issue 1 year ago • 1 comments

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);
        }

egorksv avatar Nov 12 '23 15:11 egorksv