Accept any casing for the string value of Op

[ets]

The Op enum values are all lowercase but some clients pass title case value.

Here's an example from MSFT Entra:

{
    "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
    "Operations": [{
        "op": "Replace",
        "path": "userName",
        "value": "5b50642d-79fc-4410-9e90-4c077cdd1a59@testuser.com"
    }]
}

This PR uses a pydantic validator to lower whatever is passed in before value validation.

[azmeuk]

Hi. Thank you for your contribution. A few things:

• unit tests are broken (due to a missing import)
• please add a unit (at least) test that checks the new behavior you are implementing
• a field_validator would be better than a whole model_validator in that situation.

I can fix those unless you want to edit your PR. Please let me know.

[ets]

Certainly - I'll address these later today.

On Fri, Jul 12, 2024 at 7:53 AM Éloi Rivard @.***> wrote:

Hi. Thank you for your contribution. A few things:

• unit tests are broken (due to a missing import)
• please add a unit test that checks the new behavior you are implementing
• a field_validator would be better than a whole model_validator in that situation.

I can fix those unless you want to edit your PR. Please let me know.

— Reply to this email directly, view it on GitHub https://github.com/yaal-coop/scim2-models/pull/55#issuecomment-2225423933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPNUUSNBMHSQHGUQ3HXLTZL67UNAVCNFSM6AAAAABKYE5NT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRVGQZDGOJTGM . You are receiving this because you authored the thread.Message ID: @.***>

-- Eric Simmerman President | BitAcuity, Inc. @.***

[azmeuk]

The CI is nitpicky :)

• There are some style issues, they can be fixed by applying tox -e style or pre-commit run --all-files.
• The coverage is not 100%. This is because the if condition in the validator is always True in the test suite. This can be fixed by adding a test that checks how the validator behaves with a None value for instance.
• The pytest.mark.parametrize might be removed, as it only parametrize one single value. You might simply replace context by Context.RESOURCE_REPLACEMENT_REQUEST in the test.

[ets]

Got it. This latest passes all the tox driven checks.

[azmeuk]

Thank you!

[azmeuk]

Some thoughts afterwards.

I canot find anything about sensitivity for patch operations in RFC7643, RFC7644 or RFC6902. Actually, RFC7644 §3.5.2 seems to indicate the opposite:

The body of an HTTP PATCH request MUST contain the attribute "Operations", whose value is an array of one or more PATCH operations. Each PATCH operation object MUST have exactly one "op" member, whose value indicates the operation to perform and MAY be one of "add", "remove", or "replace".

Do you see clues somewhere in favor of case insensitivity?

I can see it mentioned in Develop and plan provisioning for a SCIM endpoint in Microsoft Entra ID and illustrated in Entra SCIM compatibility issues resolutions .

Don't require a case-sensitive match on structural elements in SCIM, in particular PATCH op operation values, as defined in section 3.5.2. Microsoft Entra ID emits the values of op as Add, Replace, and Remove.

So if I am not wrong, it seems Entra is explicitly misinterpret the specs here. This is a detail so we can keep the compatibility validator in scim2-models.

Related issues in other projects:

• https://github.com/lessonly/scim_rails/issues/33

[ets]

I agree that it seems the MSFT implementation I'm witnessing is incorrect to be using titlecase. Personally I favor applying the robustness principle to this one in favor of more interoperability (rather than requiring all users of this project to implement local workarounds), so glad you are open to keeping this in place.