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

attribute keys are CAPS sensitive #7

Closed
Janne-ka opened this issue Sep 9, 2021 · 8 comments
Closed

attribute keys are CAPS sensitive #7

Janne-ka opened this issue Sep 9, 2021 · 8 comments

Comments

@Janne-ka
Copy link

Janne-ka commented Sep 9, 2021

based on specification, attribute names should be case insensitive. Now it seems they are not, e.g. "Primary", "UserName" are not valid attribute names.

@Janne-ka
Copy link
Author

to specify, this was found with postman when sending scim+json information in POST or PUT

@pond
Copy link
Member

pond commented Sep 14, 2021

I did a bit of a deep dive today to revisit issues of case sensitivity and arrived back at the original implementation decision, unfortunately, of "very hard". Some aspects of case insensitivity (e.g. in filters) are handled but main attribute names are presently not.

Ruby provides no out-of-box way to have things like case-insensitive Hash keys and a great many of the operations that Scimitar performs on Hash and Array data for inbound payload processing become invalidated once the case of any of these entities becomes indeterminate. Use of ActiveModel for attribute assignment also leads to case-sensitivity in attribute names. A lot of this kind of stuff was leant on heavily just to make the gem implementable without unreasonably vast amounts of time/resources in a moderately reliable fashion, even within the documented limitations such as those described in #6.

SCIM is a very difficult spec to implement with a lot of undocumented edge cases and deceptive amounts of deep complexity hidden in what often look like "innocent" specification examples and statements. As with many RFCs, it is a pity that the authors were not required to produce a reference implementation as this would I'm sure have gone a long way to aiding in simplification and clarification of the RFC prior to its finalisation.

I'll keep looking at this, but it's definitely an extremely large job (in the worst case, to the point of rewriting a good chunk of some of the hardest-to-implement code in the gem) and unless I have a sudden breakthrough idea on how to get it done efficiently, I can't see it happening quickly.

I would presume from the bug report that you aren't in control of the client service that's giving you non-camel-case names such as UserName and thus receiving 400 responses, so require this to be fixed in the gem for your integration to work?

@Janne-ka
Copy link
Author

So far we are testing with postman, link to test cases from azure. We have no possibility to alter our customers SCIM clients once we go operational.
It kind of sounds like "translation table" is needed, having the key as small caps and pointing to original. Possibly clients input hash keys could always be changed to small caps, as clients should be case insensitive as well

@pond
Copy link
Member

pond commented Sep 15, 2021

Well I'm making some headway, so watch this space :-)

@pond pond mentioned this issue Sep 15, 2021
@pond
Copy link
Member

pond commented Sep 15, 2021

The above is going through our review process now. Believe it or not, that's about as small as I could make the change set - borrowing from HashWithIndifferentAccess solved a lot of problems.

@pond
Copy link
Member

pond commented Sep 15, 2021

@Janne-ka Version 1.1.0 has been pushed to https://rubygems.org/gems/scimitar - hopefully this fixes your case sensitivity issues, though there may be other problems; fingers crossed anyway. We haven't had a chance to test with too many real-world SCIM endpoints yet.

Please let me know how you get on 👍

@Janne-ka
Copy link
Author

initial Postman tests pass, so looking good. Code looks great too.

@pond
Copy link
Member

pond commented Sep 16, 2021

Thanks for the update. And of course, if there are any more problems, please don't hesitate to open a new issue.

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