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
737 stars 99 forks source link

[SCIM] PUT operation on immutable group fails #247

Closed danflomin closed 2 years ago

danflomin commented 2 years ago

Setup: all the member attributes of a group are defined as immutable as defined here.

Reproduces by the following actions:

  1. POST - create a user
  2. POST - create a group with the user in it
  3. PUT - on the group with the same members payload as in 2

The response is:

{
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:Error"
    ],
    "status": "400",
    "scimType": "mutability",
    "detail": "attribute d6c036b6-51f7-4c92-a45f-d8c747fe2304 is immutable"
}

In RFC-7644 section 3.5.1 -

   immutable  If one or more values are already set for the attribute,
      the input value(s) MUST match, or HTTP status code 400 SHOULD be
      returned with a "scimType" error code of "mutability".  If the
      service provider has no existing values, the new value(s) SHALL be
      applied.

Then, it seems to me that if the members in the PUT payload is the same, we should not get a 400 status code.

This same exception occurs on the following, which also seems to not follow this RFC section.

  1. POST - create a user
  2. POST - create an empty group
  3. PUT - on the group with the user in it (to add the user)
simpleidserver commented 2 years ago

Hello,

Indeed there is an issue in the algorithm used to check the "mutability". We are going to fix it in this release.

Kind Regards,

SimpleIdServer

simpleidserver commented 2 years ago

Hello,

The issue is fixed in the "release/2.0.6" branch.

Kind Regards,

SimpleIdServer.

danflomin commented 2 years ago

Hi,

I tested the code found in branch "release/2.0.7" and found that a similar issue still exists.

I get the following message:

{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:Error"
  ],
  "status": "400",
  "scimType": "mutability",
  "detail": "attribute members is immutable"
}

To reproduce the error

  1. POST - create a user
  2. POST - create an empty group
  3. PUT - on the group with the user in it (to add the user)
  4. Perform step 3 once again
simpleidserver commented 2 years ago

Hello,

I tried to reproduce your issue in the branch "release/2.0.7" without success. You'll find in attachment the POSTMAN project used to execute the integration tests.

SCIM.postman_collection.zip

Kind Regards,

Sid

danflomin commented 2 years ago

Hi,

Indeed I could not reproduce using the attached POSTMAN collection.

I modified the body of the PUT (Update group) to the following:

{
  "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
  "displayName": "Grp",
  "members": [
    { "value": "{{userId}}, "display": "something" }
  ]
}

Now it fails for me.

simpleidserver commented 2 years ago

Hello,

You received an exception because the attribute "members.display" doesn't exist in the SCIM Schema. However, there is a small issue inside the "IsMutabilityValid" algorithm. The following request should work but an exception is thrown:

{
  "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
  "displayName": "Grp",
  "members": [
    { "value": "{{userId}}", "type":"User" }
  ]
}

I'm going to fix it.

Kind Regards,

danflomin commented 2 years ago

The schema I am using does have members.display.

Will your fix for the type sub attribute will help me with my exception as well?

simpleidserver commented 2 years ago

Yes 😉

simpleidserver commented 2 years ago

The issue is fixed in the branch "release/2.0.7". You can use this Nuget package : "https://www.myget.org/feed/advance-ict/package/nuget/SimpleIdServer.Scim.Persistence.EFNet6" version 2.0.7-ci-00276.

danflomin commented 2 years ago

Hey,

I am looking at the branch "release/2.0.7", and I don't see the new commit that fixes this.

Are you sure this was pushed?

Thanks

danflomin commented 2 years ago

I found your new commit. My UT worked.

Unfortunately, configuring Okta to use the fixed SCIM server yielded errors.

Below are the POST/PUT requests made to the server, with the error that occurred on the 2nd time we tried to update the group members using PUT.

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "displayName": "SID",
  "members": []
}
Request => Status: 201, Url: "http://localhost:8080/Groups", Method: "POST"

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "id": "434a8bba-dfce-483d-b48c-8a07b6f163a3",
  "displayName": "SID",
  "members": []
}
Request => Status: 200, Url: "http://localhost:8080/Groups/434a8bba-dfce-483d-b48c-8a07b6f163a3", Method: "PUT"

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:User"
  ],
  "userName": "asdqwe",
  "name": {
    "givenName": "First",
    "familyName": "Last"
  },
  "emails": [
    {
      "primary": true,
      "value": "asdqwe@gmail.io",
      "type": "work"
    }
  ],
  "displayName": "First Last",
  "locale": "en-US",
  "externalId": "asdqwe",
  "groups": [],
  "password": "123345",
  "active": true
}
Request => Status: 201, Url: "http://localhost:8080/Users", Method: "POST"

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "id": "434a8bba-dfce-483d-b48c-8a07b6f163a3",
  "displayName": "SID",
  "members": [
    {
      "value": "26e3c3cf-1a66-4a65-84a6-ff690f92b4a5",
      "display": "asdqwe"
    }
  ]
}
Request => Status: 200, Url: "http://localhost:8080/Groups/434a8bba-dfce-483d-b48c-8a07b6f163a3", Method: "PUT"

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "id": "434a8bba-dfce-483d-b48c-8a07b6f163a3",
  "displayName": "SID",
  "members": [
    {
      "value": "26e3c3cf-1a66-4a65-84a6-ff690f92b4a5",
      "display": "asdqwe"
    }
  ]
}
ERROR: attribute members is immutable||SimpleIdServer.Scim.Exceptions.SCIMImmutableAttributeException: attribute members is immutable
   at SimpleIdServer.Scim.Commands.Handlers.ReplaceRepresentationCommandHandler.Handle(ReplaceRepresentationCommand replaceRepresentationCommand) in /SimpleIdServer.Scim/Commands/Handlers/ReplaceRepresentationCommandHandler.cs:line 79
   at SimpleIdServer.Scim.Commands.Handlers.ReplaceRepresentationCommandHandler.Handle(ReplaceRepresentationCommand replaceRepresentationCommand) in /SimpleIdServer.Scim/Commands/Handlers/ReplaceRepresentationCommandHandler.cs:line 112
   at SimpleIdServer.Scim.Api.BaseApiController.InternalUpdate(String id, RepresentationParameter representationParameter) in /SimpleIdServer.Scim/Api/BaseApiController.cs:line 297
Request => Status: 400, Url: "http://localhost:8080/Groups/434a8bba-dfce-483d-b48c-8a07b6f163a3", Method: "PUT"
simpleidserver commented 2 years ago

Hello,

The last HTTP request is not working because the value "asdqwe" passed to the property "members[0].display" must be equals to the "displayName" of the user. In your use case the HTTP request must be:

{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "id": "434a8bba-dfce-483d-b48c-8a07b6f163a3",
  "displayName": "SID",
  "members": [
    {
      "value": "26e3c3cf-1a66-4a65-84a6-ff690f92b4a5",
      "display": "First Last"
    }
  ]
}
danflomin commented 2 years ago

Hi,

Thanks for pointing that out.

Anyway - why would the first PUT works, and the second one doesn't? The body of these 2 PUTs is the same.

According to the RFC, as far as I understand, since the body of the 2 PUTs is the same, the mutability error code should not occur.

Thanks

simpleidserver commented 2 years ago

Hello,

The issue should be fixed in the branch "release/2.0.7". In the new implementation, if one or more attributes like "members.display" or "members.type" are passed in the HTTP PUT request, then they are ignored by the SCIM API and mutability is not checked.

I made this choice because the value is overriden by executing one or more attribute mapping rule (class : SCIMAttributeMapping). In your use case, the value "members.display" is coming from the property "displayName" of the corresponding User representation. If the following attribute mapping rule is removed then the mutability check will work :

var firstAttributeMapping = new SCIMAttributeMapping
{
    Id = Guid.NewGuid().ToString(),
    SourceAttributeId = userSchema.Attributes.First(a => a.Name == "groups").Id,
    SourceResourceType = StandardSchemas.UserSchema.ResourceType,
    SourceAttributeSelector = "groups",
    TargetResourceType = StandardSchemas.GroupSchema.ResourceType,
    TargetAttributeId = groupSchema.Attributes.First(a => a.Name == "members").Id
};
danflomin commented 2 years ago

My tests seem to work now. Thanks for your effort on this !

Can you please release these new fixes as a nuget on the official channel?

simpleidserver commented 2 years ago

Hello,

The release is scheduled tomorrow (10-02).