Skip to content

Commit

Permalink
Add MsAzure workaround for illegal value-subAttributes in PatchRequests
Browse files Browse the repository at this point in the history
fixes #516
  • Loading branch information
Captain-P-Goldfish committed Oct 7, 2023
1 parent 03ed5eb commit 67da341
Show file tree
Hide file tree
Showing 10 changed files with 629 additions and 4 deletions.
2 changes: 1 addition & 1 deletion scim-sdk-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
<file>
${project.basedir}/src/main/resources/de/captaingoldfish/scim/sdk/common/meta/service-provider.schema.json
</file>
<checksum>26f37caa5c5d9bd817211da114636b40</checksum>
<checksum>fae587470a4e58531b319f7c2edcbddb</checksum>
<type>md5</type>
</requireFileChecksum>
<requireFileChecksum>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,14 @@ public static class Custom
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/416
*/
public static final String ACTIVATE_MS_AZURE_FILTER_WORKAROUND = "activateMsAzureFilterWorkaround";

/**
* A workaround indicator to handle MsAzures illegal value-subattribute notation. There might be cases in
* which this switch will cause with normal execution. That is why this must be activated explicitly.
*
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/516
*/
public static final String ACTIVATE_MS_AZURE_VALUE_SUB_ATTRIBUTE_WORKAROUND = "activateMsAzureValueSubAttributeWorkaround";
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ public class PatchConfig extends ScimObjectNode
{

@Builder
public PatchConfig(Boolean supported, Boolean activateSailsPointWorkaround, Boolean activateMsAzureWorkaround)
public PatchConfig(Boolean supported,
Boolean activateSailsPointWorkaround,
Boolean activateMsAzureWorkaround,
Boolean activateMsAzureValueSubAttributeWorkaround)
{
super(null);
setSupported(supported);
Expand Down Expand Up @@ -67,6 +70,7 @@ public void setActivateSailsPointWorkaround(Boolean activateSailsPointWorkaround
* A workaround to handle filter-expressions in patch-paths as attributes that will be added to the resource
*
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/416
* @see de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchFilterWorkaround
*/
public boolean isMsAzureFilterWorkaroundActive()
{
Expand All @@ -77,12 +81,36 @@ public boolean isMsAzureFilterWorkaroundActive()
* A workaround to handle filter-expressions in patch-paths as attributes that will be added to the resource
*
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/416
* @see de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchFilterWorkaround
*/
public void setMsAzureFilterWorkaroundActive(Boolean msAzureWorkaroundActive)
{
setAttribute(AttributeNames.Custom.ACTIVATE_MS_AZURE_FILTER_WORKAROUND, msAzureWorkaroundActive);
}

/**
* A workaround to handle MsAzures illegal value-subattribute notation
*
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/516
* @see de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchValueSubAttributeRebuilder
*/
public boolean isMsAzureValueSubAttributeWorkaroundActive()
{
return getBooleanAttribute(AttributeNames.Custom.ACTIVATE_MS_AZURE_VALUE_SUB_ATTRIBUTE_WORKAROUND).orElse(false);
}

/**
* A workaround to handle MsAzures illegal value-subattribute notation
*
* @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/516
* @see de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchValueSubAttributeRebuilder
*/
public void setMsAzureValueSubAttributeWorkaroundActive(Boolean msAzureValueSubAttributeWorkaroundActive)
{
setAttribute(AttributeNames.Custom.ACTIVATE_MS_AZURE_VALUE_SUB_ATTRIBUTE_WORKAROUND,
msAzureValueSubAttributeWorkaroundActive);
}

/**
* override lombok builder with public constructor
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,18 @@
{
"name": "activateMsAzureFilterWorkaround",
"type": "boolean",
"description": "A workaround to handle filter-expressions in patch-paths as attributes that will be added to the resource.",
"description": "A workaround to handle filter-expressions in patch-paths as attributes that will be added to the resource. @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/416",
"mutability": "readOnly",
"returned": "request",
"uniqueness": "none",
"multiValued": false,
"required": false,
"caseExact": false
},
{
"name": "activateMsAzureValueSubAttributeWorkaround",
"type": "boolean",
"description": "A workaround to handle MsAzures illegal value-subattribute notation. @see https://github.com/Captain-P-Goldfish/SCIM-SDK/issues/516",
"mutability": "readOnly",
"returned": "request",
"uniqueness": "none",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import de.captaingoldfish.scim.sdk.server.filter.AttributePathRoot;
import de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchAttributeRebuilder;
import de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchRemoveRebuilder;
import de.captaingoldfish.scim.sdk.server.patch.msazure.MsAzurePatchValueSubAttributeRebuilder;
import de.captaingoldfish.scim.sdk.server.schemas.ResourceType;
import de.captaingoldfish.scim.sdk.server.utils.RequestUtils;
import lombok.Getter;
Expand Down Expand Up @@ -166,6 +167,14 @@ private boolean handlePatchOp(ResourceNode resource, PatchRequestOperation opera
values);
path = msAzureWorkaround.fixPath();
}

if (patchConfig.isMsAzureValueSubAttributeWorkaroundActive())
{
MsAzurePatchValueSubAttributeRebuilder msAzureWorkaround;
msAzureWorkaround = new MsAzurePatchValueSubAttributeRebuilder(operation.getOp(), values);
values = msAzureWorkaround.fixValues();
}

PatchTargetHandler patchTargetHandler = new PatchTargetHandler(patchConfig, resourceType, operation.getOp(),
path);
boolean changeWasMade = patchTargetHandler.handleOperationValues(resource, values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* }
* </pre>
*
* the value in the request must not be present. Instead the request should look like this:
* the value in the request must not be present. Instead, the request should look like this:
*
* <pre>
* PATCH /scim/Users/2752513
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,61 @@
public class MsAzurePatchFilterWorkaround
{

/**
* this method is a workaround for MsAzures expected behaviour that a value is added to a multivalued complex
* node if it is reference in a patch-filter but not added within the attribute itself
*
* <pre>
* {
* "schemas": [
* "urn:ietf:params:scim:api:messages:2.0:PatchOp"
* ],
* "Operations": [
* {
* "op": "add",
* "path": "emails[type eq \"work\"].value",
* "value": "[email protected]"
* }
* ]
* }
* </pre>
*
* must be rebuilt to:
*
* <pre>
* {
* "schemas": [
* "urn:ietf:params:scim:api:messages:2.0:PatchOp"
* ],
* "Operations": [
* {
* "op": "add",
* "path": "emails[type eq \"work\"].value",
* "value": [
* {
* "value": "[email protected]"
* },
* {
* "type": "work"
* }
* ]
* }
* ]
* }
* </pre>
*
* The complete rebuild is not done within this method. This method will simply create the last objectNode
* from the filter-expression:
*
* <pre>
* {
* "type": "work"
* }
* </pre>
*
* @param path the patch filter expression node.
* @return the
*/
public ScimObjectNode createAttributeFromPatchFilter(AttributePathRoot path)
{
SchemaAttribute schemaAttribute = path.getSchemaAttribute();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
package de.captaingoldfish.scim.sdk.server.patch.msazure;

import java.util.ArrayList;
import java.util.List;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;

import de.captaingoldfish.scim.sdk.common.constants.AttributeNames;
import de.captaingoldfish.scim.sdk.common.constants.enums.PatchOp;
import de.captaingoldfish.scim.sdk.common.utils.JsonHelper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;


/**
* This class is a workaround handler in order to handle the broken patch requests of Microsoft Azure. Azure
* sends illegal patch requests that look like this:
*
* <pre>
* {
* "op": "Add",
* "path": "roles",
* "value": [
* {
* "value": "{\"id\":\"827f0d2e-be15-4d8f-a8e3-f8697239c112\",
* \"value\":\"DocumentMgmt-Admin\",
* \"displayName\":\"DocumentMgmt Admin\"}"
* },
* {
* "value": "{\"id\":\"8ae06bd4-35bb-4fcd-977e-14e074ad1192\",
* \"value\":\"Admin\",
* \"displayName\":\"Admin\"}"
* }
* ]
* }
* </pre>
*
* The problem in this request is the nested value-attribute that should not be there. Instead, the request
* should look like this:
*
* <pre>
* {
* "op": "Add",
* "path": "roles",
* "value": [
*
* "{\"id\":\"827f0d2e-be15-4d8f-a8e3-f7697239c112\",
* \"value\":\"DocumentMgmt-BuyerAdmin\",
* \"displayName\":\"DocumentMgmt BuyerAdmin\"}",
*
* "{\"id\":\"8ae06bd4-35bb-4fcd-977e-12e074ad1192\",
* \"value\":\"Buyer-Admin\",
* \"displayName\":\"Buyer Admin\"}"
* ]
* }
* </pre>
*
* @author Pascal Knueppel
* @since 07.10.2023
*/
@Slf4j
@RequiredArgsConstructor
public class MsAzurePatchValueSubAttributeRebuilder
{

/**
* the patch operation that is currently executed
*/
private final PatchOp patchOp;

/**
* the values of the patch operation. This attribute should actually be empty
*/
private final List<String> patchValues;

/**
* this method will try to resolve a PatchRequest as described in the class-documentation to its correct state
*
* @param patchValues the values sent in the PatchRequest
* @return the fixed values from the PatchRequest
*/
public List<String> fixValues()
{
// simply a check to return early from this method if a remove operation is used
if (PatchOp.REMOVE.equals(patchOp))
{
log.trace("[MS Azure value-subAttribute workaround] only handling 'REPLACE' and 'ADD' requests");
return patchValues;
}

if (patchValues == null || patchValues.isEmpty())
{
log.trace("[MS Azure value-subAttribute workaround] not executed for values-list is empty");
return patchValues;
}

List<String> fixedValues = new ArrayList<>();
for ( String patchValue : patchValues )
{
final JsonNode jsonNode;
try
{
jsonNode = JsonHelper.readJsonDocument(patchValue);
}
catch (Exception ex)
{
log.trace("[MS Azure value-subAttribute workaround] ignored value-node because it is no valid JSON "
+ "object-node");
fixedValues.add(patchValue);
continue;
}

boolean isObjectNode = jsonNode instanceof ObjectNode;
if (!isObjectNode)
{
log.trace("[MS Azure value-subAttribute workaround] ignored value because it is no JSON-ObjectNode");
fixedValues.add(patchValue);
continue;
}

ObjectNode objectNode = (ObjectNode)jsonNode;
if (objectNode.size() != 1)
{
log.trace("[MS Azure value-subAttribute workaround] ignored JSON-ObjectNode because it has less or more "
+ "than 1 sub-nodes");
fixedValues.add(patchValue);
continue;
}

JsonNode innerValueNode = objectNode.get(AttributeNames.RFC7643.VALUE);
if (innerValueNode == null)
{
log.trace("[MS Azure value-subAttribute workaround] ignored JSON-ObjectNode because it has no value-node");
fixedValues.add(patchValue);
continue;
}

final JsonNode innerObjectNode;
try
{
innerObjectNode = JsonHelper.readJsonDocument(innerValueNode.textValue());
}
catch (Exception ex)
{
log.trace("[MS Azure value-subAttribute workaround] ignored inner value-node because it is no valid JSON-node");
fixedValues.add(patchValue);
continue;
}

isObjectNode = innerObjectNode instanceof ObjectNode;
if (!isObjectNode)
{
log.trace("[MS Azure value-subAttribute workaround] ignored inner value-node because it is no not a JSON "
+ "object-node");
fixedValues.add(patchValue);
continue;
}

fixedValues.add(innerObjectNode.toString());
}

return fixedValues;
}
}
Loading

0 comments on commit 67da341

Please sign in to comment.