Created attachment 121276 [details] [review] Bug 28417: Don't use C4::Auth_with_cas if CAS is not used We should follow the same pattern as ldap and not use C4::Auth_with_cas if the cas syspref is not turned on.
This script shows that 3Mb is saved in RAM use Memory::Usage; my $mu = Memory::Usage->new(); $mu->record('Loading'); require Koha::Biblios; $mu->record('module loaded'); print $mu->report();
Test plan: confirm that CAS is still working
(In reply to Jonathan Druart from comment #3) > Test plan: confirm that CAS is still working What's the suggested way to test CAS? I've never used it but I think I thought about trying out https://hub.docker.com/r/apereo/cas/ a little while ago...
(In reply to David Cook from comment #4) > (In reply to Jonathan Druart from comment #3) > > Test plan: confirm that CAS is still working > > What's the suggested way to test CAS? I don't know, Tomas promised me to test this patch.
Created attachment 127103 [details] [review] Bug 28417: Don't use C4::Auth_with_cas if CAS is not used We should follow the same pattern as ldap and not use C4::Auth_with_cas if the cas syspref is not turned on. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This actually fixes something bigger, because C4::Auth was using a couple methods that weren't exported correctly.
Created attachment 127104 [details] [review] Bug 28417: (follow-up) Fix typo - There are CAS demo servers, pick one [1] - They all seem to have the same user/pass as of writing this: User: casuser Password: Mellon To test: 1. Enable CAS and all related preferences. I chose v3 or higher, it only affects the logout action. 2. Set the CAS URL: https://casserver.herokuapp.com/cas (note that if you click on any of the examples, they include a trailing /login, don't put it). 3. Generate a Koha user, with userid: casuser (I also used that for cardnumber, just in case). Set no password to the user, or its irrelevant anyway. Add it stff permissions. 4. Open your Koha OPAC, I use KTD thus: http://kohadev.myDNSname.org:8080 5. Try to login using CAS => SUCCESS: You are redirected to the right login page 6. Login using casuser/Mellon as advertised in the 'demos' site. => SUCCESS: You are logged in! 7. Now logout => SUCCESS: You are sent to CAS so they know you wanted to logout (if you enabled CAS logout) 8. Repeat on the staff interface. => SUCCESS: Things work! 9. Apply this patches, restart all the things: $ restart_all 10. Repeat all in OPAC and admin interfaces => SUCCESS: Things work! 11. Sign off :-D [1] https://apereo.github.io/cas/Demos.html Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I took the liberty to straight PQA. but I'm CC'ing the other David just in case he can give it a try in case I missed some use case. Also, having a simple way to use demo servers will help with other CAS reports. So I want everyone to be familiar.
(In reply to Jonathan Druart from comment #5) > (In reply to David Cook from comment #4) > > (In reply to Jonathan Druart from comment #3) > > > Test plan: confirm that CAS is still working > > > > What's the suggested way to test CAS? > > I don't know, Tomas promised me to test this patch. Sorry for the delay :-/
Created attachment 127105 [details] [review] Bug 28417: Don't use C4::Auth_with_cas if CAS is not used We should follow the same pattern as ldap and not use C4::Auth_with_cas if the cas syspref is not turned on. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 127106 [details] [review] Bug 28417: (follow-up) Fix typo - There are CAS demo servers, pick one [1] - They all seem to have the same user/pass as of writing this: User: casuser Password: Mellon To test: 1. Enable CAS and all related preferences. I chose v3 or higher, it only affects the logout action. 2. Set the CAS URL: https://casserver.herokuapp.com/cas (note that if you click on any of the examples, they include a trailing /login, don't put it). 3. Generate a Koha user, with userid: casuser (I also used that for cardnumber, just in case). Set no password to the user, or its irrelevant anyway. Add it stff permissions. 4. Open your Koha OPAC, I use KTD thus: http://kohadev.myDNSname.org:8080 5. Try to login using CAS => SUCCESS: You are redirected to the right login page 6. Login using casuser/Mellon as advertised in the 'demos' site. => SUCCESS: You are logged in! 7. Now logout => SUCCESS: You are sent to CAS so they know you wanted to logout (if you enabled CAS logout) 8. Repeat on the staff interface. => SUCCESS: Things work! 9. Apply this patches, restart all the things: $ restart_all 10. Repeat all in OPAC and admin interfaces => SUCCESS: Things work! 11. Sign off :-D [1] https://apereo.github.io/cas/Demos.html Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
(In reply to Tomás Cohen Arazi from comment #9) > I took the liberty to straight PQA. but I'm CC'ing the other David just in > case he can give it a try in case I missed some use case. > > Also, having a simple way to use demo servers will help with other CAS > reports. So I want everyone to be familiar. I've added my sign off - not sure I'm the right person for use cases for authentication related things, but things worked as per the test plan. Feel free to cc me in on any CAS bugs that required sign off - you always have great test plans! A couple of minor things I noticed when testing: 1. After step 3 a restart_all is required, otherwise the link for CAS login is not displayed. 2. As mentioned in step 1 using CAS 3 or later for casServerVersion affects the logout action. I found this confusing as it sends you back to the CAS server, then when you click on login it logs you in to the CAS server rather than sending you to Koha - I'm sure there is a use case for that, and probably something those using a CAS environment would be familiar with. Changing casServerVersion to Version 2 or earlier means you get sent back to Koha. 3. For the staff interface the heading before the link is "Cas login", I think this should be "CAS login", but pretty minor really and is the how it is currently.
> 2. As mentioned in step 1 using CAS 3 or later for casServerVersion affects > the logout action. I found this confusing as it sends you back to the CAS > server, then when you click on login it logs you in to the CAS server rather > than sending you to Koha - I'm sure there is a use case for that, and > probably something those using a CAS environment would be familiar with. > Changing casServerVersion to Version 2 or earlier means you get sent back to > Koha. Hm, it should always send you back to Koha. When you start the login process from Koha or any application its URL should be part of the link to the CAS server. It's then used to send you back after a successful login. Part of the problem with logout and the CAS versions is that this parameter name was changed. Maybe the test server is CAS 2 and it doesn't work quite right with both settings because of that? We have a CAS 3 installation and it does work there (18.11 and 20.11). Looking at the patch set I think it looks like it did not change anything in that area.
Tomas, I think your patch is adding back the problem in opac-user.pl. What about the following change? diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 6feb144e508..e610cabc2ab 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -57,8 +57,10 @@ my $query = CGI->new; # CAS single logout handling # Will print header and exit -require C4::Auth_with_cas; -C4::Context->preference('casAuthentication') and C4::Auth_with_cas::logout_if_required($query); +if ( C4::Context->preference('casAuthentication') ) { + require C4::Auth_with_cas; + C4::Auth_with_cas::logout_if_required($query); +} my ( $template, $borrowernumber, $cookie ) = get_template_and_user( {
Created attachment 127249 [details] [review] Bug 28417: Don't require C4::Auth_with_cas from opac-user if not needed
Created attachment 127251 [details] [review] Bug 28417: Don't require C4::Auth_with_cas from opac-user if not needed Note that without the use URI::QueryParam statement in C4::Auth we get: Can't locate object method "query_param_delete" via package "URI::_generic" at /kohadevbox/koha/C4/Auth.pm line 1254
Pushed to master for 21.11, thanks to everybody involved!