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

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

Closed
mohankumaru opened this issue Jun 21, 2021 · 5 comments
Closed

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

mohankumaru opened this issue Jun 21, 2021 · 5 comments
Assignees

Comments

@mohankumaru
Copy link

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
Copy link
Owner

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".

@GordonSo GordonSo self-assigned this Jun 21, 2021
@mohankumaru
Copy link
Author

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 GordonSo reopened this Jun 22, 2021
@GordonSo
Copy link
Owner

GordonSo commented Jun 22, 2021

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
Copy link
Owner

GordonSo commented Jun 26, 2021

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 GordonSo reopened this Jun 26, 2021
@GordonSo
Copy link
Owner

GordonSo commented Jul 7, 2021

Closing due to the lack of updates

@GordonSo GordonSo closed this as completed Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants