Bug 10029 - CAS authentication fails in OPAC reserve
Summary: CAS authentication fails in OPAC reserve
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 11219
  Show dependency treegraph
 
Reported: 2013-04-11 14:45 UTC by Fridolin Somers
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (1.95 KB, patch)
2013-04-11 14:54 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (revised) (1.83 KB, patch)
2013-04-23 09:38 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10029 - CAS authentication fails in OPAC reserve (1.88 KB, patch)
2013-05-24 20:56 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 10029 - CAS authentication fails in OPAC reserve (2.17 KB, patch)
2013-06-16 15:18 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-04-11 14:45:23 UTC
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.
Comment 1 Fridolin Somers 2013-04-11 14:54:57 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-04-20 10:28:08 UTC
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.
Comment 3 Fridolin Somers 2013-04-23 09:38:06 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2013-05-24 20:56:31 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2013-06-08 04:01:50 UTC
Can't QA this as I signed off, can someone else on the QA team take a look please
Comment 6 Katrin Fischer 2013-06-16 15:18:12 UTC
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.
Comment 7 Galen Charlton 2013-07-02 14:13:47 UTC
Pushed to master.  Thanks, Fridolyn!
Comment 8 Tomás Cohen Arazi 2013-07-15 23:42:42 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Fridolyn!
Comment 9 Bernardo Gonzalez Kriegel 2013-08-03 18:13:28 UTC
Pushed to 3.10.x, will be in 3.10.10
Comment 10 Chris Hall 2013-08-17 23:09:46 UTC
Pushed to 3.8.x, will be in 3.8.17