search

mitodl / pylti

PyLTI implementation
BSD 2-Clause "Simplified" License
58 stars 44 forks source link

Draconian "role" checking #42

Closed velochy closed 9 years ago

velochy commented 9 years ago

I think it would make sense to allow role checking to be completely disabled if the role specified is 'any' for a few practical concerns

Right now, even with any, the role is required to belong to a fixed list specified in LTI_ROLES. However, I am integrating my service with an LTI consumer that uses quite a few non-standard role names, and I currently have to patch them in by importing LTI_ROLES and then appending the new names to LTI_ROLES['any'].

Which mostly works. However - the consumer I am integrating with actually has a mode where no role is provided at all (for a reasonably valid reason, actually) and that case can only be solved by branching the code and changing it myself.

Making role='any' just bypass the check would allow these non-standard cases to be handled as well while not really impacting the behavior on standard cases, making it potentially usable in much wider settings.

carsongee commented 9 years ago

This makes sense to me, and should be a pretty quick code change without breaking anyone out there. Would it be possible for you to potentially provide the code for that?

velochy commented 9 years ago

Yes. On it.