https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37266 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178579|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 <oleonard@myacpl.org> 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 <phil@chetcolibrary.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> --- .../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.