From 1213b9311f59055b50f1d46d0d1d18be579039d9 Mon Sep 17 00:00:00 2001 From: Lucas Gass Date: Wed, 7 Jun 2023 18:40:48 +0000 Subject: [PATCH] Bug 33176: Enforce bad values 1. Turn on RequirePaymentType 2. Create a manual invocie on a patron account 3. Go to pay it, 'Payment type:' is marked as required. 4. In the inscept the select input ( #payment_type ) with your browser's dev tools. Removed the required attribute. 5. You are able to make the payment without a payment type. 6. Apply patch and restart_all 7. Try 4-5 again. This time you should get a 500 error and the payment should not go through. 8. Try a paymnet again this time manipulate the DOM and change the value of 'CASH' to something else like 'SOMETHINGELSE'. 9. Try to submit the payment and again you will get a 500 error. The payment should not go through. 10. Turn RequirePaymentType off. Try a payment with a payment type, you shoud be successful. 11. Make sure tests will pass Signed-off-by: Sam Lau Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 676d263687ad666184fce6d505af6d9672412732) Signed-off-by: Martin Renvoize (cherry picked from commit 6be23e725134cf2d5f20279b209b0a8aa62aaa90) Signed-off-by: Pedro Amorim --- Koha/Account.pm | 16 ++++++++++++---- Koha/Exceptions/Account.pm | 4 ++++ t/db_dependent/Koha/Account.t | 18 +++++++++++++++--- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Koha/Account.pm b/Koha/Account.pm index 030802468d..d3da13a9db 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -84,6 +84,18 @@ sub pay { my $userenv = C4::Context->userenv; + Koha::Exceptions::Account::PaymentTypeRequired->throw() + if ( C4::Context->preference("RequirePaymentType") + && !defined($payment_type) ); + + my $av = Koha::AuthorisedValues->search_with_library_limits({ category => 'PAYMENT_TYPE', authorised_value => $payment_type }); + + if ( !$av->count && C4::Context->preference("RequirePaymentType")) { + Koha::Exceptions::Account::InvalidPaymentType->throw( + error => 'Invalid payment type' + ); + } + my $manager_id = $userenv ? $userenv->{number} : undef; my $interface = $params ? ( $params->{interface} || C4::Context->interface ) : C4::Context->interface; my $payment = $self->payin_amount( @@ -102,10 +114,6 @@ sub pay { } ); - Koha::Exceptions::Account::PaymentTypeRequired->throw() - if ( C4::Context->preference("RequirePaymentType") - && !defined($payment_type) ); - # NOTE: Pay historically always applied as much credit as it could to all # existing outstanding debits, whether passed specific debits or otherwise. if ( $payment->amountoutstanding ) { diff --git a/Koha/Exceptions/Account.pm b/Koha/Exceptions/Account.pm index 26bbac593c..0b36f0818f 100644 --- a/Koha/Exceptions/Account.pm +++ b/Koha/Exceptions/Account.pm @@ -51,6 +51,10 @@ use Exception::Class ( 'Koha::Exceptions::Account::PaymentTypeRequired' => { isa => 'Koha::Exceptions::Account', description => 'Account transaction requires a payment type' + }, + 'Koha::Exceptions::Account::InvalidPaymentType' => { + isa => 'Koha::Exceptions::Account', + description => 'Invalid payment type' } ); diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index c6c60ea911..aaba8fe702 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -665,7 +665,7 @@ subtest 'reconcile_balance' => sub { subtest 'pay() tests' => sub { - plan tests => 7; + plan tests => 8; $schema->storage->txn_begin; @@ -682,13 +682,25 @@ subtest 'pay() tests' => sub { $account->pay( { amount => 5, - interface => 'intranet' + interface => 'intranet', } ); } 'Koha::Exceptions::Account::PaymentTypeRequired', 'Exception thrown for RequirePaymentType:1 + payment_type:undef'; + throws_ok { + $account->pay( + { + amount => 5, + interface => 'intranet', + payment_type => 'FOOBAR' + } + ); + } + 'Koha::Exceptions::Account::InvalidPaymentType', + 'Exception thrown for InvalidPaymentType:1 + payment_type:FOOBAR'; + t::lib::Mocks::mock_preference( 'RequirePaymentType', 0 ); my $context = Test::MockModule->new('C4::Context'); $context->mock( 'userenv', { branch => $library->id } ); @@ -724,7 +736,7 @@ subtest 'pay() tests' => sub { my $result = $account->pay( { amount => 20, - payment_Type => 'CASH', + payment_type => 'CASH', interface => 'intranet' } ); -- 2.39.5