Summary: | CAS authentication fails in OPAC reserve | ||
---|---|---|---|
Product: | Koha | Reporter: | Fridolin Somers <fridolin.somers> |
Component: | Authentication | Assignee: | Fridolin Somers <fridolin.somers> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | bgkriegel, chris, chrish, dpavlin, gmcharlt, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 11219 | ||
Attachments: |
Proposed patch
Proposed patch (revised) Bug 10029 - CAS authentication fails in OPAC reserve [PASSED QA] Bug 10029 - CAS authentication fails in OPAC reserve |
Description
Fridolin Somers
2013-04-11 14:45:23 UTC
Created attachment 17371 [details] [review] Proposed patch See commit message. Patch maybe does not cover the whole problem. Help will be warmly welcomed. Comment on attachment 17371 [details] [review] Proposed patch Review of attachment 17371 [details] [review]: ----------------------------------------------------------------- ::: C4/Auth_with_cas.pm @@ +95,5 @@ > my $casparam = $query->param('cas'); > # FIXME: This should be more generic and handle whatever parameters there might be > + if (defined $casparam) { > + $uri .= ($uri =~ m/\?/ ) ? '&' : '?'; > + $uri .= "cas=$casparam" ; You shouldn't need this, because $query->url will already contain the casparam, ie, if it exists it will be in $query->url, so we will be appending another copy of it. So if you remove this and the FIXME then I'd be happy to sign off. Created attachment 17609 [details] [review] Proposed patch (revised) Thanks a lot for your help, her is the revised patch. Created attachment 18394 [details] [review] Bug 10029 - CAS authentication fails in OPAC reserve If OPAC reserve page is accessed without being logged-in, login form is displayed as well as a CAS authentication link (if enabled). A click on this link will lead to CAS server but one comming back to Koha, page shows an error : "ERROR: No biblionumber received". This is because CAS link only contains the query path "/cgi-bin/koha/opac-reserve.pl", not the query parameters. This patch adds query parameters to URI sent to CAS. Test plan : - Enable CAS - Go to opac without been logged-in - Try to place hold on a record => You get to /cgi-bin/koha/opac-reserve.pl?biblionumber=XXX showing authentication page => Check that CAS link contains query param "biblionumber" - Click on CAS link and log in => Check you return well logged-in to reserve page with biblionumber param Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Can't QA this as I signed off, can someone else on the QA team take a look please Created attachment 19040 [details] [review] [PASSED QA] Bug 10029 - CAS authentication fails in OPAC reserve If OPAC reserve page is accessed without being logged-in, login form is displayed as well as a CAS authentication link (if enabled). A click on this link will lead to CAS server but one comming back to Koha, page shows an error : "ERROR: No biblionumber received". This is because CAS link only contains the query path "/cgi-bin/koha/opac-reserve.pl", not the query parameters. This patch adds query parameters to URI sent to CAS. Test plan : - Enable CAS - Go to opac without been logged-in - Try to place hold on a record => You get to /cgi-bin/koha/opac-reserve.pl?biblionumber=XXX showing authentication page => Check that CAS link contains query param "biblionumber" - Click on CAS link and log in => Check you return well logged-in to reserve page with biblionumber param Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> I have followed the test plan as far as I could and the links contain the biblionumber now, which they didn't before. I couldn't check the CAS login, but my normal login worked as expected. All tests and the QA script pass. Pushed to master. Thanks, Fridolyn! This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Fridolyn! Pushed to 3.10.x, will be in 3.10.10 Pushed to 3.8.x, will be in 3.8.17 |