[Bug 12398] New: CAS authentication not working
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Bug ID: 12398 Summary: CAS authentication not working Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Authentication Assignee: julian.maurice@biblibre.com Reporter: julian.maurice@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org The CAS authentication doesn't work because the 'ticket' parameter is not removed when building service url which is passed to service_validate(). $query->delete('ticket') works, but $query->url_param() doesn't seem to take into account that parameter was deleted. Checked with last version of CGI. Same behaviour. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 28756 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28756&action=edit Bug 12398: Fix CAS authentication validation CGI::url_param() also returns deleted parameters so we have to check with CGI::param() too. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- To test, you can follow the test plan of bug 12046 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #3 from Julian Maurice <julian.maurice@biblibre.com> --- Test plan of bug 12046 is not enough. So here is the test plan: 1 enable the casAuthentication syspref 2 fill the casServerUrl syspref with a working CAS server url 3 go on the opac home page 4 click on "Log in to your account" link (top right) 5 click on "click here to login" after "if you have a CAS account" 6 login on the cas server authentication page 7 you should be redirected to Koha with an error message 8 apply the patch 9 repeat steps 5 and 6 10 you should be redirected to Koha logged in -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical --- Comment #4 from Julian Maurice <julian.maurice@biblibre.com> --- Changed severity to critical -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Matthias Meusburger <matthias.meusburger@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28756|0 |1 is obsolete| | CC| |matthias.meusburger@biblibr | |e.com --- Comment #5 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Created attachment 28910 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28910&action=edit signed patch Only a few people have the ability to test and sign-off CAS-related patches (mainly because of the lack of available CAS servers I guess). Since I have a CAS server, I've tested it ok (it was indeed broken), and signed it off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Matthias Meusburger <matthias.meusburger@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Working on testing this with the help of Biblibre. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I can't get this to work, even with Matthias help it seems. I'd be glad if someone else willing to test could contact Matthias for the required certificate and give this a try. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28910|0 |1 is obsolete| | --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30048 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30048&action=edit [PASSED QA] Bug 12398: Fix CAS authentication validation CGI::url_param() also returns deleted parameters so we have to check with CGI::param() too. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Took a while to get it working, but I can confirm CAS login is not working without this patch, but does with it. Some notes: In order for this to work you have to add http:// in front of your OpacBaseURL. You will also need a CAS test server and install the certificate on your system. Tested with CAS test server provided by Biblibre. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |tomascohen@gmail.com --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'm not sure about CAS specific stuff, but there are problems with this patch: - No regression tests. - Koha's code usually expects OPACBaseUrl not to contain "http://" in front of it. So, something's wrong. Could you please at least provide a sample call to the function where all the assertions (like "url_param() returns the deleted params too"). Think of a black box test. My feeling is that CAS working with this patch is a side effect of another problem. I am open to comments and willing to push this ASAP. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #10 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Hi, About OPACBaseURL, as already stated in bug 7770 : To my opinion, http:// should not be added automatically, for one simple reason: it just might not be http:// ! OPAC might be using https, and in that case, automatic prefill would be wrong. When talking about CAS, some CAS servers won't allow authentication for http services, only for https one, so we have to be able to use https for the OPAC. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- OpacBaseUrl is kind of an annoying system preference - lots of bugs about it. If the CAS code only allows for https://, maybe it could be part of the CAS code? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm.. bug 8952 and bug 11575, both pushed the master are doing 'stuff' to the prefix in opacbaseurl.. I think it worth looking at those here. For Shibboleth, it is recognised practice to throw away not https requests at the IdP end for security reasons.. so for this reason I've forced https in my patches there.. I don't know if that's the case for CAS but maybe worth considering. As for the bigger problem.. the two bugs above seem to come close at least to fixing that mentioned in bug 7770. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #13 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Katrin, the koha CAS code (client-side) allows to use http or https, what I said is that some CAS servers only allow https. But I think we should keep both in koha. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Matthias, I was only trying to suggest a possible solution here. Tomas is right in that the CAS code requires the OpacBaseURL parameter to be filled out differently than other places in Koha expect it to be and also different to documentation. I am not sure if it won't break other things actually. I passed it anyway, because I felt that this problem was introduced earlier and was not part of the problem that the patch was trying to solve and I didn't want to leave the CAS authentication broken. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hi Matthias, Katrin, So.. the requirement to include http/https in the OpacBaseURL is not a regression caused by this patch. I personally still feel it's wrong, but I can't see it as a reason to fail this particular patch. However, agreeing with Tomas's original point. Could you/julian provide some test code that proves the original assumption regarding url_param not taking note of deleted params? Neither me, nor Tomas could prove this in our tests so we're not really sure that this patch fixes the issue as coincidence, or is actually solving the problem outright. As for the OPACBaseURL stuff, we should move that conversation to one of the other related bugs and maybe open a further bug to remind us that it should also be resolved for the CAS case. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #16 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 30332 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30332&action=edit Bug 12398: Add test for C4::Auth_with_cas::_url_with_get_params Run `prove t/Auth_with_cas.t` to run the test -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Back to signed off, the QA team should mark it passed-qa as soon as the test is tidied so I push it. FTR: I'd put the test in the t/db_dependent dir, and use something like this for the test description (missing in the current one): is(C4::Auth_with_cas::_url_with_get_params($cgi), "$opac_base_url/cgi-bin/koha/opac-user.pl?bar=baz", "_url_with_get_params should return URL without deleted parameters (Bug 12398)"); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #18 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 30333 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30333&action=edit Bug 12398: Use mocks to make the test "database independent" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30333|0 |1 is obsolete| | --- Comment #19 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 30334 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30334&action=edit Bug 12398: Use mocks to make the test "database independent" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, but the patch fails for me when I turn off MySQL. bumblebee:~/kohaclone (91-12398-cas) $ perl t/Auth_with_cas.t 1..1 DBI connect('dbname=koha;host=localhost;port=3306','katrin',...) failed: Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2) at /home/katrin/kohaclone/C4/Context.pm line 802. Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2) at /home/katrin/kohaclone/C4/Context.pm line 802. Compilation failed in require at t/Auth_with_cas.t line 7. BEGIN failed--compilation aborted at t/Auth_with_cas.t line 7. # Looks like your test exited with 255 before it could output anything. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #21 from Julian Maurice <julian.maurice@biblibre.com> --- This is due to C4::Context->preference being called in C4::Auth_with_cas outside of any functions (it gets called on "use"). Rather than writing a workaround for this, I will move the test in t/db_dependent. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30334|0 |1 is obsolete| | --- Comment #22 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 30378 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30378&action=edit Bug 12398: Move t/Auth_with_cas.t to t/db_dependent/Auth_with_cas.t This patch obsoletes the previous one with mocks -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30048|0 |1 is obsolete| | Attachment #30332|0 |1 is obsolete| | Attachment #30378|0 |1 is obsolete| | --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30442 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30442&action=edit [PASSED QA] Bug 12398: Fix CAS authentication validation CGI::url_param() also returns deleted parameters so we have to check with CGI::param() too. Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Took a while to get it working, but I can confirm CAS login is not working without this patch, but does with it. Some notes: In order for this to work you have to add http:// in front of your OpacBaseURL. You will also need a CAS test server and install the certificate on your system. Tested with CAS test server provided by Biblibre. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30443 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30443&action=edit [PASSED QA] Bug 12398: Add test for C4::Auth_with_cas::_url_with_get_params Run `prove t/db_dependent/Auth_with_cas.t` to run the test Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I took the liberty to squash the last 2 patches for the test - both from Julian. As one added a file and the second moved it, this is nicer to read now :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 30454 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30454&action=edit Bug 12398: (followup) tidy unit tests This followup removes unnecesary warnings generated by the test, and also prints a proper message for the tests. Added a can_ok test for all the exported functions btw. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Julian! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolyn.somers@biblibre.co | |m --- Comment #28 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Can this be pushed to 3.16.x ? I have feedbacks from 3.14 libraries that use CAS, its broken and corrected by this patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11219 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Tom M <misilot@fit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misilot@fit.edu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12398 Bug 12398 depends on bug 11219, which changed state. Bug 11219 Summary: CAS authentication fails with URL parameters http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11219 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org