PatchOperation$AddOperation instance creation fails on path values with selection filters

[KoichiSenada]

Describe the bug PatchOperation.AddOperation instance creation fails on path values with selection filters. https://github.com/pingidentity/scim2/commit/4b3d63c7c22ec044f5da4bb14848ac1366a3fa13#r35642158

com.unboundid.scim2.common.exceptions.BadRequestException: path field for add operations must not include any value selection filters
    at com.unboundid.scim2.common.exceptions.BadRequestException.invalidPath(BadRequestException.java:197)
    at com.unboundid.scim2.common.messages.PatchOperation$AddOperation.<init>(PatchOperation.java:101)

To Reproduce Send a SCIM2 protocol https://tools.ietf.org/html/rfc7644#section-3.5.2 compliant PATCH request.

{
  "schemas" : [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
  "Operations" : [ {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].country",
    "value" : "Australia"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].locality",
    "value" : "Australia"
  }, {
    "op" : "add",
    "path" : "addresses[type eq \"work\"].formatted",
    "value" : "Australia"
  } ]
}

Expected behavior It is expected that the PatchOperation$AddOperation instance would be created. https://tools.ietf.org/html/rfc7644#section-3.5.2

   Valid examples of "path" are as follows:

       "path":"members"

       "path":"name.familyName"

       "path":"addresses[type eq \"work\"]"

       "path":"members[value eq
              \"2819c223-7f76-453a-919d-413861904646\"]"

       "path":"members[value eq
              \"2819c223-7f76-453a-919d-413861904646\"].displayName"

[Stummi]

We seem to encounter the same issue when trying to integrate AD.

Ironically, its just the AddOperation Constructor which plainly refuses any path with a value selection filter. Ther underlaying tools can work with the filter without issues:

val body = """
    {
        "addresses": [
        {
            "type": "work",
            "locality": "old"
        }
        ]
    }
""".trimIndent()

val om = ObjectMapper()
val node: ObjectNode = om.readValue(body, ObjectNode::class.java)
val newAddr: JsonNode = om.readValue("\"new\"", JsonNode::class.java)
JsonUtils.addValue(Path.fromString("addresses[type eq \"work\"].locality"), node, newAddr)

println(node)

so the solution would just be removing the check in the AddOperation constructor

[davidmarne-wf]

@braveulysses any chance at getting fix in here, or at least an answer as to why the json creator for AddOperation has this clause?

[braveulysses]

@lance-purple-unboundid, can you please look into this issue?

[dlevesque-primalogik]

Any update on this? We also have this issue with AzureAD. Has anyone found a workaround?

[dahuber-github]

I resolved this by editing the PatchOperation class directly. I made the following method changes:

@JsonCreator
private AddOperation(
    @JsonProperty(value = "path") final Path path,
    @JsonProperty(value = "value", required = true) final JsonNode value)
    throws ScimException
{
  super(path);
  if(value == null || value.isNull() ||
       ((value.isArray() || value.isObject()) && value.size() == 0))
   {
     throw BadRequestException.invalidSyntax(
         "value field must not be null or an empty container");
   }
  if(path == null && !value.isObject())
  {
    throw BadRequestException.invalidSyntax(
        "value field must be a JSON object containing the attributes to " +
            "add");
  }
  this.value = value;
}

@Override
public void apply(final ObjectNode node) throws ScimException
{      
    if(getPath() != null)
    {
      for (Path.Element element : getPath())
      {
        if(element.getValueFilter() != null)
        {
        	JsonNode fieldNode = node.path(element.getAttribute());

        	ArrayNode arrayNode = null;
        	if (fieldNode.isMissingNode())
        		arrayNode = node.withArray(element.getAttribute());
        	else
        		arrayNode = (ArrayNode) fieldNode;
        	
    		String strType = element.getValueFilter().getAttributePath().toString();
    		String strValue = element.getValueFilter().getComparisonValue().asText();
    		        		
    		ObjectNode newNode = null;
    		for (Iterator<JsonNode> iterator = arrayNode.elements(); iterator.hasNext();)
    		{
    			JsonNode childNode = iterator.next();
    			if ((childNode.get(strType) != null) && (childNode.get(strType).asText().equals(strValue)))
    			{
    				newNode = (ObjectNode) childNode;
    				break;
    			}
    		}

    		if (newNode == null)
    			newNode = arrayNode.addObject();
    		
    		newNode.put(strType, strValue);        		
        }
      }
    }

  JsonUtils.addValue(getPath() == null ? Path.root() : getPath(), node, value);
  addMissingSchemaUrns(node);
}

I've tested these changes against Microsoft Azure and it seems to work. I'm sure there is a better way to do this. I provide this in hopes it helps the development team.

[davidmarne-wf]

@braveulysses @lance-purple-unboundid any updates?