pingidentity / scim2

The UnboundID SCIM 2.0 SDK for Java
181 stars 74 forks source link

patchOp.apply puts added enterprise fields under wrong path #104

Closed oscarrobinson closed 5 years ago

oscarrobinson commented 5 years ago

When applying a patch add operation for an enterprise field (namely department), patchOp.apply puts the added department as a field in an object under they key "User" in an object which sits under the key "urn:ietf:params:scim:schemas:extension:enterprise:2.0". The department should actually be added as a field in an object under the key "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User".

For example say we have a user resource:

{
  "schemas" : [ "urn:ietf:params:scim:schemas:core:2.0:User", "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" ],
  "externalId" : "externalId-1",
  "meta" : {
    "resourceType" : "User"
  },
  "userName" : "user1",
  "name" : {
    "familyName" : "Young",
    "givenName" : "Joy"
  },
  "displayName" : "Joy Young",
  "active" : true,
  "emails" : [ {
    "value" : "user@org.com",
    "type" : "work",
    "primary" : true
  }, {
    "value" : "user@org-proxy.com",
    "type" : "work",
    "primary" : false
  } ],
  "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" : {
    "department" : null,
    "manager" : null
  }
}

This is parsed into a UserResource object.

We then parse the following patch operation:

{
  "schemas" : [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
  "Operations" : [ {
    "op" : "add",
    "path" : "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User.department",
    "value" : "Department A"
  }, {
    "op" : "replace",
    "path" : "name.familyName",
    "value" : "Smith"
  } ]
}

We turn the UserResource into a generic scim resource by calling asGenericScimResource() on it. We then apply the patch operation by passing the generic scim resource to the patch op's apply method.

The result of this is the following:

{
  "schemas" : [ "urn:ietf:params:scim:schemas:core:2.0:User", "urn:ietf:params:scim:schemas:extension:enterprise:2.0", "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" ],
  "externalId" : "externalId-1",
  "meta" : {
    "resourceType" : "User"
  },
  "userName" : "user1",
  "name" : {
    "familyName" : "Smith",
    "givenName" : "Joy"
  },
  "displayName" : "Joy Young",
  "active" : true,
  "emails" : [ {
    "value" : "user@org.com",
    "type" : "work",
    "primary" : true
  }, {
    "value" : "user@org-proxy.com",
    "type" : "work",
    "primary" : false
  } ],
  "urn:ietf:params:scim:schemas:extension:enterprise:2.0" : {
    "User" : {
      "department" : "Department A"
    }
  },
  "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" : {
    "department" : null,
    "manager" : null
  }
}

As you can see, the department under the proper scim structure remains as null. An erroneous "urn:ietf:params:scim:schemas:extension:enterprise:2.0" field has been added with the new department under a key "User". This appears to be incorrect behaviour.

gregcIndy commented 5 years ago

Seeing the same issue.

JasonMathison commented 5 years ago

@scarrobin Greg and I spent time diving into this issue, and we are now thinking that the library may be working per the RFC. The key seemed to be that with a URN the attributes are referenced like urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department (note colon before department) instead of urn:ietf:params:scim:schemas:extension:enterprise:2.0:User.department.

Looking at page 34 and 35 of https://tools.ietf.org/html/rfc7644#section-3.5.2 the URN examples use a colon instead of a dot.

@scarrobin is your failing patch example coming from a patch request from a production SCIM system or is it an example that you came up with? We are still working through our initial implementation so we don't have much in the way of production patch examples.

oscarrobinson commented 5 years ago

Thanks for your reply @JasonMathison.

The patch operation was not directly from a production system. It is the result of correcting an off-spec patch operation from Microsoft AzureAD. AzureAD entirely omits the urn:ietf:params:scim:schemas:extension:enterprise:2.0:User prefix from the path in department patch operations so we have to manually add it before processing the patch.

I can confirm that when I use a path urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department in the patch operation it gives a correct output under the enterprise user field. Evidently I misinterpreted the spec when fixing AzureAD's off-spec patch operations, thanks for pointing me in the direction of an example in the RFC.