Bug 30524: Core CSRF checking code

Split out from bug 22990 as requested.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 30524: Add xt/find-missing-csrf.t

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 30524: (QA follow-up) Polishing xt script

Test plan:
Run it again. Same results?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 30524: Unit tests

Test plan:
Run t/Output.t
Run t/db_dependent/Auth.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 84450df291)
Signed-off-by: danyonsewell <danyonsewell@catalyst.net.nz>

C4/Auth.pm

@@ -49,6 +49,7 @@ use C4::Auth_with_shibboleth qw( shib_ok get_login_shib login_shib_url logout_sh
 use Net::CIDR;
 use C4::Log qw( logaction );
 use Koha::CookieManager;
+use Koha::Token;
 
 # use utf8;
 
@@ -299,6 +300,7 @@ sub get_template_and_user {
         $template->param( loggedinusernumber => $borrowernumber ); # FIXME Should be replaced with logged_in_user.borrowernumber
         $template->param( logged_in_user     => $patron );
         $template->param( sessionID          => $sessionID );
+        $template->param( csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $sessionID }));
 
         if ( $in->{'type'} eq 'opac' ) {
             require Koha::Virtualshelves;

C4/Output.pm

@@ -34,6 +34,7 @@ use URI::Escape;
 use C4::Auth qw( get_template_and_user );
 use C4::Context;
 use C4::Templates;
+use Koha::Token;
 
 our (@ISA, @EXPORT_OK);
 
@@ -314,9 +315,17 @@ sub is_ajax {
 To executed at the beginning of scripts to stop the script at this point if
 some errors are found.
 
-Tests for module 'members':
-* patron is not defined (we are looking for a patron that does no longer exist/never existed)
-* The logged in user cannot see patron's infos (feature 'cannot_see_patron_infos')
+A series of tests can be run for a given module, or a specific check.
+Params "module" and "check" are mutually exclusive.
+
+Tests for modules:
+* members:
+    - Patron is not defined (we are looking for a patron that does no longer exist/never existed)
+    - The logged in user cannot see patron's infos (feature 'cannot_see_patron_infos')
+
+Tests for specific check:
+* csrf_token
+    will test if the csrf_token CGI param is valid
 
 Others will be added here depending on the needs (for instance biblio does not exist will be useful).
 
@@ -332,16 +341,28 @@ sub output_and_exit_if_error {
             if ( not $current_patron ) {
                 $error = 'unknown_patron';
             }
-            elsif( not $logged_in_user->can_see_patron_infos( $current_patron ) ) {
+            elsif ( not $logged_in_user->can_see_patron_infos($current_patron) )
+            {
                 $error = 'cannot_see_patron_infos';
             }
-        } elsif ( $params->{module} eq 'cataloguing' ) {
+        }
+        elsif ( $params->{module} eq 'cataloguing' ) {
             # We are testing the record to avoid additem to fetch the Koha::Biblio
             # But in the long term we will want to get a biblio in parameter
             $error = 'unknown_biblio' unless $params->{record};
         }
     }
-
+    elsif ( $params and exists $params->{check} ) {
+        if ( $params->{check} eq 'csrf_token' ) {
+            $error = 'wrong_csrf_token'
+              unless Koha::Token->new->check_csrf(
+                {
+                    session_id => scalar $query->cookie('CGISESSID'),
+                    token      => scalar $query->param('csrf_token'),
+                }
+              );
+        }
+    }
     output_and_exit( $query, $cookie, $template, $error ) if $error;
     return;
 }

koha-tmpl/intranet-tmpl/prog/en/includes/csrf-token.inc

@@ -0,0 +1 @@
+<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />

t/Output.t

@@ -17,16 +17,23 @@
 
 use Modern::Perl;
 
-use Test::More tests => 7;
+use Test::More tests => 8;
 use Test::Warn;
+use Test::MockModule;
+
+use File::Temp qw/tempfile/;
 use CGI qw ( -utf8 );
 
+use C4::Auth qw( get_template_and_user );
+
 use t::lib::Mocks;
 
 BEGIN {
-    use_ok('C4::Output', qw( output_html_with_http_headers parametrized_url  ));
+    use_ok('C4::Output', qw( output_html_with_http_headers output_and_exit_if_error parametrized_url ));
 }
 
+our $output_module = Test::MockModule->new('C4::Output');
+
 my $query = CGI->new();
 my $cookie;
 my $output = 'foobarbaz';
@@ -93,3 +100,33 @@ subtest 'output_with_http_headers() tests' => sub {
     like($stdout, qr/Access-control-allow-origin: https:\/\/koha-community\.org/, 'Header set to https://koha-community.org');
     close STDOUT;
 };
+
+subtest 'output_and_exit_if_error() tests' => sub {
+    plan tests => 1;
+
+    $output_module->mock(
+        'output_and_exit',
+        sub {
+            my ( $query, $cookie, $template, $error ) = @_;
+            is( $error, 'wrong_csrf_token', 'Got right error message' );
+        }
+    );
+
+    t::lib::Mocks::mock_config( 'pluginsdir', [ C4::Context::temporary_directory ] );
+    my ( $fh, $fn ) = tempfile( SUFFIX => '.tt', UNLINK => 1, DIR => C4::Context::temporary_directory );
+    print $fh qq|[% blocking_error %]|;
+    close $fh;
+
+    my $query = CGI->new();
+    $query->param('csrf_token','');
+    my ( $template, $loggedinuser, $cookies ) = get_template_and_user(
+        {
+            template_name   => $fn,
+            query           => $query,
+            type            => "intranet",
+            authnotrequired => 1,
+        }
+    );
+    # Next call triggers test in the mocked sub
+    output_and_exit_if_error($query, $cookie, $template, { check => 'csrf_token' });
+};

t/db_dependent/Auth.t

@@ -388,6 +388,7 @@ subtest 'get_template_and_user' => sub {   # Tests for the language URL paramete
     );
     is($template->{VARS}->{'opac_name'}, "multibranch-19", "Opac name was set correctly");
     is($template->{VARS}->{'opac_search_limit'}, "branch:multibranch-19", "Search limit was set correctly");
+    ok(defined($template->{VARS}->{'csrf_token'}), "CSRF token returned");
 
     delete $ENV{"HTTP_COOKIE"};
 };