From 5a333e3cfa7dd55d4a1fa0d8a4e2cc40154029c4 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Thu, 15 Feb 2024 15:25:17 +0000
Subject: [PATCH] Bug 34478: Use op and cud in pay -> paycollect redirect

The pay to paycollect post/redirect flow here doesn't actually
consistute a state change, however it's much simpler to add the csrf
token check flow here than to refactor the code to a get (url's quickly
grow too large for a GET) or rework it in other ways.  I opted to do
this for now and work on a refactor at a future date.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 .../prog/en/modules/members/pay.tt            | 16 ++++++------
 members/pay.pl                                | 25 ++++++++-----------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
index 1cfdc15407..8fc5ecf22b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
@@ -109,9 +109,9 @@
     </td>
     <td class="actions">
     [% IF ( line.amountoutstanding > 0 ) %]
-        <button type="submit" class="btn btn-default btn-xs" name="pay_indiv_[% line.accountlines_id | html %]" value="Pay">Pay</button>
+        <button type="submit" class="btn btn-default btn-xs" name="op" value="cud-pay_indiv_[% line.accountlines_id | html %]">Pay</button>
         [% IF CAN_user_updatecharges_writeoff %]
-            <button type="submit" class="btn btn-default btn-xs" name="wo_indiv_[% line.accountlines_id | html %]" value="Write off">Write off</button>
+            <button type="submit" class="btn btn-default btn-xs" name="op" value="cud-wo_indiv_[% line.accountlines_id | html %]">Write off</button>
         [% END %]
     [% END %]
     <input type="hidden" name="itemnumber[% line.accountlines_id | html %]" value="[% line.itemnumber | html %]" />
@@ -185,7 +185,7 @@
     [% IF outstanding_credits.total_outstanding < 0 %]
         <tr>
             <td class="total" colspan="12">Outstanding credits could be applied: </td>
-            <td class="credit" style="text-align: right;"><button type="submit" id="apply_credits" name="apply_credits" value="apply_credits" class="btn btn-default btn-sm">Apply <strong class="credit">[% outstanding_credits.total_outstanding | $Price %]</strong></button></td>
+            <td class="credit" style="text-align: right;"><button type="submit" id="apply_credits" name="op" value="cud-apply_credits" class="btn btn-default btn-sm">Apply <strong class="credit">[% outstanding_credits.total_outstanding | $Price %]</strong></button></td>
         </tr>
         <tr>
             <td class="total" colspan="12">Total due if credit applied:</td>
@@ -196,10 +196,12 @@
 
 </table>
 <fieldset class="action">
-<input type="submit" id="paycollect" name="paycollect"  value="Pay amount" class="submit" />
-<input type="submit" id="payselected" name="payselected"  value="Pay selected" class="submit" />
-[% IF CAN_user_updatecharges_writeoff %]<input type="submit" name="woall"  id="woall" value="Write off all" class="submit" />
-<input type="submit" id="writeoff-selected" name="writeoff_selected"  value="Write off selected" class="submit" />[% END %]
+<button type="submit" id="paycollect" name="op" value="cud-paycollect" class="submit btn btn-primary">Pay amount</button>
+<button type="submit" id="payselected" name="op" value="cud-payselected"  class="submit btn btn-primary">Pay selected</button>
+[% IF CAN_user_updatecharges_writeoff %]
+<button type="submit" id="woall" name="op" value="cud-woall" value="Write off all" class="submit btn btn-primary">Write off all</button>
+<button type="submit" id="writeoff-selected" name="op" value="cud-writeoff_selected" class="submit btn btn-primary">Write off selected</button>
+[% END %]
 <a class="cancel" href="/cgi-bin/koha/members/boraccount.pl?borrowernumber=[% patron.borrowernumber | html %]">Cancel</a>
 </fieldset>
 </form>
diff --git a/members/pay.pl b/members/pay.pl
index 19ab74bdc3..e9c9378003 100755
--- a/members/pay.pl
+++ b/members/pay.pl
@@ -76,29 +76,26 @@ $user ||= q{};
 
 our $branch = C4::Context->userenv->{'branch'};
 
-if ( $input->param('paycollect') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+my $op = $input->param('op') // q{};
+
+if ( $op eq 'cud-paycollect' ) {
     print $input->redirect(
         "/cgi-bin/koha/members/paycollect.pl?borrowernumber=$borrowernumber&change_given=$change_given");
 }
-elsif ( $input->param('payselected') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+elsif ( $op eq 'cud-payselected' ) {
     payselected({ params => \@names });
 }
-elsif ( $input->param('writeoff_selected') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+elsif ( $op eq 'cud-writeoff_selected' ) {
     payselected({ params => \@names, type => 'WRITEOFF' });
 }
-elsif ( $input->param('woall') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+elsif ( $op eq 'cud-woall' ) {
     writeoff_all(@names);
 }
-elsif ( $input->param('apply_credits') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+elsif ( $op eq 'cud-apply_credits' ) {
     apply_credits({ patron => $patron, cgi => $input });
 }
 elsif ( $input->param('confirm_writeoff') ) {
-    output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+    #FIXME: This block really belongs in paycollect
     my $item_id         = $input->param('itemnumber');
     my $accountlines_id = $input->param('accountlines_id');
     my $amount          = $input->param('amountwrittenoff');
@@ -134,12 +131,10 @@ elsif ( $input->param('confirm_writeoff') ) {
 }
 
 for (@names) {
-    if (/^pay_indiv_(\d+)$/) {
-        output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+    if ($op =~ /^cud-pay_indiv_(\d+)$/) {
         my $line_no = $1;
         redirect_to_paycollect( 'pay_individual', $line_no );
-    } elsif (/^wo_indiv_(\d+)$/) {
-        output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
+    } elsif ($op =~ /^cud-wo_indiv_(\d+)$/) {
         my $line_no = $1;
         redirect_to_paycollect( 'writeoff_individual', $line_no );
     }