Bug 36076 - paycollect.tt is missing permission checks for manual credit and invoice
Summary: paycollect.tt is missing permission checks for manual credit and invoice
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Victor Grousset/tuxayo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-13 09:49 UTC by Fridolin Somers
Modified: 2024-03-19 15:35 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.04,23.05.10


Attachments
Bug 36076: paycollect.tt add permission checks for manual credit and invoice (2.59 KB, patch)
2024-02-13 09:54 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 36076: paycollect.tt add permission checks for manual credit and invoice (2.64 KB, patch)
2024-02-16 23:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 36076: paycollect.tt add permission checks for manual credit and invoice (2.75 KB, patch)
2024-03-03 22:30 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2024-02-13 09:49:12 UTC
In members/pay.tt one can see permission checks for manual credit and invoice :
CAN_user_updatecharges_manual_invoice
CAN_user_updatecharges_manual_credit

This is missing from members/paycollect.tt.
HTML is also missing classes manualcredit and manualinvoice.
Comment 1 Fridolin Somers 2024-02-13 09:54:17 UTC
Created attachment 162088 [details] [review]
Bug 36076: paycollect.tt add permission checks for manual credit and invoice

In members/pay.tt one can see permission checks for manual credit and invoice :
CAN_user_updatecharges_manual_invoice
CAN_user_updatecharges_manual_credit

This is missing from members/paycollect.tt.
HTML is also missing classes manualcredit and manualinvoice.

Test plan :
1) Create a user with permissions to manage accounting but without
   manual_invoice and manual_credit
2) Go to a patron account with an invoice
3) Click on "Make a payment", you dont see tabs manual credit/invoice
4) Click on "Pay" in "Actions" column
=> Without patch you see tabs manual credit/invoice
=> With patch you do not see them
Comment 2 Fridolin Somers 2024-02-13 09:54:48 UTC
I set major because it is a permission leak
Comment 3 David Nind 2024-02-16 23:13:34 UTC
Created attachment 162252 [details] [review]
Bug 36076: paycollect.tt add permission checks for manual credit and invoice

In members/pay.tt one can see permission checks for manual credit and invoice :
CAN_user_updatecharges_manual_invoice
CAN_user_updatecharges_manual_credit

This is missing from members/paycollect.tt.
HTML is also missing classes manualcredit and manualinvoice.

Test plan :
1) Create a user with permissions to manage accounting but without
   manual_invoice and manual_credit
2) Go to a patron account with an invoice
3) Click on "Make a payment", you dont see tabs manual credit/invoice
4) Click on "Pay" in "Actions" column
=> Without patch you see tabs manual credit/invoice
=> With patch you do not see them

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Victor Grousset/tuxayo 2024-03-03 22:30:19 UTC
Created attachment 162707 [details] [review]
Bug 36076: paycollect.tt add permission checks for manual credit and invoice

In members/pay.tt one can see permission checks for manual credit and invoice :
CAN_user_updatecharges_manual_invoice
CAN_user_updatecharges_manual_credit

This is missing from members/paycollect.tt.
HTML is also missing classes manualcredit and manualinvoice.

Test plan :
1) Create a user with permissions to manage accounting
   (remaining_permissions under updatecharges) but without
   manual_invoice and manual_credit
2) Go to a patron account with an invoice
3) Click on "Make a payment", you dont see tabs manual credit/invoice
4) Click on "Pay" in "Actions" column
=> Without patch you see tabs manual credit/invoice
=> With patch you do not see them

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2024-03-03 22:31:16 UTC
Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 6 Victor Grousset/tuxayo 2024-03-03 22:37:40 UTC
(In reply to Fridolin Somers from comment #2)
> I set major because it is a permission leak

It's just displaying links which don't work because the server checks the permission before sending the page.
And even if it did work, the post request for invoices and credit are also protected server side. (checked by loading the form, removing the permission and trying to make a manual invoice/credit)
Comment 7 Katrin Fischer 2024-03-07 14:04:23 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 8 Fridolin Somers 2024-03-11 09:43:45 UTC
Pushed to 23.11.x for 23.11.04
Comment 9 Lucas Gass 2024-03-19 15:35:59 UTC
Backported to 23.05.x for upcoming 23.05.10.