Bugzilla – Attachment 83142 Details for
Bug 21993
Be userfriendly when the CSRF token is wrong
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21993: Display a user-friendly message when the CSRF token is wrong
Bug-21993-Display-a-user-friendly-message-when-the.patch (text/plain), 14.99 KB, created by
Jonathan Druart
on 2018-12-12 18:59:17 UTC
(
hide
)
Description:
Bug 21993: Display a user-friendly message when the CSRF token is wrong
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2018-12-12 18:59:17 UTC
Size:
14.99 KB
patch
obsolete
>From dfa42fe7b3278524f5543911c5d4e81b86c68f85 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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=UTF-8 >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 >--- > 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 @@ > <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 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' %] > <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 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 @@ > > <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › <a href="/cgi-bin/koha/tools/import_borrowers.pl">Import patrons</a>[% IF ( uploadborrowers ) %] › 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 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 @@ > > <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › [% IF ( TOTAL ) %]<a href="/cgi-bin/koha/tools/picture-upload.pl">Upload patron images</a> › 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 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 5000fb2f85..d4ec73c764 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 db7a447ffa..c179feda97 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 e886c41976..f9305076a7 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -72,7 +72,7 @@ if ( $newpassword and not @errors ) { > > 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 731d4f9c56..34e8fc91f9 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -307,7 +307,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.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=83142">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21993
:
83142
|
83389
|
83675