Bug 40625: Fix duplicate cashup records via POST/REDIRECT/GET pattern

The cashup functionality on both /pos/register.pl and /pos/registers.pl
pages was using GET requests via anchor tags, which could cause
duplicate cashup records when users refreshed the page after performing
a cashup.

This patch implements proper POST/REDIRECT/GET pattern with CSRF
protection:

1. Convert anchor tag buttons to proper HTML forms with submit buttons
2. Add CSRF token protection to all cashup operations
3. Change operation from 'cashup' to 'cud-cashup' for CSRF compliance
4. Implement redirects after successful cashup operations to prevent
   resubmission
5. Update JavaScript to populate form fields instead of href attributes

This ensures cashup operations follow HTTP semantics properly (POST for
state changes) and prevents duplicate submissions while maintaining the
existing user experience.

Test plan:
1. Apply patch and restart services
2. Go to Point of Sale > Cash registers
3. Click "Record cashup" on any register - should open modal
4. Click "Confirm" - should perform cashup and reload page
5. Refresh the page - should NOT create duplicate cashup record
6. Go to Point of Sale > Register for individual register
7. Click "Record cashup" - should open modal
8. Click "Confirm" - should perform cashup and reload page
9. Refresh the page - should NOT create duplicate cashup record
10. On registers page, try "Cashup all" button - should work correctly
11. Verify all existing functionality (modals, summaries) still works
12. Check browser developer tools - all requests should be POST for
    cashup operations

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2025-08-08 15:33:42 +00:00 committed by Lucas Gass
parent a1fc548519
commit 26f1f91a13
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
4 changed files with 80 additions and 56 deletions

View File

@ -366,23 +366,30 @@
<!-- Confirm cashup modal -->
<div class="modal" id="confirmCashupModal" tabindex="-1" role="dialog" aria-labelledby="confirmCashupLabel">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupLabel">Confirm cashup of <em>[% register.description | html %]</em></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
<form id="cashup_form" method="post" enctype="multipart/form-data">
[% INCLUDE 'csrf-token.inc' %]
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupLabel">Confirm cashup of <em>[% register.description | html %]</em></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
Please confirm that you have removed [% accountlines.total( payment_type => [ 'CASH', 'SIP00' ]) * -1 | $Price %] from the cash register and left a float of [% register.starting_float | $Price %].
</div>
<!-- /.modal-body -->
<div class="modal-footer">
<input type="hidden" name="registerid" value="[% register.id | html %]" />
<input type="hidden" name="op" value="cud-cashup" />
<button type="submit" class="btn btn-primary" id="pos_cashup_confirm">Confirm</button>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
</div>
<div class="modal-body"> Please confirm that you have removed [% accountlines.total( payment_type => [ 'CASH', 'SIP00' ]) * -1 | $Price %] from the cash register and left a float of [% register.starting_float | $Price %]. </div>
<!-- /.modal-body -->
<div class="modal-footer">
<a href="/cgi-bin/koha/pos/register.pl?op=cashup" class="btn btn-primary" id="pos_cashup_confirm">Confirm</a>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
<!-- /.modal-content -->
</div>
<!-- /.modal-content -->
</div>
<!-- /.modal-dialog -->
<!-- /.modal-dialog -->
</form>
</div>
<!-- /#confirmCashupModal -->

View File

@ -149,53 +149,62 @@
<!-- Confirm cashup modal -->
<div class="modal" id="confirmCashupModal" tabindex="-1" role="dialog" aria-labelledby="confirmCashupLabel">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupLabel"
>Confirm cashup of <em><span id="registerc"></span></em
></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
<form id="cashup_individual_form" method="post" enctype="multipart/form-data">
[% INCLUDE 'csrf-token.inc' %]
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupLabel"
>Confirm cashup of <em><span id="registerc"></span></em
></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body"> Please confirm that you have removed <span id="cashc"></span> from the cash register and left a float of <span id="floatc"></span>. </div>
<!-- /.modal-body -->
<div class="modal-footer">
<input type="hidden" name="registerid" id="cashup_registerid" value="" />
<input type="hidden" name="op" value="cud-cashup" />
<button type="submit" class="btn btn-primary" id="cashup_confirm">Confirm</button>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
</div>
<div class="modal-body"> Please confirm that you have removed <span id="cashc"></span> from the cash register and left a float of <span id="floatc"></span>. </div>
<!-- /.modal-body -->
<div class="modal-footer">
<a id="cashup_confirm" href="" class="btn btn-primary">Confirm</a>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
<!-- /.modal-content -->
</div>
<!-- /.modal-content -->
</div>
<!-- /.modal-dialog -->
<!-- /.modal-dialog -->
</form>
</div>
<!-- /#confirmCashupModal -->
<!-- Confirm cashupall modal -->
<div class="modal" id="confirmCashupAllModal" tabindex="-1" role="dialog" aria-labelledby="confirmCashupAllLabel">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupAllLabel">Confirm cashup of <em>all</em> cash registers</h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
<form id="cashup_all_form" method="post" enctype="multipart/form-data">
[% INCLUDE 'csrf-token.inc' %]
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title" id="confirmCashupAllLabel">Confirm cashup of <em>all</em> cash registers</h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
<p>Please confirm that you have received [% bankable | $Price %] to cashup.</p>
<ul>
<li>Total income (cash): [% ctotal | $Price %] ([% cctotal | $Price %])</li>
<li>Total outgoing (cash): [% dtotal | $Price %] ([% cdtotal | $Price %])</li>
</ul>
</div>
<!-- /.modal-body -->
<div class="modal-footer">
<input type="hidden" name="op" value="cud-cashup" />
<button type="submit" class="btn btn-primary" id="cashup_all_confirm">Confirm</button>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
</div>
<div class="modal-body">
<p>Please confirm that you have received [% bankable | $Price %] to cashup.</p>
<ul>
<li>Total income (cash): [% ctotal | $Price %] ([% cctotal | $Price %])</li>
<li>Total outgoing (cash): [% dtotal | $Price %] ([% cdtotal | $Price %])</li>
</ul>
</div>
<!-- /.modal-body -->
<div class="modal-footer">
<a id="cashup_all_confirm" href="/cgi-bin/koha/pos/registers.pl?op=cashup" class="btn btn-primary">Confirm</a>
<button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button>
</div>
<!-- /.modal-footer -->
<!-- /.modal-content -->
</div>
<!-- /.modal-content -->
</div>
<!-- /.modal-dialog -->
<!-- /.modal-dialog -->
</form>
</div>
<!-- /#confirmCashupAllModal -->
@ -229,7 +238,7 @@
var rfloat = button.data('float');
$('#floatc').text(rfloat);
var rid = button.data('registerid');
$('#cashup_confirm').attr("href", '/cgi-bin/koha/pos/registers.pl?op=cashup&registerid='+rid);
$('#cashup_registerid').val(rid);
});
});
</script>

View File

@ -102,7 +102,7 @@ if ( !$registers->count ) {
$template->param( trange_t => $end, );
my $op = $input->param('op') // '';
if ( $op eq 'cashup' ) {
if ( $op eq 'cud-cashup' ) {
if ( $logged_in_user->has_permission( { cash_management => 'cashup' } ) ) {
$cash_register->add_cashup(
{
@ -110,6 +110,10 @@ if ( !$registers->count ) {
amount => $cash_register->outstanding_accountlines->total
}
);
# Redirect to prevent duplicate submissions (POST/REDIRECT/GET pattern)
print $input->redirect( "/cgi-bin/koha/pos/register.pl?registerid=" . $registerid );
exit;
} else {
$template->param( error_cashup_permission => 1 );
}

View File

@ -54,7 +54,7 @@ if ( !$registers->count ) {
}
my $op = $input->param('op') // '';
if ( $op eq 'cashup' ) {
if ( $op eq 'cud-cashup' ) {
if ( $logged_in_user->has_permission( { cash_management => 'cashup' } ) ) {
my $registerid = $input->param('registerid');
if ($registerid) {
@ -75,6 +75,10 @@ if ( $op eq 'cashup' ) {
);
}
}
# Redirect to prevent duplicate submissions (POST/REDIRECT/GET pattern)
print $input->redirect("/cgi-bin/koha/pos/registers.pl");
exit;
} else {
$template->param( error_cashup_permission => 1 );
}