From bugzilla-daemon@bugs.koha-community.org Thu Mar  6 12:51:22 2025
From: bugzilla-daemon@bugs.koha-community.org
To: koha-bugs@lists.koha-community.org
Subject:
 [Koha-bugs] [Bug 37266] patron_lists/delete.pl should have CSRF protection
Date: Thu, 06 Mar 2025 11:51:21 +0000
Message-ID: <bug-37266-70-58lFLr40DD@https.bugs.koha-community.org/bugzilla3/>
In-Reply-To: <bug-37266-70@https.bugs.koha-community.org/bugzilla3/>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============5177041388361429402=="

--===============5177041388361429402==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

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

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=3D178579
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/k=
oha-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 %]
>=20
> [% 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 =3D 0;
>         const AlwaysLoadCheckoutsTable =3D [% Koha.Preference('AlwaysLoadCh=
eckoutsTable') | 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 @@
>=20
>         [% IF ( lists ) %]
>=20
>+        <form action=3D"/cgi-bin/koha/patron_lists/delete.pl" method=3D"pos=
t" id=3D"patrons_lists_form">
>+            [% INCLUDE 'csrf-token.inc' %]
>+            <input type=3D"hidden" name=3D"op" value=3D"cud-delete" />
>+            <div id=3D"searchheader" class=3D"searchheader noprint sticky">
>+                <div class=3D"btn-group">
>+                    <button type=3D"submit" class=3D"btn btn-default btn-sm=
 disabled" id=3D"delete_selected_lists"><i class=3D"fa fa-trash" aria-hidden=
=3D"true"></i> Delete selected lists</button>
>+                </div>
>+            </div>
>+
>         <div class=3D"page-section">
>             <table id=3D"patron-lists-table">
>                 <thead>
>                     <tr>
>-                        <input type=3D"button" type=3D"submit" class=3D"btn=
 btn-default btn-sm disabled" value=3D"Delete selected lists" id=3D"delete_se=
lected_lists"/>
>+                        <th class=3D"NoSort"></th>
>                         <th>Name</th>
>                         <th>Patrons in list</th>
>                         <th>Shared</th>
>@@ -62,7 +71,9 @@
>                         [% SET shared_by_other =3D l.owner.id !=3D logged_i=
n_user.id %]
>                         <tr>
>                             <td>
>-                                <input class=3D"select_patron" type=3D"chec=
kbox" autocomplete=3D"off" data-patron-list-id=3D"[% l.patron_list_id | html =
%]">
>+                                <input class=3D"select_list" name=3D"patron=
_lists_ids" value=3D"[% l.patron_list_id | html %]" type=3D"checkbox" autocom=
plete=3D"off" data-patron-list-id=3D"[% l.patron_list_id | html %]" />
>+                            </td>
>+                            <td>
>                                 <a href=3D"/cgi-bin/koha/patron_lists/list.=
pl?patron_list_id=3D[% l.patron_list_id | uri %]">[% l.name | html %]</a>
>                             </td>
>                             <td>[% l.patron_list_patrons_rs.count || 0 | ht=
ml %]</td>
>@@ -85,7 +96,9 @@
>                                         <li><a class=3D"dropdown-item" href=
=3D"/cgi-bin/koha/patron_lists/list.pl?patron_list_id=3D[% l.patron_list_id |=
 uri %]"><i class=3D"fa fa-user"></i> Add patrons</a></li>
>                                         [% UNLESS shared_by_other %]
>                                             <li><a class=3D"dropdown-item" =
href=3D"/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=3D[% l.patron=
_list_id | uri %]"><i class=3D"fa-solid fa-pencil" aria-hidden=3D"true"></i> =
Edit list</a></li>
>-                                            <li><a class=3D"delete_patron d=
ropdown-item" href=3D"/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=3D[=
% l.patron_list_id | html %]" data-list-name=3D"[% l.name | html %]"><i class=
=3D"fa fa-trash-can"></i> Delete list</a></li>
>+                                            <li>
>+                                                <a class=3D"dropdown-item s=
ubmit-form-link" href=3D"#" data-patron_list_id=3D"[% l.patron_list_id | html=
 %]" data-action=3D"delete.pl" data-method=3D"post" data-op=3D"cud-delete" da=
ta-confirmation-msg=3D"Are you sure you want to delete this list?" ><i class=
=3D"fa fa-trash-can"></i> Delete list</a>
>+                                            </li>
>                                         [% END %]
>                                         [% IF ( l.patron_list_patrons_rs.co=
unt ) %]
>                                             <li><hr class=3D"dropdown-divid=
er" /></li>
>@@ -115,7 +128,8 @@
>                 </tbody>
>             </table>
>         </div> <!-- /.page-section -->
>-
>+        </form>
>+        <!-- /#patron_lists_form -->
>             <!-- Modal to print patron cards -->
>             <div class=3D"modal" id=3D"patronExportModal" tabindex=3D"-1" r=
ole=3D"dialog" aria-labelledby=3D"patronExportModal_label" aria-hidden=3D"tru=
e">
>                 <div class=3D"modal-dialog">
>@@ -147,6 +161,7 @@
>=20
> [% MACRO jsinclude BLOCK %]
>     [% Asset.js("js/tools-menu.js") | $raw %]
>+    [% Asset.js("js/form-submit.js") | $raw %]
>     [% INCLUDE 'datatables.inc' %]
>=20
>     <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 =3D $(this).data("list-name");
>-                return confirmDelete( _("Are you sure you want to delete th=
e list %s?").format(list));
>-            });
>=20
>-            $("#delete_selected_lists").on("click", function() {
>-                if (selectedPatronLists.length !=3D 0) {
>-                    if (confirm(_("Are you sure you want to delete the sele=
cted lists ?"))) {
>-                    var delete_lists_url =3D '/cgi-bin/koha/patron_lists/de=
lete.pl?patron_lists_ids=3D' + selectedPatronLists.join("&patron_lists_ids=3D=
");
>-                    window.location.href =3D delete_lists_url;
>-                    }
>+            $("#patrons_lists_form").submit(function(){
>+                var checkedItems =3D $("input[name=3Dpatron_lists_ids]:chec=
ked");
>+                if ( checkedItems.size() =3D=3D 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;
>+                 }
>             });
>=20
>-            $(document).on("click", ".select_patron", function() {
>-                if($(this).is(':checked')){
>-                    $("#delete_selected_lists").attr("class","btn btn-defau=
lt btn-sm");
>-                    selectedPatronLists.push($(this).data("patron-list-id")=
);
>-                }
>-                else {
>-                    selectedPatronLists =3D selectedPatronLists.filter(item=
 =3D> item !=3D=3D $(this).data("patron-list-id"));
>-                    if(selectedPatronLists.length =3D=3D=3D 0){
>-                        $("#delete_selected_lists").attr("class","btn btn-d=
efault btn-sm disabled");
>-                    }
>-                }
>-            });
>+            $(document).on("click", ".select_list", function() {
>+                var checkedItems =3D $("input[name=3Dpatron_lists_ids]:chec=
ked");
>+                if ( checkedItems.size() =3D=3D 0 ) {
>+                    $("#delete_selected_lists").addClass("disabled").prop("=
disabled", true);
>+                } else {
>+                    $("#delete_selected_lists").removeClass("disabled").pro=
p("disabled", false);
>+                 }
>+             });
>=20
>             $(".print_cards").on("click", function(e){
>                 e.preventDefault();
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/patron-lis=
ts-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 %]
>=20
> [% IF no_access_to_patron %]
>@@ -53,7 +56,9 @@
>                                     <li><a class=3D"dropdown-item" href=3D"=
/cgi-bin/koha/patron_lists/list.pl?patron_list_id=3D[% l.patron_list_id | uri=
 %]"><i class=3D"fa fa-user"></i> Add patrons</a></li>
>                                     [% UNLESS shared_by_other %]
>                                         <li><a class=3D"dropdown-item" href=
=3D"/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=3D[% l.patron_lis=
t_id | uri %]"><i class=3D"fa fa-pencil"></i> Edit list</a></li>
>-                                        <li><a class=3D"delete_patron dropd=
own-item" href=3D"/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=3D[% l.=
patron_list_id | html %]" data-list-name=3D"[% l.name | html %]"><i class=3D"=
fa fa-trash"></i> Delete list</a></li>
>+                                        <li>
>+                                            <a class=3D"dropdown-item submi=
t-form-link" href=3D"#" data-patron_list_id=3D"[% l.patron_list_id | html %]"=
 data-action=3D"/cgi-bin/koha/patron_lists/delete.pl" data-method=3D"post" da=
ta-op=3D"cud-delete" data-confirmation-msg=3D"Are you sure you want to delete=
 this list?" ><i class=3D"fa fa-trash-can"></i> Delete list</a>
>+                                        </li>
>                                     [% END %]
>                                     [% IF ( l.patron_list_patrons_rs.count =
) %]
>                                         <li><hr class=3D"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 );
>=20
> my $cgi =3D CGI->new;
>+my $op  =3D $cgi->param('op') // q{};
>=20
> my ( $template, $loggedinuser, $cookie ) =3D get_template_and_user(
>     {
>@@ -39,12 +40,14 @@ my ( $template, $loggedinuser, $cookie ) =3D get_templat=
e_and_user(
> my $id        =3D $cgi->param('patron_list_id');
> my @lists_ids =3D $cgi->multi_param('patron_lists_ids');
>=20
>-if ( defined $id && $id ne '' ) {
>-    DelPatronList( { patron_list_id =3D> $id } );
>-}
>-if (@lists_ids) {
>-    foreach my $list_id (@lists_ids) {
>-        DelPatronList( { patron_list_id =3D> $list_id } );
>+if ( $op eq 'cud-delete' ) {
>+    if ( defined $id && $id ne '' ) {
>+        DelPatronList( { patron_list_id =3D> $id } );
>+    }
>+    if (@lists_ids) {
>+        foreach my $list_id (@lists_ids) {
>+            DelPatronList( { patron_list_id =3D> $list_id } );
>+        }
>     }
> }
>=20
>--=20
>2.39.5

--=20
You are receiving this mail because:
You are watching all bug changes.

--===============5177041388361429402==--