Bug 21993: Display a user-friendly message when the CSRF token is wrong

Instead of dying!

Test plan:
Assuming you have a patron with borrowernumber=51 and another one that
can be deleted with borrowernumber=42

- authorities-home.pl
 * Delete an authority record
 * hit /cgi-bin/koha/authorities/authorities-home.pl?op=delete

- basket/sendbasket.pl
 * Send a basket to someone
 * hit /cgi-bin/koha/basket/sendbasket.pl?email_add=1

- members/apikeys.pl
  * Generate and delete an API key for a patron
  * hit /cgi-bin/koha/members/apikeys.pl?patron_id=51&op=delete

- members/deletemem.pl
  * Delete a patron
  * hit /cgi-bin/koha/members/deletemem.pl?member=42&op=delete_confirmed

- members/mancredit.pl
  * Add a manual credit
  * hit /cgi-bin/koha/members/mancredit.pl?borrowernumber=51&add=1

- members/maninvoice.pl
  * Add a manual invoice
  * hit /cgi-bin/koha/members/maninvoice.pl?borrowernumber=51&add=1

- members/member-flags.pl
  * Change permissions for a patron
  * hit /cgi-bin/koha/members/member-flags.pl?member=51&newflags=1

- members/member-password.pl
  * Change the password for a patron (from the staff interface)
  * hit /cgi-bin/koha/members/member-password.pl?member=51&newpassword=aA1

- members/memberentry.pl
  * Edit some patron's info
  * hit /cgi-bin/koha/members/memberentry.pl?borrowernumber=51&op=save

- members/paycollect.pl
  * Pay an individual fine
  * hit something like /cgi-bin/koha/members/paycollect.pl?borrowernumber=51&pay_individual=1&accounttype=L&amount=1.00&amountoutstanding=1.00&accountlines_id=157&paid=1
  You may need to edit some values

- tools/import_borrowers.pl
  * Import some patrons
  * hit /cgi-bin/koha/tools/import_borrowers.pl?uploadborrowers=1

- tools/picture-upload.pl
  * Upload an image for a patron
  * You will need to edit the html content
  hit Home › Tools › Upload patron images
  then locate the csrf_token input and modify its value

Note for QA:
- Opac is not done as blocking_errors.inc does not exist for this
interface
- ill/ill-requests.pl
I did not manage to replace this occurrence

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/authorities/authorities-home.pl b/authorities/authorities-home.pl
index 9eb3422..7c17b1d 100755 (executable)
--- a/authorities/authorities-home.pl
+++ b/authorities/authorities-home.pl
@@ -61,10 +61,11 @@ if ( $op eq "delete" ) {
         }
     );
 
-    die "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, 'wrong_csrf_token' )
+        unless Koha::Token->new->check_csrf({
+            session_id => scalar $query->cookie('CGISESSID'),
+            token  => scalar $query->param('csrf_token'),
+        });
 
     DelAuthority({ authid => $authid });
 

diff --git a/basket/sendbasket.pl b/basket/sendbasket.pl
index a4bbe16..2076c42 100755 (executable)
--- a/basket/sendbasket.pl
+++ b/basket/sendbasket.pl
@@ -50,10 +50,11 @@ my $email_add    = $query->param('email_add');
 my $dbh          = C4::Context->dbh;
 
 if ( $email_add ) {
-    die "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, 'wrong_csrf_token' )
+        unless Koha::Token->new->check_csrf({
+            session_id => scalar $query->cookie('CGISESSID'),
+            token  => scalar $query->param('csrf_token'),
+        });
     my $email = Koha::Email->new();
     my %mail = $email->create_message_headers({ to => $email_add });
     my $comment    = $query->param('comment');

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
index 2666400..c734706 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
@@ -11,6 +11,8 @@
         <div class="dialog message">This subscription does not exist.</div>
     [% CASE 'unknown_basket' %]
         <div class="dialog message">This basket does not exist.</div>
+    [% CASE 'wrong_csrf_token' %]
+        <div class="dialog message">The form submission failed (Wrong CSRF token). Try to come back, refresh the page, then try again.</div>
     [% CASE %][% blocking_error | html %]
     [% END %]
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasketform.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasketform.tt
index 4071e81..087d5af 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasketform.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasketform.tt
@@ -16,6 +16,7 @@
        
 [% ELSE %]
 
+[% INCLUDE 'blocking_errors.inc' %]
 <form action="/cgi-bin/koha/basket/sendbasket.pl" method="post">
 
 <fieldset class="rows"> 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
index 1ced2d5..48ee993 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
@@ -20,6 +20,7 @@
 
 <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo; <a href="/cgi-bin/koha/tools/import_borrowers.pl">Import patrons</a>[% IF ( uploadborrowers ) %] &rsaquo; Results[% END %]</div>
 
+[% INCLUDE 'blocking_errors.inc' %]
 <div class="main container-fluid">
     <div class="row">
         <div class="col-sm-10 col-sm-push-2">

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt
index d44de61..38ea53f 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt
@@ -13,6 +13,7 @@
 
 <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo; [% IF ( TOTAL ) %]<a href="/cgi-bin/koha/tools/picture-upload.pl">Upload patron images</a> &rsaquo; Results[% ELSE %]Upload patron images[% END %] </div>
 
+[% INCLUDE 'blocking_errors.inc' %]
 <div class="main container-fluid">
     <div class="row">
         <div class="col-sm-10 col-sm-push-2">

diff --git a/members/apikeys.pl b/members/apikeys.pl
index 9d1a4f1..ffbd40d 100755 (executable)
--- a/members/apikeys.pl
+++ b/members/apikeys.pl
@@ -60,11 +60,11 @@ if ( $op eq 'generate' or
      $op eq 'revoke' or
      $op eq 'activate' ) {
 
-    die "Wrong CSRF token"
-    unless Koha::Token->new->check_csrf({
-        session_id => scalar $cgi->cookie('CGISESSID'),
-        token      => scalar $cgi->param('csrf_token'),
-    });
+    output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' )
+        unless Koha::Token->new->check_csrf({
+            session_id => scalar $cgi->cookie('CGISESSID'),
+            token      => scalar $cgi->param('csrf_token'),
+        });
 }
 
 if ($op) {

diff --git a/members/deletemem.pl b/members/deletemem.pl
index 4789cf4..3d8712e 100755 (executable)
--- a/members/deletemem.pl
+++ b/members/deletemem.pl
@@ -109,7 +109,7 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor )
     }
 } elsif ( $op eq 'delete_confirmed' ) {
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf( {
             session_id => $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/members/mancredit.pl b/members/mancredit.pl
index 7c5ec3c..6da0fe2 100755 (executable)
--- a/members/mancredit.pl
+++ b/members/mancredit.pl
@@ -62,7 +62,7 @@ my $add = $input->param('add');
 
 if ($add){
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf( {
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/members/maninvoice.pl b/members/maninvoice.pl
index e101d07..2d22666 100755 (executable)
--- a/members/maninvoice.pl
+++ b/members/maninvoice.pl
@@ -40,6 +40,15 @@ use Koha::Patron::Categories;
 
 my $input=new CGI;
 my $flagsrequired = { borrowers => 'edit_borrowers' };
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {   template_name   => "members/maninvoice.tt",
+        query           => $input,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => $flagsrequired,
+        debug           => 1,
+    }
+);
 
 my $borrowernumber=$input->param('borrowernumber');
 
@@ -52,7 +61,7 @@ unless ( $patron ) {
 my $add=$input->param('add');
 if ($add){
     if ( checkauth( $input, 0, $flagsrequired, 'intranet' ) ) {
-        die "Wrong CSRF token"
+        output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
             unless Koha::Token->new->check_csrf( {
                 session_id => scalar $input->cookie('CGISESSID'),
                 token => scalar $input->param('csrf_token'),
@@ -71,15 +80,6 @@ if ($add){
         my $note    = $input->param('note');
         my $error   = manualinvoice( $borrowernumber, $itemnum, $desc, $type, $amount, $note );
         if ($error) {
-            my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
-                {   template_name   => "members/maninvoice.tt",
-                    query           => $input,
-                    type            => "intranet",
-                    authnotrequired => 0,
-                    flagsrequired   => $flagsrequired,
-                    debug           => 1,
-                }
-            );
             if ( $error =~ /FOREIGN KEY/ && $error =~ /itemnumber/ ) {
                 $template->param( 'ITEMNUMBER' => 1 );
             }

diff --git a/members/member-flags.pl b/members/member-flags.pl
index 40ea583..10324dd 100755 (executable)
--- a/members/member-flags.pl
+++ b/members/member-flags.pl
@@ -52,7 +52,7 @@ $member2{'borrowernumber'}=$member;
 
 if ($input->param('newflags')) {
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/members/member-password.pl b/members/member-password.pl
index 34795c3..013e7e5 100755 (executable)
--- a/members/member-password.pl
+++ b/members/member-password.pl
@@ -64,7 +64,7 @@ push( @errors, 'NOMATCH' ) if ( ( $newpassword && $newpassword2 ) && ( $newpassw
 
 if ( $newpassword and not @errors) {
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/members/memberentry.pl b/members/memberentry.pl
index 036c27e..788f76c 100755 (executable)
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -306,7 +306,7 @@ $debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled
 my $extended_patron_attributes = ();
 if ($op eq 'save' || $op eq 'insert'){
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/members/paycollect.pl b/members/paycollect.pl
index a15267f..abd96b5 100755 (executable)
--- a/members/paycollect.pl
+++ b/members/paycollect.pl
@@ -112,7 +112,7 @@ if ( $total_paid and $total_paid ne '0.00' ) {
             total_due => $total_due
         );
     } else {
-        die "Wrong CSRF token"
+        output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
             unless Koha::Token->new->check_csrf( {
                 session_id => $input->cookie('CGISESSID'),
                 token  => scalar $input->param('csrf_token'),

diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index bba512f..ec784be 100755 (executable)
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -111,7 +111,7 @@ my $patronlistname = $uploadborrowers . ' (' . $timestamp .')';
 $template->param( SCRIPT_NAME => '/cgi-bin/koha/tools/import_borrowers.pl' );
 
 if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),

diff --git a/tools/picture-upload.pl b/tools/picture-upload.pl
index 5f99906..54f18b5 100755 (executable)
--- a/tools/picture-upload.pl
+++ b/tools/picture-upload.pl
@@ -83,7 +83,7 @@ our %errors = ();
 # Case is important in these operational values as the template must use case to be visually pleasing!
 if ( ( $op eq 'Upload' ) && $uploadfile ) {
 
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),
@@ -172,7 +172,7 @@ elsif ( ( $op eq 'Upload' ) && !$uploadfile ) {
     $template->param( filetype   => $filetype );
 }
 elsif ( $op eq 'Delete' ) {
-    die "Wrong CSRF token"
+    output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
         unless Koha::Token->new->check_csrf({
             session_id => scalar $input->cookie('CGISESSID'),
             token  => scalar $input->param('csrf_token'),