Attachment 168288 Details for Bug 37206 – Bug 37206: Removing an item from a label batch should be a CSRF-protected POST operation

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

Bug-37206-Removing-an-item-from-a-label-batch-shou.patch (text/plain), 6.54 KB, created by David Nind on 2024-06-28 18:58:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2024-06-28 18:58:35 UTC

Size: 6.54 KB

patch

obsolete

>From 7d3c608ab18b40f164fc1b324cafc59d7fd4ab8e Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Fri, 28 Jun 2024 13:06:43 +0000
>Subject: [PATCH] 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>
>---
> .../en/modules/labels/label-edit-batch.tt     | 26 +++++++++++++------
> labels/label-edit-batch.pl                    |  6 +----
> 2 files changed, 19 insertions(+), 13 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-edit-batch.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-edit-batch.tt
>index ddf93049e0..15acdb833e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-edit-batch.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-edit-batch.tt
>@@ -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> 
>      </div> 
> 
>+    <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 == '') {
>diff --git a/labels/label-edit-batch.pl b/labels/label-edit-batch.pl
>index 23bd9e77d4..1a8759b49b 100755
>--- a/labels/label-edit-batch.pl
>+++ b/labels/label-edit-batch.pl
>@@ -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
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 37206: 168237 | 168288 | 168341