Bugzilla – Attachment 179008 Details for
Bug 37266
patron_lists/delete.pl should have CSRF protection
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37266: patron_lists/delete.pl should have CSRF protection
Bug-37266-patronlistsdeletepl-should-have-CSRF-pro.patch (text/plain), 21.33 KB, created by
Owen Leonard
on 2025-03-06 12:17:11 UTC
(
hide
)
Description:
Bug 37266: patron_lists/delete.pl should have CSRF protection
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2025-03-06 12:17:11 UTC
Size:
21.33 KB
patch
obsolete
>From 9952eee649a616557fc77ff83383e31a91c47746 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> >--- > .../prog/en/modules/circ/circulation.tt | 1 + > .../prog/en/modules/members/moremember.tt | 1 + > .../prog/en/modules/patron_lists/lists.tt | 191 ++++++++++-------- > .../modules/patron_lists/patron-lists-tab.tt | 20 +- > patron_lists/delete.pl | 15 +- > 5 files changed, 134 insertions(+), 94 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 5d05ba5c13e..66cb852175d 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 c86144d82c1..16c2b333803 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 45e9af808a2..36d5fe8c59d 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"> </th> >- </tr> >- </thead> >- >- <tbody> >- [% FOREACH l IN lists %] >- [% SET shared_by_other = l.owner.id != logged_in_user.id %] >+ <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> >- <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 >+ <th class="NoSort"></th> >+ <th>Name</th> >+ <th>Patrons in list</th> >+ <th>Shared</th> >+ <th class="NoSort"> </th> >+ </tr> >+ </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 %] >- [% 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 >- > >+ </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="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 >+ ><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 > > >- [% 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 >+ [% 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> >- [% 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> >+ <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 CAN_user_tools_delete_anonymize_patrons %] >+ [% IF ( l.patron_list_patrons_rs.count ) %] >+ <li><hr class="dropdown-divider" /></li> > <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> >+ <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 %] >- [% END %] >- </ul> >- </div> >- </td> >- </tr> >- [% END %] >- </tbody> >- </table> >- </div> >- <!-- /.page-section --> >+ </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 2c748ae84f5..950d019a371 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 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 cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37266
:
177131
|
177132
|
177352
|
177887
|
178579
|
178676
| 179008