Bugzilla – Attachment 185404 Details for
Bug 40625
Cashup re-submissions on page reload
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40625: Fix duplicate cashup records via POST/REDIRECT/GET pattern
Bug-40625-Fix-duplicate-cashup-records-via-POSTRED.patch (text/plain), 12.61 KB, created by
Owen Leonard
on 2025-08-14 16:31:07 UTC
(
hide
)
Description:
Bug 40625: Fix duplicate cashup records via POST/REDIRECT/GET pattern
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2025-08-14 16:31:07 UTC
Size:
12.61 KB
patch
obsolete
>From 80b303aa5f4c48063815ce7f9531a1f1f90c2d31 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Fri, 8 Aug 2025 15:33:42 +0000 >Subject: [PATCH] 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> >--- > .../prog/en/modules/pos/register.tt | 37 ++++---- > .../prog/en/modules/pos/registers.tt | 87 ++++++++++--------- > pos/register.pl | 6 +- > pos/registers.pl | 6 +- > 4 files changed, 80 insertions(+), 56 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >index 7e70e2c62d7..e01152f7515 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >@@ -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> >- </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> >+ <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> >- <!-- /.modal-footer --> >+ <!-- /.modal-content --> > </div> >- <!-- /.modal-content --> >- </div> >- <!-- /.modal-dialog --> >+ <!-- /.modal-dialog --> >+ </form> > </div> > <!-- /#confirmCashupModal --> > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >index d345a60306b..c79ad6c20fd 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >@@ -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> >- </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> >+ <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> >- <!-- /.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®isterid='+rid); >+ $('#cashup_registerid').val(rid); > }); > }); > </script> >diff --git a/pos/register.pl b/pos/register.pl >index 2043ded9ce3..a215f36267c 100755 >--- a/pos/register.pl >+++ b/pos/register.pl >@@ -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 ); > } >diff --git a/pos/registers.pl b/pos/registers.pl >index 65accdf10c3..67d72fd6635 100755 >--- a/pos/registers.pl >+++ b/pos/registers.pl >@@ -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 ); > } >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40625
:
185284
| 185404