[Bug 37767] New: Fix forms that POST without an op in Authority types
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Bug ID: 37767 Summary: Fix forms that POST without an op in Authority types Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P3 Component: System Administration Assignee: koha-bugs@lists.koha-community.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. In Authority types (the odd name for MARC authority frameworks), that's the "No, do not delete" cancel button when you decide not to go through with deleting a tag, which doesn't need to POST since all it does is take you back to where you were, and the OK button in the page that tells you the tag was deleted when you decide to go ahead, which currently doesn't even show, but when it does, it doesn't need to POST because it's just taking you back to where you were. 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 the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |phil@chetcolibrary.org |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 --- Comment #1 from Phil Ringnalda <phil@chetcolibrary.org> --- Lesson learned: don't try to file multiple near-identical bugs late in the evening, you'll confuse them. The two forms in Authority types are the cancel button when you decide not to delete a tag, and the search for tags form (there is no confirmation page after deleting a tag). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 --- Comment #2 from Phil Ringnalda <phil@chetcolibrary.org> --- Sigh. There is a delete confirm page, and it doesn't show because auth_tag_structure.pl just does a print $input->redirect() and exits after deleting a tag. The cancel button when you decide not to delete a tag has already been switched to a get. The copy-pasted but not kept in sync duplicated code between Authority types and MARC bibliographic frameworks is annoying. The two forms that POST without an op are dead code in the template for a delete confirm page, and the search for tags, which like MARC bibliographic frameworks in bug 37766 should be a GET so you can bookmark or link to searches for a particular tag in a particular framework. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 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=37767 --- Comment #3 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 170894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170894&action=edit Bug 37767: Fix forms that POST without an op in Authority types 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. In Authority types, that's dead code in the template that expects to show a "Data deleted" confirmation page, but it never shows because rather than setting the variable delete_confirmed and outputting the template, the op cud-delete_confirmed just does a print $input->redirect() and exits, and, the search for tags form that should be a GET so it can be bookmarked and linked to. Test plan: 1. Without the patch, Administration - Authority types - choose one other than default, so you can see the code rather than "" in the URL - Actions - MARC structure 2. For any tag (you *are* testing in a throwaway database, right?) click Actions - Delete then Yes, delete 3. Note that you don't get a confirmation page, just redirected back to a search for your tag which no longer exists, with the searchfield and authtypecode nicely in the URL 4. Click the Search button for the Search for tag form, without changing anything 5. Note that your URL lost the searchfield and authtypecode 6. Apply patch, restart_all 7. Repeat steps 1, 2, and 3, with identical results like they should be 8. Repeat step 4, but this time note that the searchfield and authtypecode stay in the URL. 9. Change the select menu for In framework and click Search, note that the searchfield and frameworkcode are still in the URL and still correct and that the correct results show 10. Change the tag number and hit Enter, and verify that the URL and the page show the correct results 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=37767 --- Comment #4 from Phil Ringnalda <phil@chetcolibrary.org> --- And yes, after all my confusion about what I was fixing here, I did apply the patch from bug 37728 on top of my patch and verify that xt/find-missing-op-in-forms.t was no longer finding any in auth_tag_structure.tt :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Sukhmandeep <sukhmandeep.benipal@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170894|0 |1 is obsolete| | --- Comment #5 from Sukhmandeep <sukhmandeep.benipal@inLibro.com> --- Created attachment 171216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171216&action=edit Bug 37767: Fix forms that POST without an op in Authority types 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. In Authority types, that's dead code in the template that expects to show a "Data deleted" confirmation page, but it never shows because rather than setting the variable delete_confirmed and outputting the template, the op cud-delete_confirmed just does a print $input->redirect() and exits, and, the search for tags form that should be a GET so it can be bookmarked and linked to. Test plan: 1. Without the patch, Administration - Authority types - choose one other than default, so you can see the code rather than "" in the URL - Actions - MARC structure 2. For any tag (you *are* testing in a throwaway database, right?) click Actions - Delete then Yes, delete 3. Note that you don't get a confirmation page, just redirected back to a search for your tag which no longer exists, with the searchfield and authtypecode nicely in the URL 4. Click the Search button for the Search for tag form, without changing anything 5. Note that your URL lost the searchfield and authtypecode 6. Apply patch, restart_all 7. Repeat steps 1, 2, and 3, with identical results like they should be 8. Repeat step 4, but this time note that the searchfield and authtypecode stay in the URL. 9. Change the select menu for In framework and click Search, note that the searchfield and frameworkcode are still in the URL and still correct and that the correct results show 10. Change the tag number and hit Enter, and verify that the URL and the page show the correct results Sponsored-by: Chetco Community Public Library Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Sukhmandeep <sukhmandeep.benipal@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sukhmandeep.benipal@inLibro | |.com 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=37767 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171216|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 171537 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171537&action=edit Bug 37767: Fix forms that POST without an op in Authority types 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. In Authority types, that's dead code in the template that expects to show a "Data deleted" confirmation page, but it never shows because rather than setting the variable delete_confirmed and outputting the template, the op cud-delete_confirmed just does a print $input->redirect() and exits, and, the search for tags form that should be a GET so it can be bookmarked and linked to. Test plan: 1. Without the patch, Administration - Authority types - choose one other than default, so you can see the code rather than "" in the URL - Actions - MARC structure 2. For any tag (you *are* testing in a throwaway database, right?) click Actions - Delete then Yes, delete 3. Note that you don't get a confirmation page, just redirected back to a search for your tag which no longer exists, with the searchfield and authtypecode nicely in the URL 4. Click the Search button for the Search for tag form, without changing anything 5. Note that your URL lost the searchfield and authtypecode 6. Apply patch, restart_all 7. Repeat steps 1, 2, and 3, with identical results like they should be 8. Repeat step 4, but this time note that the searchfield and authtypecode stay in the URL. 9. Change the select menu for In framework and click Search, note that the searchfield and frameworkcode are still in the URL and still correct and that the correct results show 10. Change the tag number and hit Enter, and verify that the URL and the page show the correct results Sponsored-by: Chetco Community Public Library Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com> 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=37767 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=37767 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 --- Comment #7 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=37767 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED Resolution|--- |FIXED CC| |lucas@bywatersolutions.com --- Comment #8 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Doesnt apply to 24.05.x, please rebase if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37767 Bug 37767 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