Bugzilla – Attachment 162784 Details for
Bug 36193
CSRF - Code review missed
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36193: (follow-up) Use buttons and don't nest forms
Bug-36193-follow-up-Use-buttons-and-dont-nest-form.patch (text/plain), 17.56 KB, created by
Tomás Cohen Arazi (tcohen)
on 2024-03-05 17:10:16 UTC
(
hide
)
Description:
Bug 36193: (follow-up) Use buttons and don't nest forms
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2024-03-05 17:10:16 UTC
Size:
17.56 KB
patch
obsolete
>From 556ca512142664fa1c3e111ab343a5777f7c26f8 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 5 Mar 2024 16:21:31 +0000 >Subject: [PATCH] Bug 36193: (follow-up) Use buttons and don't nest forms > >When changing action links into forms, we should use a submit button rather than JS to submit > >Receipt cancelling needed a form embedded in the page as we cannot nest forms > >Fixed searching on invoice.pl > >Fixed resetting mappings (original button did not need cud-) > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > acqui/invoices.pl | 4 +-- > .../prog/en/modules/acqui/parcel.tt | 15 ++++---- > .../prog/en/modules/admin/cash_registers.tt | 35 ++++--------------- > .../prog/en/modules/admin/credit_types.tt | 15 ++------ > .../prog/en/modules/admin/debit_types.tt | 15 ++------ > .../prog/en/modules/admin/restrictions.tt | 8 +---- > .../searchengine/elasticsearch/mappings.tt | 2 +- > 7 files changed, 22 insertions(+), 72 deletions(-) > >diff --git a/acqui/invoices.pl b/acqui/invoices.pl >index dce1a4d668b..5cf2e338aae 100755 >--- a/acqui/invoices.pl >+++ b/acqui/invoices.pl >@@ -80,7 +80,7 @@ for my $field (@additional_fields) { > } > > my $invoices = []; >-if ( $op and $op eq 'cud-do_search' ) { >+if ( $op and $op eq 'do_search' ) { > @{$invoices} = GetInvoices( > invoicenumber => $invoicenumber, > supplierid => $supplierid, >@@ -142,7 +142,7 @@ $template->{'VARS'}->{'budgets_loop'} = \@budgets_loop; > $template->param( > openedinvoices => \@openedinvoices, > closedinvoices => \@closedinvoices, >- do_search => ( $op and $op eq 'cud-do_search' ) ? 1 : 0, >+ do_search => ( $op and $op eq 'do_search' ) ? 1 : 0, > invoices => $invoices, > invoicenumber => $invoicenumber, > booksellerid => $supplierid, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt >index 74badf6d47e..33dd94ba605 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt >@@ -294,12 +294,6 @@ > Can't cancel receipt > </span> > [% ELSE %] >- <form id="cancel_receipt_[% order.ordernumber | html %]" method="post" action="/cgi-bin/koha/acqui/parcel.pl"> >- [% INCLUDE 'csrf-token.inc' %] >- <input type="hidden" name="op" value="cud-cancelreceipt" /> >- <input type="hidden" name="ordernumber" value="[% order.ordernumber | html %]" /> >- <input type="hidden" name="invoiceid" value="[% invoiceid | html %]" /> >- </form> > <a href="#" class="cancel_receipt" data-ordernumber="[% order.ordernumber | html %]">Cancel receipt</a> > [% END %] > </td> >@@ -308,6 +302,12 @@ > </tbody> > </table> > </form> >+ <form id="cancel_receipt" method="post" action="/cgi-bin/koha/acqui/parcel.pl"> >+ [% INCLUDE 'csrf-token.inc' %] >+ <input type="hidden" name="op" value="cud-cancelreceipt" /> >+ <input type="hidden" name="ordernumber" id="cancel_ordernumber" value="" /> >+ <input type="hidden" name="invoiceid" value="[% invoiceid | html %]" /> >+ </form> > > [% ELSE %]There are no received orders.[% END %] > </div> >@@ -367,7 +367,8 @@ > > $(".cancel_receipt").on( 'click', function(e){ > e.preventDefault(); >- $('#cancel_receipt_' + $(this).data('ordernumber')).submit(); >+ $('#cancel_ordernumber').val( $(this).data('ordernumber') ); >+ $('#cancel_receipt').submit(); > }); > > if ( $("#receivedt").length ) { >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt >index 06ef94bcaef..e8a733149d4 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt >@@ -189,39 +189,39 @@ > <td>[% cash_register.library.branchname | html %]</td> > <td>[% IF cash_register.branch_default %]Yes[% ELSE %]No[% END %]</td> > <td>[% cash_register.starting_float | $Price %]</td> >- [% IF cash_register.archived == '0' %] > <td class="actions"> >+ [% IF cash_register.archived == '0' %] > <a class="btn btn-default btn-xs" href="cash_registers.pl?op=add_form&id=[% cash_register.id | uri %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a> > [% IF cash_register.branch_default %] > <form id="drop_default_[% cash_register.id | html %]" method="post" action="/cgi-bin/koha/admin/cash_registers.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-drop_default" /> > <input type="hidden" name="id" value="[% cash_register.id | html %]" /> >+ <button type="submit" id="drop_default_button" class="btn btn-default btn-xs" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Drop default</button> > </form> >- <a id="drop_default_button" class="btn btn-default btn-xs" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Drop default</a> > [% ELSE %] > <form id="make_default_[% cash_register.id | html %]" method="post" action="/cgi-bin/koha/admin/cash_registers.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-make_default" /> > <input type="hidden" name="id" value="[% cash_register.id | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs make_default_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Make default</button> > </form> >- <a class="btn btn-default btn-xs make_default_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Make default</a> > [% END %] > <form id="archive_[% cash_register.id | html %]" method="post" action="/cgi-bin/koha/admin/cash_registers.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-archive" /> > <input type="hidden" name="id" value="[% cash_register.id | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs archive_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Archive</button> > </form> >- <a class="btn btn-default btn-xs archive_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-archive"></i> Archive</a> >- </td> > [% ELSE %] > <form id="unarchive_[% cash_register.id | html %]" method="post" action="/cgi-bin/koha/admin/cash_registers.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-unarchive" /> > <input type="hidden" name="id" value="[% cash_register.id | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs unarchive_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-trash-can-restore"></i> Restore</button> > </form> >- <td class="actions"><a class="btn btn-default btn-xs unarchive_button" href="#" data-id="[% cash_register.id | html %]"><i class="fa fa-trash-can-restore"></i> Restore</a></td> > [% END %] >+ </td> > </tr> > [% END %] > </tbody> >@@ -277,29 +277,6 @@ > filterDataTable( crtable, 2, opt ); > }); > >- $("#drop_default_button").on("click", function(e){ >- e.preventDefault(); >- var id = $(this).data('id'); >- $('#drop_default_'+id).submit(); >- }); >- >- $(".make_default_button").on("click", function(e){ >- e.preventDefault(); >- var id = $(this).data('id'); >- $('#make_default_'+id).submit(); >- }); >- >- $(".archive_button").on("click", function(e){ >- e.preventDefault(); >- var id = $(this).data('id'); >- $('#archive_'+id).submit(); >- }); >- >- $(".unarchive_button").on("click", function(e){ >- e.preventDefault(); >- var id = $(this).data('id'); >- $('#unarchive_'+id).submit(); >- }); > }); > </script> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt >index 6d378cf2934..3415d0141a4 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt >@@ -206,16 +206,16 @@ > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-archive" /> > <input type="hidden" name="code" value="[% credit_type.code | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs archive_button" href="#" data-code="[% credit_type.code | html %]"><i class="fa fa-archive"></i> Archive</button> > </form> >- <a class="btn btn-default btn-xs archive_button" href="#" data-code="[% credit_type.code | html %]"><i class="fa fa-archive"></i> Archive</a> > [% END %] > [% IF !credit_type.is_system && credit_type.archived %] > <form id="unarchive_[% credit_type.code | html %]" method="post" action="/cgi-bin/koha/admin/credit_types.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-unarchive" /> > <input type="hidden" name="code" value="[% credit_type.code | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs unarchive_button" href="#" data-code="[% credit_type.code | html %]"><i class="fa fa-undo"></i> Restore</button> > </form> >- <a class="btn btn-default btn-xs unarchive_button" href="#" data-code="[% credit_type.code | html %]"><i class="fa fa-undo"></i> Restore</a> > [% END %] > </td> > </tr> >@@ -275,17 +275,6 @@ > //Start filtered > $('#filter_system').click(); > >- $(".archive_button").on("click", function(e){ >- e.preventDefault(); >- var code = $(this).data('code'); >- $('#archive_'+code).submit(); >- }); >- >- $(".unarchive_button").on("click", function(e){ >- e.preventDefault(); >- var code = $(this).data('code'); >- $('#unarchive_'+code).submit(); >- }); > }); > </script> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/debit_types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/debit_types.tt >index e42ab70a51d..df3737ed163 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/debit_types.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/debit_types.tt >@@ -227,16 +227,16 @@ > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-archive" /> > <input type="hidden" name="code" value="[% debit_type.code | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs archive_button" href="#" data-code="[% debit_type.code | html %]"><i class="fa fa-archive"></i> Archive</button> > </form> >- <a class="btn btn-default btn-xs archive_button" href="#" data-code="[% debit_type.code | html %]"><i class="fa fa-archive"></i> Archive</a> > [% END %] > [% ELSIF debit_type.archived %] > <form id="unarchive_[% debit_type.code | html %]" method="post" action="/cgi-bin/koha/admin/debit_types.pl"> > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-unarchive" /> > <input type="hidden" name="code" value="[% debit_type.code | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs unarchive_button" href="#" data-code="[% debit_type.code | html %]"><i class="fa fa-undo"></i> Restore</button> > </form> >- <a class="btn btn-default btn-xs unarchive_button" href="#" data-code="[% debit_type.code | html %]"><i class="fa fa-undo"></i> Restore</a> > [% END %] > </td> > </tr> >@@ -295,17 +295,6 @@ > //Start filtered > $('#filter_system').click(); > >- $(".archive_button").on("click", function(e){ >- e.preventDefault(); >- var code = $(this).data('code'); >- $('#archive_'+code).submit(); >- }); >- >- $(".unarchive_button").on("click", function(e){ >- e.preventDefault(); >- var code = $(this).data('code'); >- $('#unarchive_'+code).submit(); >- }); > }); > </script> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt >index 0ed1afdc3f0..b6de3187529 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt >@@ -229,8 +229,8 @@ > [% INCLUDE 'csrf-token.inc' %] > <input type="hidden" name="op" value="cud-make_default" /> > <input type="hidden" name="code" value="[% restriction.code | html %]" /> >+ <button type="submit" class="btn btn-default btn-xs make_default_button" href="#" data-code="[% restriction.code | html %]"><i class="fa fa-archive"></i> Make default</button> > </form> >- <a class="btn btn-default btn-xs make_default_button" href="#" data-code="[% restriction.code | html %]"><i class="fa fa-archive"></i> Make default</a> > [% END %] > </td> > </tr> >@@ -276,12 +276,6 @@ > } > }); > >- $(".make_default_button").on("click", function(e){ >- e.preventDefault(); >- var code = $(this).data('code'); >- $('#make_default_'+code).submit(); >- }); >- > </script> > [% Asset.js("js/restrictiontypes.js") | $raw %] > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >index cd6dc0dd9f9..3ade4c3c1e7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >@@ -506,7 +506,7 @@ a.add, a.delete { > > <fieldset class="action"> > <button class="btn btn-primary" type="submit" name="op" value="cud-edit"><i class="fa-solid fa-hard-drive" aria-hidden="true"></i> Save</button> >- <button class="btn btn-default" type="submit" name="op" value="cud-reset_confirm"><i class="fa-solid fa-rotate" aria-hidden="true"></i> Reset mappings</button> >+ <button class="btn btn-default" type="submit" name="op" value="reset_confirm"><i class="fa-solid fa-rotate" aria-hidden="true"></i> Reset mappings</button> > </fieldset> > </form> > >-- >2.44.0
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 36193
:
162684
|
162733
|
162734
|
162749
|
162750
|
162751
|
162752
|
162755
|
162769
|
162770
|
162771
|
162772
|
162773
|
162774
|
162775
|
162776
|
162777
| 162784 |
162830
|
162831
|
162837