The cashup action is available via the registers page, which summarises a libraries takings for all registers and is available to users with the 'refund' OR 'cashup' permission. Users without the 'cashup' permission should not be able to take the cashup action.
Created attachment 107081 [details] [review] Bug 26023: Properly secure the cashup action for libraries The libraries summary page for cash management is available for users wit the 'anonymous_refund' permission to allow them to navigate to alternate cash registers and search for the prior transaction to refund. However, currently the cashup option appears, and is not blocked at the server, for all user who may access the page. It should be blocked for those users without the 'cashup' permission.
Test Plan 1/ Setup some cash registers 2/ Login as a user with just the 'refund' permission 3/ Note that you can still access the 'Cashup registers' page from either 'Tools' or the left menu that appears on Point of Sale pages. 4/ Note that you do not see the 'Cashup' actions available 5/ Login as a user with the 'cashup' permission 6/ You should still be able to access the above page 7/ You should not see the cashup actions Bonus points 8/ Without the 'cashup' permission attempt to 'POST' a cashup action (copy a the URL for a cashup action that appears when you were logged in as a user with correction permissions, and paste it into the address bar once you are logged in as a user without the permission 9/ You should be displayed with the registers page with an error message appearing to state that the cashup action was not allowed to take place due to permissions deficiencies.
Created attachment 107082 [details] [review] Bug 26023: Properly secure the cashup action for libraries The libraries summary page for cash management is available for users wit the 'anonymous_refund' permission to allow them to navigate to alternate cash registers and search for the prior transaction to refund. However, currently the cashup option appears, and is not blocked at the server, for all user who may access the page. It should be blocked for those users without the 'cashup' permission.
Created attachment 107767 [details] [review] Bug 26023: Properly secure the cashup action for libraries The libraries summary page for cash management is available for users wit the 'anonymous_refund' permission to allow them to navigate to alternate cash registers and search for the prior transaction to refund. However, currently the cashup option appears, and is not blocked at the server, for all user who may access the page. It should be blocked for those users without the 'cashup' permission.
Created attachment 107768 [details] [review] Bug 26023: Properly secure the cashup and refund actions The cash register summary page for cash management is available for users with the 'anonymous_refund' or 'cashup' permission and the actions available are appropriately displayed. However, the actions are not yet correctly tested for at the server and so a user may force submit to accomplish the action.
Created attachment 108171 [details] [review] Bug 26023: Properly secure the cashup action for libraries The libraries summary page for cash management is available for users wit the 'anonymous_refund' permission to allow them to navigate to alternate cash registers and search for the prior transaction to refund. However, currently the cashup option appears, and is not blocked at the server, for all user who may access the page. It should be blocked for those users without the 'cashup' permission. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 108172 [details] [review] Bug 26023: Properly secure the cashup and refund actions The cash register summary page for cash management is available for users with the 'anonymous_refund' or 'cashup' permission and the actions available are appropriately displayed. However, the actions are not yet correctly tested for at the server and so a user may force submit to accomplish the action. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Martin Renvoize from comment #2) > Test Plan > 1/ Setup some cash registers > 2/ Login as a user with just the 'refund' permission > 3/ Note that you can still access the 'Cashup registers' page from either > 'Tools' or the left menu that appears on Point of Sale pages. > 4/ Note that you do not see the 'Cashup' actions available > 5/ Login as a user with the 'cashup' permission > 6/ You should still be able to access the above page > 7/ You should not see the cashup actions > Bonus points > 8/ Without the 'cashup' permission attempt to 'POST' a cashup action (copy a > the URL for a cashup action that appears when you were logged in as a user > with correction permissions, and paste it into the address bar once you are > logged in as a user without the permission > 9/ You should be displayed with the registers page with an error message > appearing to state that the cashup action was not allowed to take place due > to permissions deficiencies. I have a bit of trouble following the test plan here: 1-4) My user has catalog and refund permissions. With the patch applied, this prevents me from accessing: http://localhost:8081/cgi-bin/koha/pos/registers.pl 5-7) If the user has cashup permission, should they not be able to see the cashup actions? (typo) So I cannot check for the actions not showing.
(In reply to Katrin Fischer from comment #8) > (In reply to Martin Renvoize from comment #2) > > Test Plan > > 1/ Setup some cash registers > > 2/ Login as a user with just the 'refund' permission Oops.. this should have been 'anonymous_refund'.. i.e. the subpermission in cash_management rather than the subpermission of accounts.. my apologies. > > I have a bit of trouble following the test plan here: > > 1-4) > My user has catalog and refund permissions. > With the patch applied, this prevents me from accessing: > http://localhost:8081/cgi-bin/koha/pos/registers.pl See above: However I do wonder if at some point the availability of this page may need/want to fall outside of the cash_management permissions or have it's own permission associated with it (one for another bug however, once I understand the possible use case)
I think we might still want to some refinements, for example having "cash register" on admin and on tools, but not being the same thing is a little confusing. But that's out of scope here and this improves things.
Created attachment 108772 [details] [review] Bug 26023: Properly secure the cashup action for libraries The libraries summary page for cash management is available for users wit the 'anonymous_refund' permission to allow them to navigate to alternate cash registers and search for the prior transaction to refund. However, currently the cashup option appears, and is not blocked at the server, for all user who may access the page. It should be blocked for those users without the 'cashup' permission. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 108773 [details] [review] Bug 26023: Properly secure the cashup and refund actions The cash register summary page for cash management is available for users with the 'anonymous_refund' or 'cashup' permission and the actions available are appropriately displayed. However, the actions are not yet correctly tested for at the server and so a user may force submit to accomplish the action. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Martin, I think we should use "blocking errors" way for that kind of error. At the top of the script we can have output_and_exit( $input, $cookie, $template, 'insufficient_permission' ) if $op eq 'cashup' && not $logged_in_user->has_permission( { cash_management => 'cashup' } ) Not enough to block the push however.
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.04 (comment for Lucas) missing dependencies, not backported to 19.11.x