Issue #315: Crowd authorization module doesn't allow https connections
| Reported by: | Chris Rule |
| State: | resolved |
| Created on: | 2018-04-20 21:16 |
| Updated on: | 2018-05-04 19:19 |
Description
The current crowd authorization module only allows http connections to the crowd server for authorization. This means that crowd servers hosted behind a proxy to server https or direct using https cannot be used by the current auth_crowd.py plugin.
What I expected: To be able to connect to crowd using https. What I got: There's no way to configure the crowd authorization to use https.
It appears the back-end code is written for it. There just isn't an interface option.
Attachments
Comments
Comment by Chris Rule, on 2018-04-20 21:16
Comment by Thomas De Schampheleire, on 2018-04-21 07:49
At first sight, it indeed seems sufficient to add a 'method' setting to the settings list at https://kallithea-scm.org/repos/kallithea/files/b22bed4434c50ee48f1beac9d0634bb1943542c7/kallithea/lib/auth_modules/auth_crowd.py#L139
Could you perhaps try that?
Comment by Chris Rule, on 2018-04-23 15:37
Adding a 'method' setting appears to work for me. I've added a patch to this issue detailing what I changed. I opted to use a checkbox to represent https connections (if checked, use https to connect to crowd. If not, use http). If the patch needs to be submitted a different way, please let me know.
Comment by Thomas De Schampheleire, on 2018-05-01 14:56
Thanks for the patch. I modified it slightly to use a dropdown with options http/https instead of the checkbox, as this avoids a separate conversion from boolean to string. Other than that it looked fine. We'll see what the maintainer @kiilerix prefers.
Could you let me know your email address so we can properly attribute the contribution? Currently the patch only contained 'crule'.
Comment by Chris Rule, on 2018-05-01 15:00
Have no problem with a drop down. I originally thought check boxes would be easier for the user.
Sounds like a plan. Thanks for looking at it.
My email is "crule" at "aegistg.com".
Comment by Thomas De Schampheleire, on 2018-05-04 19:19
Fix has been committed on default branch. Thanks for contributing!