The authentication code needs to be rewritten. There were several attempts (bugs 7174, 18315) and other bugs without patch that talk about it (12547, 12548, 15204). Bug 13503 says that before rewriting one of the most sensible area of Koha, we need to write tests. Bug 19181 contain some tests, but they require Selenium, firefox, xfvb, java, and a running web server. With Mojolicious we can write equivalent tests without all those dependencies.
Created attachment 74709 [details] [review] Bug 20635: Write authentication tests with Test::Mojo The authentication code needs to be rewritten. There were several attempts (bugs 7174, 18315) and other bugs without patch that talk about it (12547, 12548, 15204). Bug 13503 says that before rewriting one of the most sensible area of Koha, we need to write tests. Bug 19181 contain some tests, but they require Selenium, firefox, xfvb, java, and a running web server. With Mojolicious we can write equivalent tests without all those dependencies. This patch add basic tests for password and CAS authentication. Test plan: 1. Apply bug 20582 2. Apply this patch 3. Run `prove t/integration/auth`
Just tried, but without any success, not sure what's wrong here: 1..2 # Subtest: intranet 1..10 Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68. A context appears to have been destroyed without first calling release(). Based on $@ it does not look like an exception was thrown (this is not always a reliable test) This is a problem because the global error variables ($!, $@, and $?) will not be restored. In addition some release callbacks will not work properly from inside a DESTROY method. Here are the context creation details, just in case a tool forgot to call release(): File: t/integration/auth/password.t Line: 58 Tool: Test::More::subtest Cleaning up the CONTEXT stack... # Test ended with extra hubs on the stack! # No tests run!
prove -v t/integration/auth/ t/integration/auth/cas.t ....... 1..2 # Subtest: intranet 1..10 [Tue Jul 2 05:13:43 2019] [debug] GET "/cgi-bin/koha/mainpage.pl" [Tue Jul 2 05:13:43 2019] [debug] Routing to a callback A context appears to have been destroyed without first calling release(). Based on $@ it does not look like an exception was thrown (this is not always a reliable test) This is a problem because the global error variables ($!, $@, and $?) will not be restored. In addition some release callbacks will not work properly from inside a DESTROY method. Here are the context creation details, just in case a tool forgot to call release(): File: t/integration/auth/cas.t Line: 74 Tool: Test::More::subtest Cleaning up the CONTEXT stack... # Test ended with extra hubs on the stack! # No tests run! Dubious, test returned 255 (wstat 65280, 0xff00) Failed 2/2 subtests t/integration/auth/password.t .. 1..2 # Subtest: intranet 1..10 [Tue Jul 2 05:13:46 2019] [debug] GET "/cgi-bin/koha/mainpage.pl" [Tue Jul 2 05:13:46 2019] [debug] Routing to a callback A context appears to have been destroyed without first calling release(). Based on $@ it does not look like an exception was thrown (this is not always a reliable test) This is a problem because the global error variables ($!, $@, and $?) will not be restored. In addition some release callbacks will not work properly from inside a DESTROY method. Here are the context creation details, just in case a tool forgot to call release(): File: t/integration/auth/password.t Line: 58 Tool: Test::More::subtest Cleaning up the CONTEXT stack... # Test ended with extra hubs on the stack! # No tests run! Dubious, test returned 255 (wstat 65280, 0xff00) Failed 2/2 subtests Test Summary Report ------------------- t/integration/auth/cas.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 2 tests but ran 0. t/integration/auth/password.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 2 tests but ran 0. Files=2, Tests=0, 5 wallclock secs ( 0.02 usr 0.00 sys + 4.16 cusr 0.59 csys = 4.77 CPU) Result: FAIL -- confirmed the same problem in comment #2, marking Failed QA. Though, I don't know if it is this patch, or the one it depends on, that is the problem.
This has something to do with the safe_exit subroutine in C4::Auth, called by checkauth, which calls exit because psgi_env returns false. %ENV does not contain any 'psgi.' variables, but I don't know why...
Created attachment 91176 [details] [review] Bug 20635: Make sure CGI::Compile is loaded first in tests CGI::Compile needs to be loaded before Koha modules for it to work normally. Even if not directly needed in test files, we need to 'use' it explicitely so that it loads before Koha modules.
The last patch fixes the problem mentioned in comment 2 and comment 3, and make t/integration/auth/password.t pass. However the CAS test still fails, for other reasons.