Captain-P-Goldfish / SCIM-SDK

a scim implementation as described in RFC7643 and RFC7644
https://github.com/Captain-P-Goldfish/SCIM/wiki
BSD 3-Clause "New" or "Revised" License
123 stars 37 forks source link

PUT/PATCH is missing validation for Group members with an empty object #747

Closed Afira-Zahari-i2 closed 1 month ago

Afira-Zahari-i2 commented 1 month ago

On a Group PATCH request, if the request body contains an empty object value for members path, e.g.

{
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:PatchOp"
    ],
    "Operations": [
        {
            "name": "addMember",
            "op": "add",
            "path": "members",
            "value": [
                {
                    "value": "123-456"
                },
                {}
            ]
        }
    ]
}

The response returned is a 500. When I had a look at the exception, I can see this as the cause

java.lang.NullPointerException: Cannot invoke "com.fasterxml.jackson.databind.JsonNode.isNull()" because "attribute" is null

Similarly on a Group PUT request,

{
   "schemas": [
      "urn:ietf:params:scim:schemas:core:2.0:Group"
   ],
   "displayName": "GroupDisplayName",
   "members": [
       {}
   ]
}

The same PUT request body used in a POST returns 400 with this

{
   "detail": "Required 'IMMUTABLE' attribute 'urn:ietf:params:scim:schemas:core:2.0:Group:members.value' must be set on object creation",
   "schemas": [
      "urn:ietf:params:scim:api:messages:2.0:Error"
   ],
   "status": 400,
   "errors": {
       "fieldErrors": {
           "members.value": [
               "Required 'IMMUTABLE' attribute 'urn:ietf:params:scim:schemas:core:2.0:Group:members.value' must be set on object creation"
           ],
           "members": [
               "Found unsupported value in multivalued complex attribute '[{}]'"
           ]
       }
   }
}
Captain-P-Goldfish commented 1 month ago

I am not able to reproduce the NullPointerException. Can you give me the full stackTrace?

But due to your report I found another troublesome behaviour that I just fixed:

I added a unit test for a second case for patch which was handled erroneously:

{
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:PatchOp"
    ],
    "Operations": [
        {
            "name": "addMember",
            "op": "add",
            "path": "members",
            "value": [
                {}
            ]
        }
    ]
}

This will cause a BadRequestException now:

Required sub-attribute 'urn:ietf:params:scim:schemas:core:2.0:Group:members.value' is missing in patch object.

because previously it was erroneously removing existing entries from the resource.

Captain-P-Goldfish commented 1 month ago

if you can provide me with the full stacktrace today, we can probably add the fix to the next release. I am going to provide the next release today.

Colvin-Cowie-i2 commented 1 month ago

Hello. I've had a look and we were still on 1.24, it looks like this was already fixed by https://github.com/Captain-P-Goldfish/SCIM-SDK/commit/676b04ca8a14a5ec3a7431c5e7894bd60a807acf and this can be closed. Sorry, thanks.