Issue with test -- Group patch add member

[plamenGo]

The test in question submits the following payload to /Groups PATCH

{
	"id": "{{1stgroupid}}",
    "Operations": [
        {
         "op":"add", 
         "path": "members", 
         "value": "string id 1"
        }
    ],
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:PatchOp"
    ]
}

However, I don't think value can be a string, it should probably be like this example from the RFC: https://tools.ietf.org/html/rfc7644

The following example shows how to add a member to a group. Some text was removed for readability (indicated by "..."):

PATCH /Groups/acbf3ae7-8463-...-9b4da3f908ce Host: example.com Accept: application/scim+json Content-Type: application/scim+json Authorization: Bearer h480djs93hd8 If-Match: W/"a330bc54f0671c9"

{ "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations":[ { "op":"add", "path":"members", "value":[ { "display": "Babs Jensen", "$ref": "https://example.com/v2/Users/2819c223...413861904646", "value": "2819c223-7f76-453a-919d-413861904646" } ] } ] }

Can you please let me know if this test should pass as is or not. The test expects a 204 which seems incorrect -- result should be 400 with this payload.

[ArvindHarinder1]

@plamenGo you're right the test needs to be fixed on this. Was this included in your PR?

[weyto]

@ArvindHarinder1 or/and @plamenGo: Was there any kind of investigation on that?

I see that Azure AD send requests like below:

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

the Reference Code only supports a value of type string like:

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

Thanks to any kind of feedback

[dnapoleon]

@weyto - I just want to add this little example here too:

Page 37 of the RFC 7644 shows an example where the "path" is omitted, the "value" is an object type instead of a string or array. https://tools.ietf.org/html/rfc7644#section-3.5.2

{ "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations":[{ "op":"add", "value":{ "emails":[ { "value":"[email protected]", "type":"home" } ], "nickname":"Babs" }] }

[lockelost]

In addition to @weyto 's, I'd like to comment that the JSON Serialization causes a stack-overflow exception.

Serialization of the following object throws a StackOverflow exception.

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

Following function here is the culprit. https://github.com/AzureAD/SCIMReferenceCode/blob/master/Microsoft.SystemForCrossDomainIdentityManagement/Schemas/JsonSerializer.cs

public Dictionary<string, object> ToJson()

After changing the function as follows, it works fine.

        public Dictionary<string, object> ToJson()
        {
            Type type = this.dataContractValue.GetType();
            DataContractJsonSerializer serializer =
                new DataContractJsonSerializer(type, JsonSerializer.SerializerSettings.Value);

            string json;
            JsonSerializerSettings settings = new JsonSerializerSettings
            {
                TypeNameHandling = TypeNameHandling.Objects,
                PreserveReferencesHandling = PreserveReferencesHandling.Objects,
                ReferenceLoopHandling = ReferenceLoopHandling.Ignore
            };
            json = JsonConvert.SerializeObject(this.dataContractValue, Formatting.Indented); // <----!!!!!
            Dictionary<string, object> result = JsonFactory.Instance.Create(json, true);
            return result;
        }

This sample itself is fine, but when port this to existing Asp Net Core project with different NewtonSoft Json causes the crash.