GordonSo / scimschema

JSON Validator for SCIM (System for Cross-domain Identity Management)
MIT License
8 stars 3 forks source link

The accepted case exact value for "binary" should be "true" #69

Closed mohankumaru closed 3 years ago

mohankumaru commented 3 years ago

Hi @GordonSo ,

According to https://tools.ietf.org/html/rfc7643#section-2.3.6, the binary value should be case exact, but module expects value to be false. In Class BinaryAttribute, the following change should be made, _accepted_case_exact_value = {True}

Regards, Mohan

GordonSo commented 3 years ago

Thanks for raising that the case exact should be "true".

Note, PR has been updated to address this. As per https://tools.ietf.org/html/rfc7643#section-2.2 exact case is False if not provided. As such, any binary attribute will be invalid if the "case exact" property is not explicitly specified "true".

mohankumaru commented 3 years ago

I noticed, "x509Certificates.value" attribute of user core schema is not case sensitive, this contradicts statement here https://tools.ietf.org/html/rfc7643#section-2.3.6 , Should we have accepted case exact value check ?

GordonSo commented 3 years ago

The good news is, this library does not validate against the core schema, as proven by the green tests, so the recent change should not break everything due to "x509Certificates.value".

However, it would affect the projects currently using binary attributes in the extension schemas. Despite that the rule contradicts the core schemas/ examples, I lean towards honouring the "True" check, but I realise this is a subjective opinion.

"ReferenceAttribute" also face a similar issue with the "True" expectation of case-exact.

I have captured this information in an email to scim-chairs@ietf.org for advice.

GordonSo commented 3 years ago

I tried the email from the rfc and it bounced back from the email server, and the email noted above has seemingly gone into a blackhole (for now).

Another look at the definition of case exact, it suggests that it is applicable for string attribute, e.g.

A Boolean value that specifies whether or not a string
         attribute is case sensitive.  The server SHALL use case
         sensitivity when evaluating filters.  For attributes that are
         case exact, the server SHALL preserve case for any value
         submitted.  If the attribute is case insensitive, the server
         MAY alter case for a submitted value.  Case sensitivity also
         impacts how attribute values MAY be compared against filter
         values

Then it is quite strange it was explicitly mentioned it on the binary and reference attribute definition. Also, common sense would suggest that binary/reference value should be case sensitive as the casing can lead to completely different ASCII/ meanings.

On this note, declaration of 'True' case exact still stands to be sensible; to honour the explicit nature of Python. But would also be happy drop it to a classic 'warning' given the 'SHALL/MAY' keywords being used in the definition if such requirement is needed.

That can be done by extending the class with an overriding method.

Feel free to feedback and I will give it a couple of days/ weeks on hold as YAGNI for now.

GordonSo commented 3 years ago

Closing due to the lack of updates