[Bug 37768] New: Fix form that POSTs without an op in itemtype administration
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Bug ID: 37768 Summary: Fix form that POSTs without an op in itemtype administration Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P3 Component: System Administration Assignee: phil@chetcolibrary.org Reporter: phil@chetcolibrary.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Depends on: 36192 Blocks: 37728 We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. For itemtype administration, that's the "No, do not delete" cancel button when you decide not to delete an itemtype, which doesn't need to POST anything since it's just taking you back to the list of itemtypes Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36192 [Bug 36192] [OMNIBUS] CSRF Protection for Koha https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37728 [Bug 37728] More "op" are missing in POSTed forms -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #1 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 170892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170892&action=edit Bug 37768: Fix form that POSTs without an op in itemtype administration We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. For itemtype administration, that's the "No, do not delete" cancel button when you decide not to delete an itemtype, which doesn't need to POST anything since it's just taking you back to the list of itemtypes. The only visible change from switching to a GET is that the URL ends with a "?" from a GET with no params, but someone can fix that by choosing one of our various link-as-a-cancel-button styles and switching it to a link in a bug that doesn't block an RM_priority bug. Test plan: 1. You aren't going to see a visible difference, so start with the patch applied 2. Administration - Item types 3. You need an itemtype that isn't in use to be able to delete it - ktd provides you with an unused Computer Files type, so click the Delete button for that row 4. In the "Are you sure..." page, click No, do not delete 5. Verify that you are back at the list of itemtypes, with only the "?" at the end of the URL to tell you that you did a GET rather than a POST Sponsored-by: Chetco Community Public Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- Owen is this the expected UI? Don't we want to display a link instead? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #3 from Phil Ringnalda <phil@chetcolibrary.org> --- Do we want to display a link, like we sometimes do, or a link pretending to be a button, like we sometime do, or a button being a button, like we sometimes do? Most of these bugs blocking bug 37728 have at least one form that doesn't need to be a form, if you want to delay landing that long enough to change them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #4 from Phil Ringnalda <phil@chetcolibrary.org> --- Oh, and most of them are loading a new page to ask for confirmation even when they don't need to look up whether they are going to disallow deleting something because it's in use, a thing which we generally ask with just a confirm popup now, do we want to switch to doing that instead? And even for things that we won't let you delete because they are in use, we generally don't load a new page to look that up, we just don't let you delete them, like authority terms linked to bib records where there just isn't a delete button if you can't delete, should we switch that as well? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170892|0 |1 is obsolete| | --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 170958 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170958&action=edit Bug 37768: Fix form that POSTs without an op in itemtype administration We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. For itemtype administration, that's the "No, do not delete" cancel button when you decide not to delete an itemtype, which doesn't need to POST anything since it's just taking you back to the list of itemtypes. The only visible change from switching to a GET is that the URL ends with a "?" from a GET with no params, but someone can fix that by choosing one of our various link-as-a-cancel-button styles and switching it to a link in a bug that doesn't block an RM_priority bug. Test plan: 1. You aren't going to see a visible difference, so start with the patch applied 2. Administration - Item types 3. You need an itemtype that isn't in use to be able to delete it - ktd provides you with an unused Computer Files type, so click the Delete button for that row 4. In the "Are you sure..." page, click No, do not delete 5. Verify that you are back at the list of itemtypes, with only the "?" at the end of the URL to tell you that you did a GET rather than a POST Sponsored-by: Chetco Community Public Library Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- Definitely not the place to fix the UI discrepancies, but would be nice to see more consistencies. I have no idea what we want however :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Trivial, skipping QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #8 from Phil Ringnalda <phil@chetcolibrary.org> --- I haven't come up with a way to find all instances of this pattern, but I have 32 tabs open to the ones that were easy to find, if anyone wants to choose our One True Way to do a cancel buttonlink. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Phil Ringnalda from comment #8)
I haven't come up with a way to find all instances of this pattern, but I have 32 tabs open to the ones that were easy to find, if anyone wants to choose our One True Way to do a cancel buttonlink.
I discussed this with Owen (pm) and the conclusion is that "No, do not delete" is in an alert box and the use of a button is the current pattern in this case. It seems that we are consistent in Koha (always using a "yes" button and a "no" button). Then... to go further, it's a rabbit hole. You can start there: https://ux.stackexchange.com/questions/82492/yes-no-button-placement-on-conf... We could discuss it more on a separate bug report, but we will need time and energy to "fix" all of the occurrences. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think working towards more consistency is a good goal. Could we get a bug report about the findings here so that we don't lose sight of this? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to main |RESOLVED CC| |lucas@bywatersolutions.com --- Comment #12 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Merge conflicts in 24.05.x, no backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37768 Bug 37768 depends on bug 36192, which changed state. Bug 36192 Summary: [OMNIBUS] CSRF Protection for Koha https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36192 What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org