SimpleIdServer icon indicating copy to clipboard operation
SimpleIdServer copied to clipboard

[SCIM] upgrade to EF6 with 2.0.15

Open danflomin opened this issue 3 years ago • 14 comments

Hello :)

I tried using EF6 with v2.0.15 and noticed a few issues.

  1. Data was corrupted in the DB, see the attached csv in the zip. Some attributes are duplicated. corrupted_user.zip

  2. GET on corrupted users does not work. The error I get when trying to query a corrupted user:

System.ArgumentException: Can not add property type to Newtonsoft.Json.Linq.JObject. Property with the same name already exists on object.
   at Newtonsoft.Json.Linq.JObject.ValidateToken(JToken o, JToken existing)
   at Newtonsoft.Json.Linq.JContainer.InsertItem(Int32 index, JToken item, Boolean skipParentCheck)
   at Newtonsoft.Json.Linq.JContainer.Add(Object content)
   at SimpleIdServer.Scim.Domain.SCIMRepresentationExtensions.EnrichResponse(IEnumerable`1 attributes, JObject jObj, Boolean mergeExtensionAttributes, Boolean isGetRequest)
   at SimpleIdServer.Scim.Domain.SCIMRepresentationExtensions.ToResponse(SCIMRepresentation representation, String location, Boolean isGetRequest, Boolean includeStandardAttributes, Boolean addEmptyArray, Boolean mergeExtensionAttributes)
   at SimpleIdServer.Scim.Api.BaseApiController.InternalGet(String id)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)

Thank you very much for your help!

danflomin avatar Aug 16 '22 11:08 danflomin

Hello,

Some changes have been made in the master branch. Now the change tracker is properly working with EFCore6. There was an issue because the property Children was set when building the hierarchy.

Can-you please try again and check if you still have duplicate data in your database when adding and updating users ? The SQLScript below can be executed to remove corrupted attributes :

DELETE FROM dbo.SCIMRepresentationAttributeLst where LEN(dbo.SCIMRepresentationAttributeLst.FullPath) - LEN(REPLACE(dbo.SCIMRepresentationAttributeLst.FullPath, '.', '')) > 0 AND dbo.SCIMRepresentationAttributeLst.ParentAttributeId IS NULL

KR,

Sid

simpleidserver avatar Aug 16 '22 13:08 simpleidserver

Hello,

Thanks for the quick response.

A few months ago you already proposed this solution for this issue, and I said that I am not sure if it will work for me as I inherit from the SCIM db context and add some logic of my own.

If there is another way of accomplishing this, then it would be great!

If not, I'll need to have some modifications in my code, before I could check if it works or not.

I will keep you updated.

Kind regards, Dan

danflomin avatar Aug 17 '22 06:08 danflomin

Some changes have been made in the master branch.

Now, there is only one DBContext and a new property has been added to build the hierarchy. Can-you please check if you still have duplicate data when addind and updating users ?

KR,

SID

simpleidserver avatar Aug 17 '22 14:08 simpleidserver

Hello,

Thank you for taking care of this.

I will be able to check next week only.

Can you please release a preview nuget?

Thanks

On Wed, Aug 17, 2022, 17:03 SimpleIdServer @.***> wrote:

Some changes have been made in the master branch.

Now, there is only one DBContext and a new property has been added to build the hierarchy. Can-you please check if you still have duplicate data when addind and updating users ?

KR,

SID

— Reply to this email directly, view it on GitHub https://github.com/simpleidserver/SimpleIdServer/issues/310#issuecomment-1218054833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIB5S7PTQL2TPTV3HFL7AFDVZTWMJANCNFSM56VORV2A . You are receiving this because you authored the thread.Message ID: @.***>

danflomin avatar Aug 18 '22 06:08 danflomin

You can use the following pre-release version

dotnet add package SimpleIdServer.Scim.Persistence.EFNet6 --version 2.0.16-ci-00326 --source https://www.myget.org/F/advance-ict/api/v3/index.json

simpleidserver avatar Aug 18 '22 19:08 simpleidserver

Hey !

I still see the new db context in here .

Is it ok? I thought we should have only 1 db context with the new hierarchy.

Thanks

danflomin avatar Aug 29 '22 13:08 danflomin

My mistaske, I've forgotten to checked-in my changes. They are present in the master branch.

simpleidserver avatar Aug 29 '22 13:08 simpleidserver

Cool. So I'd like to test this now.

Can you please upload this as a preview nuget?

danflomin avatar Aug 29 '22 14:08 danflomin

The version 2.0.17-ci-00331 is available here :

https://www.myget.org/feed/advance-ict/package/nuget/SimpleIdServer.Scim.Persistence.EFNet6

simpleidserver avatar Aug 29 '22 14:08 simpleidserver

I ran some tests using this version and it looks ok.

The thing is I don't know how to reproduce the issue you solved, so I can't verify if this fixes is or not.

If you have an idea to verify everything works as expected, then I'd like to hear.

If not, please release this as a new official nuget and I'll try again to migrate to EF6.

Kind regards Dan

danflomin avatar Aug 30 '22 10:08 danflomin

Several scenarios have been executed to check if the bug is solved. I'm checking if there is no duplicate data in the representations after executing UPDATE operations.

Scenario 1 :

  • Create a user
{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:User",
    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"
  ],
  "userName": "{{$guid}}",
  "active": true,
  "displayName": "lennay",
  "externalId": "e1f4c134-2dec-4754-bb50-9e786e4ecae9",
  "name": {
    "formatted": "Andrew Ryan",
    "familyName": "Ryan",
    "givenName": "Andrew"
  },
  "emails": [
    {
      "primary": true,
      "type": "work",
      "value": "[email protected]"
    },
    {
      "primary": false,
      "type": "home",
      "value": "[email protected]"
    }
  ]
}
  • Update a user
{
  "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User", "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"],
  "externalId":"dschrute",
  "name":{
    "familyName": "Schrute",
    "givenName": "Dwight"
  },
	"department": "department",
    "userName": "145465f3-64cb-4e2e-984f-c8835036ab4c",
	"costCenter": null,
  "emails":[
    {
      "value":"[email protected]",
      "type":"work",
      "primary": false
    }
  ]
}
  • Check the number of parameters in the database is minimal and there is no duplicate.

Scenario 2 : Add a user to a group

  • Add a user
{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:User",
    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"
  ],
  "userName": "{{$guid}}",
  "active": true,
  "displayName": "lennay",
  "externalId": "e1f4c134-2dec-4754-bb50-9e786e4ecae9",
  "name": {
    "formatted": "Andrew Ryan",
    "familyName": "Ryan",
    "givenName": "Andrew"
  },
  "emails": [
    {
      "primary": true,
      "type": "work",
      "value": "[email protected]"
    },
    {
      "primary": false,
      "type": "home",
      "value": "[email protected]"
    }
  ]
}
  • Add a group
{
  "schemas": [
    "urn:ietf:params:scim:schemas:core:2.0:Group"
  ],
  "displayName": "SID22",
  "members": []
}
  • Assign a user to a group
{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations": [
    {
      "op": "add",
      "path": "members",
      "value": {
            "value": "{{userId}}"
        }
      
    }
  ]
}
  • Open the database and check there is one group in the User Representation, and there is one member in the group Representation.

  • Remove a user from a group

{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations": [
    {
      "op": "remove",
      "path": "members",
      "value": {
            "value": "{{userId}}"
        }
      
    }
  ]
}
  • Open the database and check there is no group in the User Representation and there is no member in the group representation.

simpleidserver avatar Aug 30 '22 12:08 simpleidserver

Hello,

While trying to upgrade to 2.0.17 with EF6 I found a few issues.

  1. When adding a user to an empty group through PATCH, the user is not seen as a member of the group (the group's members list is empty).
  2. When I PATCH a group's name, I don't see it was changed. I also don't see it was changed in the user's groups list.
  3. When adding a user to a group, I do not see a RepresentationReferenceAttributeAddedEvent for the user.

I'd appreciate your help with this.

Thanks Dan

danflomin avatar Sep 07 '22 16:09 danflomin

Hello,

I couldn't reproduce the bugs with the latest version (2.0.17). The sample project has been updated and is using the version 2.0.17. The project is located here : https://github.com/simpleidserver/SimpleIdServer/tree/master/samples/UseSCIMSqlServer

All POSTMAN requests are passing. I used the following POSTMAN collection to execute the test : Ticket310.postman_collection.zip

Can-you please check if your configuration (in the Startup.cs) is matching the one present inside the sample project. I think the SCIMAttributeMapping are missing from your database.

KR,

SID

simpleidserver avatar Sep 08 '22 12:09 simpleidserver

Hey,

I'm sorry for the inconvenience.

Indeed it was an issue in my code.

I will soon try to deploy SCIM with EF6 :)

Kind regards Dan

danflomin avatar Sep 11 '22 20:09 danflomin