search

simpleidserver / SimpleIdServer

OpenID, OAuth 2.0, SCIM2.0, UMA2.0, FAPI, CIBA & OPENBANKING Framework for ASP.NET Core
https://simpleidserver.com/
Apache License 2.0
685 stars 90 forks source link

Update SimpleIdServer.Scim from version 4.0.0 on 4.0.5 - Patch request processing #644

Closed alexander-durovich-swi closed 6 months ago

alexander-durovich-swi commented 7 months ago

Hello,

We have found some inconsistencies between version 4.0.0 and 4.0.5, when in the PATCH request there are indicated 2 "replace" operations for the same filed.

Earlier the behaviour was as indicated in the https://www.rfc-editor.org/rfc/rfc7644#section-3.5.2:

Each PATCH operation represents a single action to be applied to the same SCIM resource specified by the request URI. Operations are applied sequentially in the order they appear in the array. Each operation in the sequence is applied to the target resource; the resulting resource becomes the target of the next operation.

Now always the value of first operation is applied. As I see, the reason is in the code https://github.com/simpleidserver/SimpleIdServer/blob/v4.0.5/src/Scim/SimpleIdServer.Scim/Extensions/SCIMRepresentationExtensions.cs#L25, i.e. for each "replace" operation the new attribute is added, in result if request has 2 replace operation for the same field, 2 new attributes will be added for that field. And value of first attribute is applied and returned in response.

Moreover, if send such request several times, the first or second values will be applied by turn. By sending request first time, the first value will be applied, by sending second time - second value, by sending third time - again first value and so on. The reason is that by checking if operation should be apllied, the new value is compared with the current value in DB, and if values are equal, then operation is ignored. As result, by sending the same request several time, either first or second operation is ignored as its value is the same with value in DB. It seems additionally the new value should be checked with the result of previous operations in the same request. Current code that checks old data in DB and new data: https://github.com/simpleidserver/SimpleIdServer/blob/v4.0.5/src/Scim/SimpleIdServer.Scim/Helpers/RepresentationHelper.cs#L301

The similar behaviour is observed when several "add" operations are indicated for the same field.

Are you agree that these points are bugs? If yes, will you fix them?

Thank you in advance. Best Regards, Alex

simpleidserver commented 7 months ago

Indeed there is a regression in the version 4.0.5 :(.

This issue is fixed in the master branch.

alexander-durovich-swi commented 7 months ago

Hello,

Ok, thank you!

Best Regards, Alex

alexander-durovich-swi commented 6 months ago

Hello,

We verified it on version 4.0.6. It is working. Thank you!

Best Regards, Alex