Bug 21973

Summary: CAS URL escaped twice, preventing login
Product: Koha Reporter: Stefan Berndtsson <stefan.berndtsson>
Component: AuthenticationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: dpavlin, fridolin.somers, jonathan.druart, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 21526    
Bug Blocks: 22585    
Attachments: Bug 21973: Do not url escape casServerUrl
Bug 21973: Do not url escape casServerUrl
Bug 21973: Do not url escape casServerUrl
Bug 21973: Do not url escape casServerUrl

Description Stefan Berndtsson 2018-12-07 10:25:40 UTC
Bug 21526 applies the "url" filter on the CAS login link despite it being escaped already in C4/Auth_with_cas.pm, resulting in a double escape and a non-functioning login via CAS.
Comment 1 Jonathan Druart 2018-12-07 13:29:35 UTC
Created attachment 82978 [details] [review]
Bug 21973: Do not url escape casServerUrl

It is already escaped correctly in
C4::Auth_with_cas::_url_with_get_params using URI::Escape::uri_escape

Note that shibbolethLoginUrl is not and must be url escaped in template
("be consistent, they said")
Comment 2 Owen Leonard 2018-12-10 18:02:25 UTC
Created attachment 83022 [details] [review]
Bug 21973: Do not url escape casServerUrl

It is already escaped correctly in
C4::Auth_with_cas::_url_with_get_params using URI::Escape::uri_escape

Note that shibbolethLoginUrl is not and must be url escaped in template
("be consistent, they said")
Comment 3 Kyle M Hall 2018-12-13 14:23:12 UTC
Created attachment 83169 [details] [review]
Bug 21973: Do not url escape casServerUrl

It is already escaped correctly in
C4::Auth_with_cas::_url_with_get_params using URI::Escape::uri_escape

Note that shibbolethLoginUrl is not and must be url escaped in template
("be consistent, they said")

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Martin Renvoize 2018-12-13 16:08:29 UTC
Created attachment 83178 [details] [review]
Bug 21973: Do not url escape casServerUrl

It is already escaped correctly in
C4::Auth_with_cas::_url_with_get_params using URI::Escape::uri_escape

Note that shibbolethLoginUrl is not and must be url escaped in template
("be consistent, they said")

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2018-12-13 16:09:27 UTC
Be nice to fix this in with Auth_with_cas or Auth_with_shibboleth to be consistent.. but that work shouldn't hold this patch up.
Comment 6 Nick Clemens 2018-12-14 15:33:28 UTC
Awesome work all!

Pushed to master for 19.05
Comment 7 Martin Renvoize 2018-12-17 12:06:17 UTC
Pushed to 18.11.x for 18.11.01
Comment 8 Fridolin Somers 2019-02-25 06:51:52 UTC
Not needed for 18.05.x