https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37266

Katrin Fischer <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #178579|0                           |1
        is obsolete|                            |

--- Comment #15 from Katrin Fischer <[email protected]> ---
Comment on attachment 178579
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178579
Bug 37266: [24.11.x] patron_lists/delete.pl should have CSRF protection

>From 6c321cdefe352c5369b36c34aafb7fa364cd33d4 Mon Sep 17 00:00:00 2001
>From: Owen Leonard <[email protected]>
>Date: Mon, 24 Feb 2025 14:44:03 +0000
>Subject: [PATCH] Bug 37266: [24.11.x] 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 <[email protected]>
>Signed-off-by: Julian Maurice <[email protected]>
>---
> .../prog/en/modules/circ/circulation.tt       |  1 +
> .../prog/en/modules/members/moremember.tt     |  1 +
> .../prog/en/modules/patron_lists/lists.tt     | 67 +++++++++++--------
> .../modules/patron_lists/patron-lists-tab.tt  |  7 +-
> patron_lists/delete.pl                        | 15 +++--
> 5 files changed, 56 insertions(+), 35 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 a3c9d73f1bb..fc77a84b8db 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>@@ -1167,6 +1167,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 a12b9e3a504..c25adbf4b11 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
>@@ -776,6 +776,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 5e418df153d..38159d1afcb 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
>@@ -45,11 +45,20 @@
> 
>         [% IF ( lists ) %]
> 
>+        <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>
>+
>         <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 class="NoSort"></th>
>                         <th>Name</th>
>                         <th>Patrons in list</th>
>                         <th>Shared</th>
>@@ -62,7 +71,9 @@
>                         [% SET shared_by_other = l.owner.id != 
> logged_in_user.id %]
>                         <tr>
>                             <td>
>-                                <input class="select_patron" type="checkbox" 
>autocomplete="off" data-patron-list-id="[% l.patron_list_id | html %]">
>+                                <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>
>@@ -85,7 +96,9 @@
>                                         <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>
>+                                            <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>
>@@ -115,7 +128,8 @@
>                 </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">
>                 <div class="modal-dialog">
>@@ -147,6 +161,7 @@
> 
> [% MACRO jsinclude BLOCK %]
>     [% Asset.js("js/tools-menu.js") | $raw %]
>+    [% Asset.js("js/form-submit.js") | $raw %]
>     [% INCLUDE 'datatables.inc' %]
> 
>     <script>
>@@ -161,35 +176,31 @@
>                 "columnDefs": [
>                     { "orderable": false, "searchable":  false, "targets": [ 
> 'NoSort' ] }
>                 ],
>-                "pagingType": "full"
>+                "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);
>+                 }
>+             });
> 
>             $(".print_cards").on("click", function(e){
>                 e.preventDefault();
>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 d626d217107..de77c611def 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 %]
>@@ -53,7 +56,9 @@
>                                     <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 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 e7c227ea1f7..45421acf554 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 } );
>+        }
>     }
> }
> 
>-- 
>2.39.5

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to