Bug 23526 - Shibboleth login url with query has double encoded '?' %3F
Summary: Shibboleth login url with query has double encoded '?' %3F
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-30 17:16 UTC by Nick Clemens
Modified: 2019-10-17 19:36 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.04


Attachments
Bug 23526: Don't use encoded question mark in shib_login_url (1.02 KB, patch)
2019-09-10 16:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23526: Don't use encoded question mark in shib_login_url (1.08 KB, patch)
2019-09-18 13:23 UTC, Matthias Meusburger
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-08-30 17:16:06 UTC
In login_shib_url we place an encoded '?' into the login url:
 65     if ( $query->query_string() ) {
 66         $param = $param . '%3F' . $query->query_string();
 67     }

Then in the templates we filter that using the URL filter:
<p>If you have a Shibboleth account, please <a href="[% shibbolethLoginUrl | url %]">click here to login</a>.</p>


The URL filter turns the % into %25 and the string comes out as
https://anykoha.bywatersolutions.com/Shibboleth.sso/Login?target=https://anykoha.bywatersolutions.com/cgi-bin/koha/opac-search.pl%253Fq=cats

This returns a 4040 on successful login
Comment 1 Nick Clemens 2019-09-10 16:46:41 UTC
Created attachment 92699 [details] [review]
Bug 23526: Don't use encoded question mark in shib_login_url

To test:
1 - Enable shib
2 - Do a search
3 - Try to login from search
4 - Get an error
5 - Apply patch
6 - Retry

Note: You don't haveto really enable shib, you can simply enable it in koha-conf.xml and check the URLs
Comment 2 Matthias Meusburger 2019-09-18 13:23:50 UTC
Created attachment 92939 [details] [review]
Bug 23526: Don't use encoded question mark in shib_login_url

To test:
1 - Enable shib
2 - Do a search
3 - Try to login from search
4 - Get an error
5 - Apply patch
6 - Retry

Note: You don't haveto really enable shib, you can simply enable it in koha-conf.xml and check the URLs
Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Comment 3 Katrin Fischer 2019-09-22 10:28:59 UTC
I am moving this PQA as the change is tiny and I trust Nick and Matthias. Also our RM is a Shibboleth pro.
Comment 4 Martin Renvoize 2019-09-23 09:44:27 UTC
Nice work!

Pushed to master for 19.11.00
Comment 5 Fridolin SOMERS 2019-09-24 06:59:51 UTC
Pushed to 19.05.x for 19.05.04
Comment 6 Fridolin SOMERS 2019-09-24 07:45:44 UTC
Oh but UT needs to be fixed :

Auth_with_shibboleth.t "login shib url" calls :

is(
    login_shib_url($query),
    'https://testopac.com'
      . '/Shibboleth.sso/Login?target='
      . 'https://testopac.com/cgi-bin/koha/opac-user.pl' . '%3F'
      . $query_string,
    "login shib url"
);
Comment 7 Fridolin SOMERS 2019-09-24 08:00:17 UTC
(In reply to Fridolin SOMERS from comment #6)
> Oh but UT needs to be fixed :
> 
> Auth_with_shibboleth.t "login shib url" calls :
> 
> is(
>     login_shib_url($query),
>     'https://testopac.com'
>       . '/Shibboleth.sso/Login?target='
>       . 'https://testopac.com/cgi-bin/koha/opac-user.pl' . '%3F'
>       . $query_string,
>     "login shib url"
> );

Corrected by : 
Bug 23526: (RM follow-up) Fix failing test

Perfect 👌
Comment 8 Lucas Gass 2019-10-17 19:36:01 UTC
backported to 18.11.x for 18.11.11