From 08a9ded6b008d5d90d7951440b7f672284eb3bc7 Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Fri, 24 Jan 2025 17:38:00 +0000
Subject: [PATCH] Bug 37266: patron_lists/delete.pl should have CSRF protection

This patch adds CSRF protection to patron list deletions.

Also changed: The "Delete selected lists" button is now in a floating
toolbar.

To test, apply the patch and go to Tools -> Patron lists.

- If necessary, create a few patron lists.
- Test the two methods for list deletion available on the page:
  - Check one or more checkboxes and then click the "Delete selected
    lists" at the top of the page.
  - Click the "Actions" button for an individual list and choose "Delete
    list."
- Open the checkout page for a patron.
  - Under the "Patron lists" tab, add the patron to a list.
  - Click the "Actions" button for an that list and choose "Delete
    list."
  - When you are taken to the patron lists page the list should have
    been deleted.
- Perform the same test on the patron details page.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
---
 .../prog/en/modules/circ/circulation.tt       |   1 +
 .../prog/en/modules/members/moremember.tt     |   1 +
 .../prog/en/modules/patron_lists/lists.tt     | 207 ++++++++++--------
 .../modules/patron_lists/patron-lists-tab.tt  |  20 +-
 patron_lists/delete.pl                        |  15 +-
 5 files changed, 142 insertions(+), 102 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index 5d05ba5c13..66cb852175 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -1199,6 +1199,7 @@
     [% Asset.js("js/checkouts.js") | $raw %]
     [% Asset.js("js/tables/bookings.js") | $raw %]
     [% Asset.js("js/recalls.js") | $raw %]
+    [% Asset.js("js/form-submit.js") | $raw %]
 [% END %]
 
 [% INCLUDE 'intranet-bottom.inc' %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
index c86144d82c..16c2b33380 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
@@ -786,6 +786,7 @@
     [% INCLUDE 'str/members-menu.inc' %]
     [% Asset.js("js/members-menu.js") | $raw %]
     [% Asset.js("js/recalls.js") | $raw %]
+    [% Asset.js("js/form-submit.js") | $raw %]
     <script>
         const LoadCheckoutsTableDelay = 0;
         const AlwaysLoadCheckoutsTable = [% Koha.Preference('AlwaysLoadCheckoutsTable') | html %];
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
index 45e9af808a..36d5fe8c59 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
@@ -40,84 +40,106 @@
     <h1>Patron lists</h1>
 
     [% IF ( lists ) %]
-        <div class="page-section">
-            <table id="patron-lists-table">
-                <thead>
-                    <tr>
-                        <input type="button" type="submit" class="btn btn-default btn-sm disabled" value="Delete selected lists" id="delete_selected_lists" />
-                        <th>Name</th>
-                        <th>Patrons in list</th>
-                        <th>Shared</th>
-                        <th class="NoSort">&nbsp;</th>
-                    </tr>
-                </thead>
+        <form action="/cgi-bin/koha/patron_lists/delete.pl" method="post" id="patrons_lists_form">
+            [% INCLUDE 'csrf-token.inc' %]
+            <input type="hidden" name="op" value="cud-delete" />
+            <div id="searchheader" class="searchheader noprint sticky">
+                <div class="btn-group">
+                    <button type="submit" class="btn btn-default btn-sm disabled" id="delete_selected_lists"><i class="fa fa-trash" aria-hidden="true"></i> Delete selected lists</button>
+                </div>
+            </div>
 
-                <tbody>
-                    [% FOREACH l IN lists %]
-                        [% SET shared_by_other = l.owner.id != logged_in_user.id %]
+            <div class="page-section">
+                <table id="patron-lists-table">
+                    <thead>
                         <tr>
-                            <td>
-                                <input class="select_patron" type="checkbox" autocomplete="off" data-patron-list-id="[% l.patron_list_id | html %]" />
-                                <a href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id | uri %]">[% l.name | html %]</a>
-                            </td>
-                            <td>[% l.patron_list_patrons_rs.count || 0 | html %]</td>
-                            <td>
-                                [% IF l.shared %]
-                                    [% IF shared_by_other %]
-                                        by <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% l.owner.id | uri %]">[% INCLUDE 'patron-title.inc' patron=l.owner %]</a>
-                                    [% ELSE %]
-                                        by you
-                                    [% END %]
-                                [% END %]
-                            </td>
-                            <td>
-                                <div class="btn-group dropup">
-                                    <a class="btn btn-default btn-xs dropdown-toggle" id="listactions[% l.patron_list_id | html %]" role="button" data-bs-toggle="dropdown" href="#"> Actions </a>
-                                    <ul class="dropdown-menu" role="menu" aria-labelledby="listactions[% l.patron_list_id | html %]">
-                                        <li
-                                            ><a class="dropdown-item" href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id | uri %]"><i class="fa fa-user"></i> Add patrons</a></li
-                                        >
-                                        [% UNLESS shared_by_other %]
-                                            <li
-                                                ><a class="dropdown-item" href="/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=[% l.patron_list_id | uri %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit list</a></li
-                                            >
-                                            <li
-                                                ><a class="delete_patron dropdown-item" href="/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=[% l.patron_list_id | html %]" data-list-name="[% l.name | html %]"
-                                                    ><i class="fa fa-trash-can"></i> Delete list</a
-                                                ></li
-                                            >
-                                        [% END %]
-                                        [% IF ( l.patron_list_patrons_rs.count ) %]
-                                            <li><hr class="dropdown-divider" /></li>
-                                            <li>
-                                                <a class="print_cards dropdown-item" href="/cgi-bin/koha/patroncards/print.pl?patronlist_id=[% l.patron_list_id | html %]" data-patron_list_id="[% l.patron_list_id | html %]"
-                                                    ><i class="fa fa-print"></i> Print patron cards</a
-                                                >
-                                            </li>
-                                            [% IF CAN_user_tools_edit_patrons %]
-                                                <li>
-                                                    <a class="dropdown-item dropdown-item" href="/cgi-bin/koha/tools/modborrowers.pl?patron_list_id=[% l.patron_list_id | uri %]&op=show">
-                                                        <i class="fa-solid fa-pencil" aria-hidden="true"></i> Batch edit patrons
-                                                    </a>
-                                                </li>
-                                            [% END %]
-                                            [% IF CAN_user_tools_delete_anonymize_patrons %]
-                                                <li>
-                                                    <a class="dropdown-item" href="/cgi-bin/koha/tools/cleanborrowers.pl?step=2&patron_list_id=[% l.patron_list_id | uri %]&checkbox=borrower">
-                                                        <i class="fa fa-trash-can"></i> Batch delete patrons
-                                                    </a>
-                                                </li>
-                                            [% END %]
-                                        [% END %]
-                                    </ul>
-                                </div>
-                            </td>
+                            <th class="NoSort"></th>
+                            <th>Name</th>
+                            <th>Patrons in list</th>
+                            <th>Shared</th>
+                            <th class="NoSort">&nbsp;</th>
                         </tr>
-                    [% END %]
-                </tbody>
-            </table>
-        </div>
-        <!-- /.page-section -->
+                    </thead>
+
+                    <tbody>
+                        [% FOREACH l IN lists %]
+                            [% SET shared_by_other = l.owner.id != logged_in_user.id %]
+                            <tr>
+                                <td>
+                                    <input class="select_list" name="patron_lists_ids" value="[% l.patron_list_id | html %]" type="checkbox" autocomplete="off" data-patron-list-id="[% l.patron_list_id | html %]" />
+                                </td>
+                                <td>
+                                    <a href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id | uri %]">[% l.name | html %]</a>
+                                </td>
+                                <td>[% l.patron_list_patrons_rs.count || 0 | html %]</td>
+                                <td>
+                                    [% IF l.shared %]
+                                        [% IF shared_by_other %]
+                                            by <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% l.owner.id | uri %]">[% INCLUDE 'patron-title.inc' patron=l.owner %]</a>
+                                        [% ELSE %]
+                                            by you
+                                        [% END %]
+                                    [% END %]
+                                </td>
+                                <td>
+                                    <div class="btn-group dropup">
+                                        <a class="btn btn-default btn-xs dropdown-toggle" id="listactions[% l.patron_list_id | html %]" role="button" data-bs-toggle="dropdown" href="#"> Actions </a>
+                                        <ul class="dropdown-menu" role="menu" aria-labelledby="listactions[% l.patron_list_id | html %]">
+                                            <li
+                                                ><a class="dropdown-item" href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id | uri %]"><i class="fa fa-user"></i> Add patrons</a></li
+                                            >
+                                            [% UNLESS shared_by_other %]
+                                                <li
+                                                    ><a class="dropdown-item" href="/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=[% l.patron_list_id | uri %]"
+                                                        ><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit list</a
+                                                    ></li
+                                                >
+                                                <li>
+                                                    <a
+                                                        class="dropdown-item submit-form-link"
+                                                        href="#"
+                                                        data-patron_list_id="[% l.patron_list_id | html %]"
+                                                        data-action="delete.pl"
+                                                        data-method="post"
+                                                        data-op="cud-delete"
+                                                        data-confirmation-msg="Are you sure you want to delete this list?"
+                                                        ><i class="fa fa-trash-can"></i> Delete list</a
+                                                    >
+                                                </li>
+                                            [% END %]
+                                            [% IF ( l.patron_list_patrons_rs.count ) %]
+                                                <li><hr class="dropdown-divider" /></li>
+                                                <li>
+                                                    <a class="print_cards dropdown-item" href="/cgi-bin/koha/patroncards/print.pl?patronlist_id=[% l.patron_list_id | html %]" data-patron_list_id="[% l.patron_list_id | html %]"
+                                                        ><i class="fa fa-print"></i> Print patron cards</a
+                                                    >
+                                                </li>
+                                                [% IF CAN_user_tools_edit_patrons %]
+                                                    <li>
+                                                        <a class="dropdown-item dropdown-item" href="/cgi-bin/koha/tools/modborrowers.pl?patron_list_id=[% l.patron_list_id | uri %]&op=show">
+                                                            <i class="fa-solid fa-pencil" aria-hidden="true"></i> Batch edit patrons
+                                                        </a>
+                                                    </li>
+                                                [% END %]
+                                                [% IF CAN_user_tools_delete_anonymize_patrons %]
+                                                    <li>
+                                                        <a class="dropdown-item" href="/cgi-bin/koha/tools/cleanborrowers.pl?step=2&patron_list_id=[% l.patron_list_id | uri %]&checkbox=borrower">
+                                                            <i class="fa fa-trash-can"></i> Batch delete patrons
+                                                        </a>
+                                                    </li>
+                                                [% END %]
+                                            [% END %]
+                                        </ul>
+                                    </div>
+                                </td>
+                            </tr>
+                        [% END %]
+                    </tbody>
+                </table>
+            </div>
+            <!-- /.page-section -->
+        </form>
+        <!-- /#patron_lists_form -->
 
         <!-- Modal to print patron cards -->
         <div class="modal" id="patronExportModal" tabindex="-1" role="dialog" aria-labelledby="patronExportModal_label" aria-hidden="true">
@@ -140,6 +162,7 @@
 
 [% MACRO jsinclude BLOCK %]
     [% Asset.js("js/tools-menu.js") | $raw %]
+    [% Asset.js("js/form-submit.js") | $raw %]
     [% INCLUDE 'datatables.inc' %]
     <script>
         $(document).ready(function() {
@@ -152,32 +175,34 @@
                 autoWidth: false,
                 columnDefs: [{ orderable: false, searchable: false, targets: ["NoSort"] }],
                 pagingType: "full",
+                "sorting": [[ 1, "asc" ]]
             });
+
             $(".delete_patron").on("click", function(){
                 $(".dropdown").removeClass("open");
                 var list = $(this).data("list-name");
                 return confirmDelete( _("Are you sure you want to delete the list %s?").format(list));
             });
 
-            $("#delete_selected_lists").on("click", function() {
-                if (selectedPatronLists.length != 0) {
-                    if (confirm(_("Are you sure you want to delete the selected lists ?"))) {
-                    var delete_lists_url = '/cgi-bin/koha/patron_lists/delete.pl?patron_lists_ids=' + selectedPatronLists.join("&patron_lists_ids=");
-                    window.location.href = delete_lists_url;
-                    }
+            $("#patrons_lists_form").submit(function(){
+                var checkedItems = $("input[name=patron_lists_ids]:checked");
+                if ( checkedItems.size() == 0) {
+                    alert(_("You must select one or more lists to delete"));
+                    return false;
+                }
+                if( confirm(_("Are you sure you want to delete the selected lists?")) ) {
+                    return true;
+                } else {
+                    return false;
                 }
             });
 
-            $(document).on("click", ".select_patron", function() {
-                if($(this).is(':checked')){
-                    $("#delete_selected_lists").attr("class","btn btn-default btn-sm");
-                    selectedPatronLists.push($(this).data("patron-list-id"));
-                }
-                else {
-                    selectedPatronLists = selectedPatronLists.filter(item => item !== $(this).data("patron-list-id"));
-                    if(selectedPatronLists.length === 0){
-                        $("#delete_selected_lists").attr("class","btn btn-default btn-sm disabled");
-                    }
+            $(document).on("click", ".select_list", function() {
+                var checkedItems = $("input[name=patron_lists_ids]:checked");
+                if ( checkedItems.size() == 0 ) {
+                    $("#delete_selected_lists").addClass("disabled").prop("disabled", true);
+                } else {
+                    $("#delete_selected_lists").removeClass("disabled").prop("disabled", false);
                 }
             });
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/patron-lists-tab.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/patron-lists-tab.tt
index 2c748ae84f..950d019a37 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/patron-lists-tab.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/patron-lists-tab.tt
@@ -1,3 +1,6 @@
+[% USE raw %]
+[% USE Koha %]
+[% USE Asset %]
 [% USE KohaDates %]
 
 [% IF no_access_to_patron %]
@@ -57,11 +60,18 @@
                                             <li
                                                 ><a class="dropdown-item" href="/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=[% l.patron_list_id | uri %]"><i class="fa fa-pencil"></i> Edit list</a></li
                                             >
-                                            <li
-                                                ><a class="delete_patron dropdown-item" href="/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=[% l.patron_list_id | html %]" data-list-name="[% l.name | html %]"
-                                                    ><i class="fa fa-trash"></i> Delete list</a
-                                                ></li
-                                            >
+                                            <li>
+                                                <a
+                                                    class="dropdown-item submit-form-link"
+                                                    href="#"
+                                                    data-patron_list_id="[% l.patron_list_id | html %]"
+                                                    data-action="/cgi-bin/koha/patron_lists/delete.pl"
+                                                    data-method="post"
+                                                    data-op="cud-delete"
+                                                    data-confirmation-msg="Are you sure you want to delete this list?"
+                                                    ><i class="fa fa-trash-can"></i> Delete list</a
+                                                >
+                                            </li>
                                         [% END %]
                                         [% IF ( l.patron_list_patrons_rs.count ) %]
                                             <li><hr class="dropdown-divider" /></li>
diff --git a/patron_lists/delete.pl b/patron_lists/delete.pl
index e7c227ea1f..45421acf55 100755
--- a/patron_lists/delete.pl
+++ b/patron_lists/delete.pl
@@ -26,6 +26,7 @@ use C4::Output;
 use Koha::List::Patron qw( DelPatronList );
 
 my $cgi = CGI->new;
+my $op  = $cgi->param('op') // q{};
 
 my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     {
@@ -39,12 +40,14 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 my $id        = $cgi->param('patron_list_id');
 my @lists_ids = $cgi->multi_param('patron_lists_ids');
 
-if ( defined $id && $id ne '' ) {
-    DelPatronList( { patron_list_id => $id } );
-}
-if (@lists_ids) {
-    foreach my $list_id (@lists_ids) {
-        DelPatronList( { patron_list_id => $list_id } );
+if ( $op eq 'cud-delete' ) {
+    if ( defined $id && $id ne '' ) {
+        DelPatronList( { patron_list_id => $id } );
+    }
+    if (@lists_ids) {
+        foreach my $list_id (@lists_ids) {
+            DelPatronList( { patron_list_id => $list_id } );
+        }
     }
 }