Support ADD patch ops targeting existing values

In the 3.0.0 release, we implemented support for ADD patch operations with a value filter. The SDK would always append a new value to the array, since it is technically an "add". However, this does not play well with SCIM provisioners that send multiple individual updates, since these intend to target the same value within a multi-valued attribute, such as a user's work email. The intention is to add a new value to the field that matches the path filter (e.g., "type eq \"work\""). This commit adds support for this behavior. The new behavior is available via an opt-in setting in a static boolean variable, PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY. To opt into this setting, set the value of this variable to false. As a result of this change, the version of the SCIM SDK has been updated to 3.2.0-SNAPSHOT. Reviewer: vyhhuang Reviewer: dougbulkley JiraIssue: DS-49194
pingidentity · Sep 19, 2024 · 6f74c28 · 6f74c28
1 parent 0c75698
commit 6f74c28
Show file tree

Hide file tree

Showing 9 changed files with 290 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,12 +2,40 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](https://semver.org/).
 
-## v3.1.1 - TBD
+## v3.2.0 - TBD
 Fixed an issue where `AndFilter.equals()` and `OrFilter.equals()` could incorrectly evaluate to
 true.
 
 Updated Jackson dependencies to 2.17.2.
 
+Added a property that allows ADD patch operations with value filters to target an existing value.
+For example, consider the following patch request. This request aims to add a `display` field on a
+user's work email.
+```json
+{
+  "schemas": [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
+  "Operations": [
+    {
+      "op": "add",
+      "path": "emails[type eq \"work\"].display",
+      "value": "apollo.j@example.com"
+    }
+  ]
+}
+```
+When the new behavior is configured, this operation will search the resource for an existing "work"
+email and add a `"display": "apollo.j@example.com"` field to that email. This behavior allows for
+better integration with SCIM provisioners that send individual requests such as
+`emails[type eq "work"].display` followed by `emails[type eq "work"].value`, which are intended to
+target the same email. To use this behavior, toggle the property by adding the following Java code
+in your application:
+```
+PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY = false;
+```
+The default value of `APPEND_NEW_PATCH_VALUES_PROPERTY` is `true`, which will always add a new
+value (i.e., email) on the multi-valued attribute instead of updating an existing value/email.
+This matches the behavior of the SDK since the 3.0.0 release.
+
 ## v3.1.0 - 2024-Jun-25
 Updated all classes within the UnboundID SCIM 2 SDK to utilize `@Nullable` and `@NotNull`
 annotations for all non-primitive input parameters, member variables, and return values. These

diff --git a/pom.xml b/pom.xml
@@ -18,7 +18,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.unboundid.product.scim2</groupId>
   <artifactId>scim2-parent</artifactId>
-  <version>3.1.1-SNAPSHOT</version>
+  <version>3.2.0-SNAPSHOT</version>
   <packaging>pom</packaging>
   <name>UnboundID SCIM2 SDK Parent</name>
   <description>

diff --git a/scim2-assembly/pom.xml b/scim2-assembly/pom.xml
@@ -19,7 +19,7 @@
   <parent>
     <artifactId>scim2-parent</artifactId>
     <groupId>com.unboundid.product.scim2</groupId>
-    <version>3.1.1-SNAPSHOT</version>
+    <version>3.2.0-SNAPSHOT</version>
   </parent>
   <artifactId>scim2-assembly</artifactId>
   <packaging>pom</packaging>

diff --git a/scim2-sdk-client/pom.xml b/scim2-sdk-client/pom.xml
@@ -19,7 +19,7 @@
     <parent>
         <artifactId>scim2-parent</artifactId>
         <groupId>com.unboundid.product.scim2</groupId>
-        <version>3.1.1-SNAPSHOT</version>
+        <version>3.2.0-SNAPSHOT</version>
     </parent>
     <artifactId>scim2-sdk-client</artifactId>
     <packaging>jar</packaging>

diff --git a/scim2-sdk-common/pom.xml b/scim2-sdk-common/pom.xml
@@ -19,7 +19,7 @@
     <parent>
         <artifactId>scim2-parent</artifactId>
         <groupId>com.unboundid.product.scim2</groupId>
-        <version>3.1.1-SNAPSHOT</version>
+        <version>3.2.0-SNAPSHOT</version>
     </parent>
     <artifactId>scim2-sdk-common</artifactId>
     <packaging>jar</packaging>

diff --git a/scim2-sdk-common/src/main/java/com/unboundid/scim2/common/messages/PatchOperation.java b/scim2-sdk-common/src/main/java/com/unboundid/scim2/common/messages/PatchOperation.java
@@ -42,6 +42,7 @@
 import com.unboundid.scim2.common.filters.EqualFilter;
 import com.unboundid.scim2.common.filters.Filter;
 import com.unboundid.scim2.common.filters.FilterType;
+import com.unboundid.scim2.common.utils.FilterEvaluator;
 import com.unboundid.scim2.common.utils.JsonUtils;
 import com.unboundid.scim2.common.utils.SchemaUtils;
 
@@ -215,7 +216,6 @@ public void apply(@NotNull final ObjectNode node) throws ScimException
       Path path = (getPath() == null) ? Path.root() : getPath();
       if (hasValueFilter(path))
       {
-        validateAddOpWithFilter(path, value);
         applyAddWithValueFilter(path, node, value);
       }
       else
@@ -377,8 +377,10 @@ private void applyAddWithValueFilter(
         @NotNull final Path path,
         @NotNull final ObjectNode existingResource,
         @NotNull final JsonNode value)
-            throws BadRequestException
+            throws ScimException
     {
+      validateAddOpWithFilter(path, value);
+
       Filter valueFilter = path.getElement(0).getValueFilter();
       String filterAttributeName = valueFilter.getAttributePath().toString();
       ValueNode filterValue = valueFilter.getComparisonValue();
@@ -391,8 +393,8 @@ private void applyAddWithValueFilter(
       JsonNode jsonAttribute = existingResource.get(attributeName);
       if (jsonAttribute == null)
       {
-        // There are no existing values for the attribute, so we should add this
-        // value ourselves.
+        // There are no existing values for the attribute, so we should
+        // prepare to add this value ourselves.
         jsonAttribute = JsonUtils.getJsonNodeFactory().arrayNode(1);
       }
       if (!jsonAttribute.isArray())
@@ -405,13 +407,101 @@ private void applyAddWithValueFilter(
       }
       ArrayNode attribute = (ArrayNode) jsonAttribute;
 
-      // Construct the new attribute value that should be added to the resource.
-      ObjectNode newValue = JsonUtils.getJsonNodeFactory().objectNode();
-      newValue.set(subAttributeName, value);
-      newValue.set(filterAttributeName, filterValue);
+      // When operations with a value filter add data, we can either append
+      // the data to a new value in the multi-valued attribute, or we can update
+      // an existing value.
+      //
+      // If the APPEND_NEW_PATCH_VALUES_PROPERTY property is enabled, the
+      // provided data should be added as a new value regardless of the existing
+      // resource's state (so we pretend that there are no matched values).
+      // Otherwise, any new data should update the existing value, if it is
+      // present.
+      ObjectNode matchedValue = null;
+      if (!APPEND_NEW_PATCH_VALUES_PROPERTY)
+      {
+        matchedValue = fetchExistingValue(attribute, valueFilter, attributeName);
+      }
+
+      // If there are no existing values that match the filter, or if no values
+      // were fetched, then we should add the two attribute values to the array.
+      // In the former case, this indicates that something like a home address
+      // does not yet exist on the resource, so a new one should be added.
+      if (matchedValue == null)
+      {
+        ObjectNode newValue = JsonUtils.getJsonNodeFactory().objectNode();
+        newValue.set(subAttributeName, value);
+        newValue.set(filterAttributeName, filterValue);
+
+        attribute.add(newValue);
+        existingResource.replace(attributeName, attribute);
+        return;
+      }
+
+      // Ensure that the data to be added is not already populated on the
+      // resource. For example, adding the "locality" sub-attribute to a home
+      // address should not be allowed if the home address already has a
+      // locality defined.
+      if (FilterEvaluator.evaluate(Filter.pr(subAttributeName), matchedValue))
+      {
+        throw BadRequestException.invalidValue(String.format(
+            "The add operation attempted to add a new '%s' field, but the"
+                + " specified path already has a '%s' defined.",
+            subAttributeName,
+            subAttributeName
+        ));
+      }
 
-      attribute.add(newValue);
-      existingResource.replace(attributeName, attribute);
+      matchedValue.set(subAttributeName, value);
+    }
+
+    /**
+     * Checks a multi-valued attribute for an existing value and returns a value
+     * based on the following conditions:
+     * <ul>
+     *   <li> If a single existing value is present, it will be returned.
+     *   <li> If no existing values are present, {@code null} will be returned.
+     *   <li> If multiple existing values are present, a
+     *        {@link BadRequestException} will be thrown, since it is unclear
+     *        which value should be targeted.
+     * </ul>
+     *
+     * @param attribute      The multi-valued attribute.
+     * @param valueFilter    The value selection filter provided with the patch
+     *                       add operation.
+     * @param attributeName  The name of {@code attribute}.
+     *
+     * @return  An ObjectNode representing the single value that matched the
+     *          criteria of the {@code valueFilter}, or {@code null} if no
+     *          attributes matched the filter.
+     *
+     * @throws ScimException  If there was an error processing the filter, or
+     *                        if multiple values were matched.
+     */
+    @Nullable
+    private static ObjectNode fetchExistingValue(
+        @NotNull final ArrayNode attribute,
+        @NotNull final Filter valueFilter,
+        @NotNull final String attributeName)
+          throws ScimException
+    {
+      ObjectNode matchedValue = null;
+
+      for (var arrayVal : attribute)
+      {
+        if (FilterEvaluator.evaluate(valueFilter, arrayVal))
+        {
+          if (matchedValue != null)
+          {
+            throw BadRequestException.noTarget(
+                "The operation could not be applied on the resource because the"
+                    + " value filter matched more than one element in the '"
+                    + attributeName + "' array of the resource.");
+          }
+          matchedValue = (ObjectNode) arrayVal;
+        }
+      }
+
+      return matchedValue;
     }
 
     /**
@@ -459,6 +549,7 @@ public int hashCode()
       result = 31 * result + value.hashCode();
       return result;
     }
+
   }
 
   static final class RemoveOperation extends PatchOperation
@@ -1948,7 +2039,7 @@ private static void validateOperationValue(@Nullable final Path path,
       throws ScimException
   {
     if (value == null || value.isNull() ||
-            (value.isObject() && value.size() == 0))
+            (value.isObject() && value.isEmpty()))
     {
       throw BadRequestException.invalidSyntax(
           "The patch operation value must not be null or an empty object");
@@ -1961,4 +2052,47 @@ private static void validateOperationValue(@Nullable final Path path,
                   + " attributes to " + type);
     }
   }
+
+  /**
+   * This field represents a property that can customize behavior when
+   * processing ADD PATCH operations with a value filter. This is used for
+   * multi-valued attributes such as {@code emails}, and is not used for
+   * {@code remove} or {@code replace} operations. For example, consider the
+   * following patch request:
+   * <pre>
+   *   {
+   *     "schemas": [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
+   *     "Operations": [
+   *       {
+   *         "op": "add",
+   *         "path": "emails[type eq \"work\"].display",
+   *         "value": "apollo.j@example.com"
+   *       }
+   *     ]
+   *   }
+   * </pre>
+   *
+   * When this property is enabled and the above patch request is applied, the
+   * following JSON will be appended to the {@code emails} of the user
+   * resource:
+   * <pre>
+   *   {
+   *     "type": "work",
+   *     "display": "apollo.j@example.com"
+   *   }
+   * </pre>
+   * Note that this value is added regardless of the current state of the
+   * resource, so enabling this property for PATCH requests has the potential
+   * to result in multiple emails containing a {@code type} of {@code work}.
+   * <br><br>
+   *
+   * If this property is <em>disabled</em>, then in the above example, the
+   * {@code display} field will be added to the resource's existing work email,
+   * if it exists. If the work email does not exist, then a new value will be
+   * appended. If there is already more than one existing work email on the
+   * resource, a {@link BadRequestException} will be thrown.
+   *
+   * @since 3.2.0
+   */
+  public static boolean APPEND_NEW_PATCH_VALUES_PROPERTY = true;
 }