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

PatchOp lowercases all JSON keys on a value #70

Open
verbitan opened this issue Aug 19, 2024 · 2 comments
Open

PatchOp lowercases all JSON keys on a value #70

verbitan opened this issue Aug 19, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@verbitan
Copy link
Contributor

[RFC7644] (Section 3.5.2.3) lists the following example of a valid urn:ietf:params:scim:api:messages:2.0:PatchOp object,

   The following example shows how to change a User's entire "work"
   address, using a "valuePath" filter.  Note that by setting "primary"
   to "true", the service provider will reset "primary" to "false" for
   any other existing values of "addresses".

   PATCH /Users/2819c223-7f76-453a-919d-413861904646
   Host: example.com
   Accept: application/scim+json
   Content-Type: application/scim+json
   Authorization: Bearer h480djs93hd8
   If-Match: W/"a330bc54f0671c9"

   {
     "schemas":
       ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
     "Operations": [{
       "op":"replace",
       "path":"addresses[type eq \"work\"]",
       "value":
       {
         "type": "work",
         "streetAddress": "911 Universal City Plaza",
         "locality": "Hollywood",
         "region": "CA",
         "postalCode": "91608",
         "country": "US",
         "formatted":
   "911 Universal City Plaza\nHollywood, CA 91608 US",
         "primary": true
       }
     }]
   }

PatchOp.model_validate() validates this request as expected, but it lowercases all the keys of the JSON object, meaning they no longer match the schema. I don't believe this to be correct.

[RFC7644] (Section 3.10) does make a statement that All facets (URN, attribute, and sub-attribute name) of the fully encoded attribute name are case insensitive, and the keys of the JSON object are attributes, which is where I suspect the reasoning of lowercasing all the keys originated, but it's at odds with all other models provided by this library (e.g. User dumps userName).

PatchOp.model_validate(
    {
        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
        "Operations": [
            {
                "op": "replace",
                "path": 'addresses[type eq "work"]',
                "value": {
                    "type": "work",
                    "streetAddress": "911 Universal City Plaza",
                    "locality": "Hollywood",
                    "region": "CA",
                    "postalCode": "91608",
                    "country": "US",
                    "formatted": "911 Universal City Plaza\nHollywood, CA 91608 US",
                    "primary": True,
                },
            }
        ],
    }
).model_dump()
{
  "schemas":[
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations":[
    {
      "op":"replace",
      "path":"addresses[type eq \"work\"]",
      "value":{
        "type":"work",
        "streetaddress":"911 Universal City Plaza",
        "locality":"Hollywood",
        "region":"CA",
        "postalcode":"91608",
        "country":"US",
        "formatted":"911 Universal City Plaza\nHollywood, CA 91608 US",
        "primary":true
      }
    }
  ]
}
@azmeuk azmeuk added the bug Something isn't working label Aug 19, 2024
@azmeuk
Copy link
Contributor

azmeuk commented Aug 19, 2024

Thank you for your report. Indeed, I would indeed expect the case to be kept during the export.

This might be due to BaseModel.normalize_attribute_name lowering everything, and PatchOp.value being of type Any it does not get reformatted on exports like other Resource do with that to_camel serializer..

However, I am not really sure how to fix this. There might be needed to add type parameterss to PatchOp so it can guess how to correctly case attributes (and also, reject bad attributes by the way?). Something like PatchOp[User] 🤔

@verbitan
Copy link
Contributor Author

verbitan commented Aug 22, 2024

Another interesting example of this is the following patch request from https://learn.microsoft.com/en-us/entra/identity/app-provisioning/application-provisioning-config-problem-scim-compatibility. In this example, the value keys are lowercased and the . is removed, so name.givenName becomes namegivenname.

PatchOp.model_validate(
    {
        "schemas": [
            "urn:ietf:params:scim:api:messages:2.0:PatchOp"
        ],
        "Operations": [
            {
                "op": "replace",
                "value": {
                    "displayName": "Bjfe",
                    "name.givenName": "Kkom",
                    "name.familyName": "Unua",
                    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber": "Aklq"
                }
            }
        ]
    }
).model_dump()
{
  "schemas":[
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations":[
    {
      "op":"replace",
      "value":{
        "displayname":"Bjfe",
        "namegivenname":"Kkom",
        "namefamilyname":"Unua",
        "urn:ietf:params:scim:schemas:extension:enterprise:2.0:user:employeenumber":"Aklq"
      }
    }
  ]
}

I think a type parameter to PatchOp is a nice solution, as you can then validate the attributes as part of the model. Though it'd have to handle filters, which complicates things somewhat.

{
  "schemas": [
      "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ],
  "Operations": [
      {
          "op": "replace",
          "path": "emails[type eq \"work\"].value",
          "value": "[email protected]"
      }
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants