Skip to content

Commit

Permalink
Support ADD patch ops targeting existing values
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@kqarryzada
kqarryzada committed Sep 19, 2024
1 parent f7e6150 commit a33e2a1
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 42 deletions.
23 changes: 22 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,33 @@
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, for the following patch request:
```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 `PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY` property is set to `false`, this
operation will search the resource for an existing "work" email and add the value to that email.
This 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. The default value of `PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY` is
`true`, which will always create a new value on the multi-valued attribute.

## 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
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion scim2-assembly/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion scim2-sdk-client/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion scim2-sdk-common/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
156 changes: 145 additions & 11 deletions scim2-sdk-common/src/main/java/com/unboundid/scim2/common/messages/PatchOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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())
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -459,6 +549,7 @@ public int hashCode()
result = 31 * result + value.hashCode();
return result;
}

}

static final class RemoveOperation extends PatchOperation
Expand Down Expand Up @@ -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");
Expand All @@ -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;
}
Loading

0 comments on commit a33e2a1

Please sign in to comment.