From 466e29263910c85ce6417f322dc505558eeaa8a3 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Sat, 27 Oct 2018 09:11:33 -0300 Subject: [PATCH] Bug 21702: Make mancredit.pl use the patron id for the staff user I think unsafe SQL modes and the fact that manager_id has no FK allowed this to go unnoticed. But now we catch it! To test: - On a patron, try adding a new manual credit of any type => FAIL: It fails telling the userid of the logged user is not a valid integer! - Apply this patch - Try adding a manual credit of any type => SUCCESS: Manual credit added! - Sign off! Signed-off-by: Tomas Cohen Arazi Signed-off-by: Andrew Isherwood Signed-off-by: Josef Moravec Signed-off-by: Nick Clemens --- members/mancredit.pl | 90 +++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 48 deletions(-) diff --git a/members/mancredit.pl b/members/mancredit.pl index 813461eaeb..49ab8a4213 100755 --- a/members/mancredit.pl +++ b/members/mancredit.pl @@ -33,73 +33,67 @@ use C4::Accounts; use C4::Items; use C4::Members::Attributes qw(GetBorrowerAttributes); -use Koha::Account; use Koha::Items; use Koha::Patrons; use Koha::Patron::Categories; use Koha::Token; my $input=new CGI; -my $flagsrequired = { borrowers => 'edit_borrowers', updatecharges => 1 }; -my $borrowernumber=$input->param('borrowernumber'); +my ($template, $loggedinuser, $cookie) = get_template_and_user( + { + template_name => "members/mancredit.tt", + query => $input, + type => "intranet", + authnotrequired => 0, + flagsrequired => { borrowers => 'edit_borrowers', + updatecharges => 'remaining_permissions' } + } +); + +my $logged_in_user = Koha::Patrons->find($loggedinuser) or die "Not logged in"; +my $borrowernumber = $input->param('borrowernumber'); +my $patron = Koha::Patrons->find($borrowernumber); -my $patron = Koha::Patrons->find( $borrowernumber ); unless ( $patron ) { print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber"); exit; } -my $add=$input->param('add'); +my $add = $input->param('add'); if ($add){ - my ( $user_id ) = checkauth( $input, 0, $flagsrequired, 'intranet' ); - - if ( $user_id ) { - - die "Wrong CSRF token" - unless Koha::Token->new->check_csrf( { - session_id => scalar $input->cookie('CGISESSID'), - token => scalar $input->param('csrf_token'), - }); - - # Note: If the logged in user is not allowed to see this patron an invoice can be forced - # Here we are trusting librarians not to hack the system - my $barcode = $input->param('barcode'); - my $item_id; - if ($barcode) { - my $item = Koha::Items->find({barcode => $barcode}); - $item_id = $item->itemnumber if $item; - } - my $description = $input->param('desc'); - my $note = $input->param('note'); - my $amount = $input->param('amount') || 0; - my $type = $input->param('type'); - - Koha::Account->new({ patron_id => $borrowernumber })->add_credit({ - amount => $amount, - description => $description, - item_id => $item_id, - note => $note, - type => $type, - user_id => $user_id + die "Wrong CSRF token" + unless Koha::Token->new->check_csrf( { + session_id => scalar $input->cookie('CGISESSID'), + token => scalar $input->param('csrf_token'), }); - print $input->redirect("/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber"); + # Note: If the logged in user is not allowed to see this patron an invoice can be forced + # Here we are trusting librarians not to hack the system + my $barcode = $input->param('barcode'); + my $item_id; + if ($barcode) { + my $item = Koha::Items->find({barcode => $barcode}); + $item_id = $item->itemnumber if $item; } + my $description = $input->param('desc'); + my $note = $input->param('note'); + my $amount = $input->param('amount') || 0; + my $type = $input->param('type'); + + $patron->account->add_credit({ + amount => $amount, + description => $description, + item_id => $item_id, + note => $note, + type => $type, + user_id => $logged_in_user->id + }); + + print $input->redirect("/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber"); + } else { - my ($template, $loggedinuser, $cookie) = get_template_and_user( - { - template_name => "members/mancredit.tt", - query => $input, - type => "intranet", - authnotrequired => 0, - flagsrequired => { borrowers => 'edit_borrowers', - updatecharges => 'remaining_permissions' }, - debug => 1, - } - ); - my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); if (C4::Context->preference('ExtendedPatronAttributes')) { -- 2.39.5