Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PATCH Remove operation for sub-attribute in a complex (multi-valued) attribute not working #57

Closed
jivpif opened this issue Nov 16, 2020 · 4 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@jivpif
Copy link
Contributor

jivpif commented Nov 16, 2020

Hello,

I am having the following "roles" complex multi-valued model as part of my user schema model:

"roles": [
{
"primary": false,
"type": "WindowsAzureActiveDirectoryRole",
"value": "Member",
"display": "Member"
}
]

When I try to execute the following Remove patch operation on it, the display sub-attribute is not removed:

"op": "Remove",
"path": "roles[value eq "Member"].display"

I was trying to debug it a bit, and I think that the problem may be is somewhere around the following code:
https://github.com/simpleidserver/SimpleIdServer/blob/master/src/Scim/SimpleIdServer.Scim/Extensions/SCIMRepresentationExtensions.cs#L59
The Parent value seem to be null, although it should be the "roles" attribute to which the "display" sub-attribute belongs.

There is a similar issue with the Replace operation on sub-attribute in a complex attribute - the old value of the sub-attribute is not replaced with the new one. But instead, the new value of the sub-attribute is added as an attribute to the root level of the user object.

Thanks a lot in advance for the time

@simpleidserver
Copy link
Owner

I tried to reproduce the bugs with the latest changes made on the branch "release/1.1.9" without success.
Can-you try again ?

@simpleidserver simpleidserver self-assigned this Nov 18, 2020
@simpleidserver simpleidserver added the question Further information is requested label Nov 18, 2020
@jivpif
Copy link
Contributor Author

jivpif commented Nov 18, 2020

Hey, yep, I was able to still reproduce it.

I think the problem is that the Parent of the sub-attribute I am trying to Replace or Remove is null.

It seem that on a Patch request, the first thing is to Get the current data from the database. Our Get implementation includes parsing a Json object to SCIMRepresentation using this method:
https://github.com/simpleidserver/SimpleIdServer/blob/master/src/Scim/SimpleIdServer.Scim/Helpers/SCIMRepresentationHelper.cs#L35

I think that when generating a complex attribute from JObject the Parent of the sub-attributes is not set.
I added the following code:

foreach (var val in record.Values)
{
val.Parent = record;
}

here after line 142:
https://github.com/simpleidserver/SimpleIdServer/blob/master/src/Scim/SimpleIdServer.Scim/Helpers/SCIMRepresentationHelper.cs#L142

And with that additional change, both Remove and Replace operations worked well for the sub-attributes with the latest changes on the release 1.1.9 branch.

Do you think this method can be fixed as well to set the Parent of the sub-attributes?
https://github.com/simpleidserver/SimpleIdServer/blob/master/src/Scim/SimpleIdServer.Scim/Helpers/SCIMRepresentationHelper.cs#L132

Thanks again

thabart added a commit that referenced this issue Nov 18, 2020
@simpleidserver simpleidserver added the bug Something isn't working label Nov 18, 2020
@simpleidserver
Copy link
Owner

Oups sorry, I didn't check if there were regressions on the Entity Framework project.
The issue should be fixed now, I couldn't reproduce the bug in the branch "release/1.1.9".

@jivpif
Copy link
Contributor Author

jivpif commented Nov 18, 2020

Great. I tried it, indeed the operation works smoothly when the Parent of the sub-attribute is present. Thanks a lot!

@jivpif jivpif closed this as completed Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants