We have notices, with RequirePaymentType on, that sometimes payments still go through without a payment type. RequirePaymentType adds the required attribute to the template but this can be circumnavigated by using your browser's dev tools. We should add a check in the script to make sure there is a payment type and if not, prevent the payment.
Maybe also in the module? Thinking of the API.
(In reply to Katrin Fischer from comment #1) > Maybe also in the module? Thinking of the API. Yes, good point.
Created attachment 149617 [details] [review] Bug 33176: Handle RequirePaymentType Test plan: 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. Turn RequirePaymentType off. Try a payment with a payment type, you shoud be successful. 9. Make sure tests will pass: prove -v t/db_dependent/Koha/Account.t
Created attachment 149619 [details] [review] Bug 33176: Enforce RequirePaymentType with API
Hi Lucas. I get an error trace after applying the patch and repeating the steps. The payment is recorded. The start of the error trace: Exception 'Koha::Exceptions::Account::PaymentTypeRequired' thrown 'Account transaction requires a payment type' at /usr/share/perl5/Exception/Class/Base.pm line 88 in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm line 88 85: 86: $proto->rethrow if ref $proto; 87: 88: die $proto->new(@_); 89: } 90: 91: sub rethrow { Show function arguments in Koha::Account::pay at /kohadevbox/koha/Koha/Account.pm line 105 .... David Testing notes (using KTD): 1. Editing the page HTML using Firefox: - Right click on the payment type label - Select 'Inspect' - In the web development tools window that opens, select <select name="payment_type" id="payment_type" class="required" required="required">, right click, and then select 'Edit HTML' - Remove: class="required" required="required" - Click on the Submit button in the page
(In reply to David Nind from comment #5) > Hi Lucas. > > I get an error trace after applying the patch and repeating the steps. The > payment is recorded. The start of the error trace: > > Exception 'Koha::Exceptions::Account::PaymentTypeRequired' thrown 'Account > transaction requires a payment type' > at /usr/share/perl5/Exception/Class/Base.pm line 88 > > in Exception::Class::Base::throw at > /usr/share/perl5/Exception/Class/Base.pm line 88 > > 85: > 86: $proto->rethrow if ref $proto; > 87: > 88: die $proto->new(@_); > 89: } > 90: > 91: sub rethrow { > > Show function arguments > in Koha::Account::pay at /kohadevbox/koha/Koha/Account.pm line 105 > .... > > David > > Testing notes (using KTD): > > 1. Editing the page HTML using Firefox: > - Right click on the payment type label > - Select 'Inspect' > - In the web development tools window that opens, select <select > name="payment_type" id="payment_type" class="required" required="required">, > right click, and then select 'Edit HTML' > - Remove: class="required" required="required" > - Click on the Submit button in the page David, if this is happening after you remove class="required" required="required" it is the desired behavior.
Created attachment 150254 [details] [review] Bug 33176: Handle RequirePaymentType Test plan: 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. Turn RequirePaymentType off. Try a payment with a payment type, you shoud be successful. 9. Make sure tests will pass: prove -v t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 150255 [details] [review] Bug 33176: Enforce RequirePaymentType with API Signed-off-by: David Nind <david@davidnind.com>
Updated testing notes (using KTD): 1. For step 4, editing the page HTML using Firefox: - Right click on the payment type label - Select 'Inspect' - In the web development tools window that opens, select <select name="payment_type" id="payment_type" class="required" required="required">, right click, and then select 'Edit HTML' - Remove: class="required" required="required" - Click on the Submit button in the page - Note: If you just remove 'required="required"', a standard form error message is displayed saying this is required 2. For step 7, when editing the HTML just remove: required="required" - If you remove 'class="required" required="required"', you will get an error trace and the payment will be recorded (as expected - see comment #6)
If you want to prevent manual edition of the DOM to get empty value, then you should also prevent bad value. It's possible to get payment_type=FOO.
(In reply to Jonathan Druart from comment #10) > If you want to prevent manual edition of the DOM to get empty value, then > you should also prevent bad value. It's possible to get payment_type=FOO. Good point.
Created attachment 152131 [details] [review] 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
Created attachment 152203 [details] [review] Bug 33176: Handle RequirePaymentType Test plan: 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. Turn RequirePaymentType off. Try a payment with a payment type, you shoud be successful. 9. Make sure tests will pass: prove -v t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com>
Created attachment 152204 [details] [review] Bug 33176: Enforce RequirePaymentType with API Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com>
Created attachment 152205 [details] [review] 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 <samalau@gmail.com>
Created attachment 152271 [details] [review] Bug 33176: Handle RequirePaymentType Test plan: 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. Turn RequirePaymentType off. Try a payment with a payment type, you shoud be successful. 9. Make sure tests will pass: prove -v t/db_dependent/Koha/Account.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 152272 [details] [review] Bug 33176: Enforce RequirePaymentType with API Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 152273 [details] [review] 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 <samalau@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 23.11. Nice work everyone, thanks!
Thanks for all the hard work! Pushed to 23.05.x for the next release
Nice work everyone! Pushed to 22.11.x for next release