From b990b953b324dc18c013f94d1828b61bb5118c60 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 12 Dec 2018 14:59:54 -0300 Subject: [PATCH] Bug 21993: Display a user-friendly message when the CSRF token is wrong MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Nick Clemens --- authorities/authorities-home.pl | 9 +++++---- basket/sendbasket.pl | 9 +++++---- .../prog/en/includes/blocking_errors.inc | 2 ++ .../prog/en/modules/basket/sendbasketform.tt | 1 + .../prog/en/modules/tools/import_borrowers.tt | 1 + .../prog/en/modules/tools/picture-upload.tt | 1 + members/apikeys.pl | 10 +++++----- members/deletemem.pl | 2 +- members/mancredit.pl | 2 +- members/maninvoice.pl | 20 +++++++++---------- members/member-flags.pl | 2 +- members/member-password.pl | 2 +- members/memberentry.pl | 2 +- members/paycollect.pl | 2 +- tools/import_borrowers.pl | 2 +- tools/picture-upload.pl | 4 ++-- 16 files changed, 39 insertions(+), 32 deletions(-) diff --git a/authorities/authorities-home.pl b/authorities/authorities-home.pl index 9eb342228e..7c17b1df02 100755 --- 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 a4bbe16bf6..2076c425fa 100755 --- 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 26664001fc..c73470694b 100644 --- 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 @@
This subscription does not exist.
[% CASE 'unknown_basket' %]
This basket does not exist.
+ [% CASE 'wrong_csrf_token' %] +
The form submission failed (Wrong CSRF token). Try to come back, refresh the page, then try again.
[% 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 4071e81fd8..087d5afb65 100644 --- 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' %]
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 1ced2d5a49..48ee993b68 100644 --- 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 @@ +[% INCLUDE 'blocking_errors.inc' %]
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 d44de61b54..38ea53f6df 100644 --- 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 @@ +[% INCLUDE 'blocking_errors.inc' %]
diff --git a/members/apikeys.pl b/members/apikeys.pl index 9d1a4f12d5..ffbd40d329 100755 --- 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 4789cf4fb1..3d8712ede7 100755 --- 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 7c5ec3c603..6da0fe256f 100755 --- 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 e101d07fe2..2d22666bd5 100755 --- 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 40ea583511..10324dd959 100755 --- 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 34795c30ab..013e7e5ae3 100755 --- 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 036c27e186..788f76ce13 100755 --- 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 a15267fa3d..abd96b5de3 100755 --- 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 bba512f5f8..ec784be2d9 100755 --- 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 5f99906d6d..54f18b53ad 100755 --- 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'), -- 2.39.5