Bug 37206: Removing an item from a label batch should be a CSRF-protected POST operation

This patch updates the label batch edit template so that removing a
single or multiple items from a batch is a CSRF-protected POST
operation.

The patch also removes the existing "if ($op eq 'cud-delete') {" section
of label-edit-batch.pl because it was unused.

To test, apply the patch and go to Cataloging -> Labels.

- Create a label batch and add multiple items to it.
- From the list of label batches, click "Edit" on the batch you created.
- Click the "Delete" button for one of the items in the batch.
  - If you confirm, the item should be deleted.
- In the "Select" column, check multiple checkboxes.
- Click the "Remove selected items" button in the toolbar.
  - Verify that confirming this operation results in the items being
    deleted from the batch.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Owen Leonard 2024-06-28 13:06:43 +00:00 committed by Katrin Fischer
parent edc5caf976
commit a964d98dc8
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
2 changed files with 19 additions and 13 deletions

View File

@ -166,8 +166,9 @@
<tr>
[% FOREACH text_field IN table_loo.text_fields %]
[% IF ( text_field.select_field ) %]
<td>
<a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/labels/label-edit-batch.pl?op=remove&amp;batch_id=[% batch_id |url %]&amp;label_id=[% text_field.field_value |url %]"><i class="fa fa-trash-can"></i> Delete</a>
<td class="actions">
<a class="btn btn-default btn-xs submit-form-link" id="deletebatch" href="#" data-op="cud-delete" data-batch_id="[% batch_id | html %]" data-label_id="[% text_field.field_value | html %]" data-method="post" data-action="/cgi-bin/koha/labels/label-edit-batch.pl" data-confirmation-msg="Are you sure you want to delete this?"><i class="fa fa-trash-can"></i> Delete</a>
<a class="btn btn-default btn-xs export" href="#" data-batch-id="[% batch_id | html %]" data-label-id="[% text_field.field_value | html %]"><i class="fa-solid fa-share-from-square"></i> Export</a>
</td>
<td><input type="checkbox" name="action" value="[% text_field.field_value | html %]"></td>
@ -205,6 +206,11 @@
</div> <!-- /.col-sm-2.col-sm-pull-10 -->
</div> <!-- /.row -->
<form action="/cgi-bin/koha/labels/label-edit-batch.pl" id="batch_remove_form" method="post">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="op" value="cud-delete" />
<input type="hidden" name="batch_id" value="[% batch_id | html %]" />
</form>
[% MACRO jsinclude BLOCK %]
[% INCLUDE 'greybox.inc' %]
[% INCLUDE 'datatables.inc' %]
@ -222,17 +228,18 @@
return true; // ok
}
}
function Remove() {
const batch_remove_form = $("#batch_remove_form");
items = new Array;
item_num = new Array;
if(document.items.action.length > 0) {
for (var i=0; i < document.items.action.length; i++) {
if (document.items.action[i].checked) {
items.push("label_id=" + document.items.action[i].value);
item_num.push(i+1);
items.push( document.items.action[i].value );
item_num.push( i + 1 );
}
}
getstr = items.join("&");
item_msg = item_num.join(", ");
var msg = _("Are you sure you want to remove label number(s): %s from this batch?").format(item_msg);
} else if (document.items.action.checked) {
@ -242,13 +249,16 @@
alert(_("Please select at least one label to delete."));
return; // no item selected
}
var answer = confirm(msg);
if (answer) {
window.location = "/cgi-bin/koha/labels/label-edit-batch.pl?op=remove&amp;batch_id=[% batch_id | html %]&amp;" + getstr;
if ( confirm( msg ) ) {
items.forEach(( label_id ) => {
batch_remove_form.append('<input type="hidden" name="label_id" value="' + label_id + '" />');
});
batch_remove_form.submit();
} else {
return; // abort delete
}
}
function Add() {
var number_list = document.getElementById("number_list");
if (number_list.value == '') {

View File

@ -67,7 +67,7 @@ $number_list = $cgi->param('number_list') if $cgi->param('number_list');
my $branch_code = C4::Context->userenv->{'branch'};
if ($op eq 'remove') {
if ($op eq 'cud-delete') {
$batch = C4::Labels::Batch->retrieve(batch_id => $batch_id);
foreach my $label_id (@label_ids) {
$err = $batch->remove_item($label_id);
@ -77,10 +77,6 @@ if ($op eq 'remove') {
# print $cgi->redirect("label-edit-batch.pl?op=edit&batch_id=$batch_id");
# exit;
}
elsif ($op eq 'cud-delete') {
$err = C4::Labels::Batch::delete(batch_id => $batch_id, branch_code => $branch_code);
$errtype = 'BATCH_NOT_DELETED' if $err;
}
elsif ($op eq 'cud-add') {
if ($number_list) {
my @numbers_list = split /\n/, $number_list; # Entries are effectively passed in as a <cr> separated list