[AdminApi] PUT /api/Clients/{id} duplicates client claims

[ig-sinicyn]

Describe the bug

HTTP PUT /api/Clients/{id} inserts new claims instead of updating the client claims list.

To Reproduce

1. Receive client json (we 're using admin api swagger). Example:

curl -X GET "http://localhost:1330/api/Clients/42" -H "accept: application/json" -H "Authorization: Bearer ..."
200 OK
{
  "absoluteRefreshTokenLifetime": 2592000,
  ...
  "claims": [
    {
      "id": 25,
      "type": "role",
      "value": "Administrator"
    },
    {
      "id": 26,
      "type": "role",
      "value": "Test"
    }
  ],
  "properties": []...
}

2. Call PUT received json /api/Clients/{id}
3. Call GET/api/Clients/{id}. Claims are duplicated

curl -X GET "http://localhost:1330/api/Clients/42" -H "accept: application/json" -H "Authorization: Bearer ..."
200 OK
{
  "absoluteRefreshTokenLifetime": 2592000,
  ...
  "claims": [
    {
      "id": 25,
      "type": "role",
      "value": "Administrator"
    },
    {
      "id": 26,
      "type": "role",
      "value": "Test"
    },
    {
      "id": 27,
      "type": "role",
      "value": "Administrator"
    },
    {
      "id": 28,
      "type": "role",
      "value": "Test"
    }
  ],
  "properties": []...
}

[skoruba]

Thanks for reporting this issue: for updating ClientClaims is special endpoint called: api/ClientClaims/

[alxnbl]

Hey @skoruba , the PUT /Clients/{id}/ duplicate values of clientClaims but also of "properties" field. Even when the PUT request do not modify any of this 2 fields. More over, once the duplicated properties have been added, a GET of the client returns an error due to non-unique key in clientClaims and properties collections.

A fix could be to update the model of the body request of the PUT method to remove the clientClaims and properties collection, avoiding to update this fields as part of this method. But for my use case it is not ideal because I am trying to update multiple values of the collection in a single atomic operation (transactional).

The other fix more handy for me would be to make an evolution of the PUT method of /Clients/{Id} to implement the following behavior :

• Get the current values of the collection
• Compare with new value of the PUT request, if any change found (valued added, updated, or removed) then remove all the existing values of the collection and add the new ones

Does it seams an acceptable behavior for you ? And If so, would you be interested that we send you a PR of this fix ?

Thanks you for you answer, and for all your job on this project by the way ! Alex

[skoruba]

Sure, you can send a PR. :) thx