[Bug 41197] New: opac/opac-account-pay-return.pl should not require CSRF
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Bug ID: 41197 Summary: opac/opac-account-pay-return.pl should not require CSRF Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org This page is intended for redirect/return from external payment vendors - they are not going to have/get a CSRF token from Koha for this. The payments are handled in a POST to the API, so this should not be a sensitive page, it just provides user confirmation. Some vendors use only a POST, we should not require CSRF on this page. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Can you run us through how this should work? I see a reference to opac-account-pay-return.pl in "opac_online_payment_begin.tt" in dev-koha-plugin-kitchen-sink, but it looks like it is an update so it should have a CSRF token (although it's missing in the plugin version I'm looking at). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to David Cook from comment #1)
Can you run us through how this should work?
I see a reference to opac-account-pay-return.pl in "opac_online_payment_begin.tt" in dev-koha-plugin-kitchen-sink, but it looks like it is an update so it should have a CSRF token (although it's missing in the plugin version I'm looking at).
So the general process is: 1 -opac payment begin - We select the charges to pay - the user/charges are then sent to the vendor site for processing payment 2 -the vendor should then POST to the API to make the actual payment and verify things on their end 3 - opac payment end - the payment vendor redirects the patron back to the Koha catalog, they may send some information about the payment, but the actual changes have been handled in step 2, step 3 is just reporting -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Assignee|oleonard@myacpl.org |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Kyle M Hall (khall) <kyle@bywatersolutions.com> 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=41197 --- Comment #3 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 203767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203767&action=edit Bug 41197: Add unit tests Patch from commit 328b8c8 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #4 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 203768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203768&action=edit Bug 41197: Don't require a CSRF token for opac-account-pay-return.pl This page is intended for redirect/return from external payment vendors - they are not going to have/get a CSRF token from Koha for this. The payments are handled in a POST to the API, so this should not be a sensitive page, it just provides user confirmation. Some vendors use only a POST, we should not require CSRF on this page. Test Plan: 1) Apply the first patch 2) prove t/Koha/Middleware/CSRF.t 3) Note the tests fail! 4) Apply this patch 5) prove t/Koha/Middleware/CSRF.t 6) All tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |critical --- Comment #5 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Bumping severity as this breaks some payment plugins. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Mmm OK I think that I understand now. Kyle's commit message clarifies things nicely. In some cases, they'd redirect with a GET, but some vendors POST back to Koha. That's reasonable. In the longer term, I think we'll want to think about an alternative to the %path_exceptions, but I think it's the best we've got right now, so this makes sense. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203767|0 |1 is obsolete| | Attachment #203768|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Created attachment 203775 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203775&action=edit Bug 41197: Add unit tests Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Created attachment 203776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203776&action=edit Bug 41197: Don't require a CSRF token for opac-account-pay-return.pl This page is intended for redirect/return from external payment vendors - they are not going to have/get a CSRF token from Koha for this. The payments are handled in a POST to the API, so this should not be a sensitive page, it just provides user confirmation. Some vendors use only a POST, we should not require CSRF on this page. Test Plan: 1) Apply the first patch 2) prove t/Koha/Middleware/CSRF.t 3) Note the tests fail! 4) Apply this patch 5) prove t/Koha/Middleware/CSRF.t 6) All tests pass! Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #9 from David Cook <dcook@prosentient.com.au> --- I think that this is a simple enough change but also an important enough change to go straight through to Passed QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.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=41197 --- Comment #10 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.11.00 |26.11.00,26.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41197 --- Comment #11 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to 26.05.x for the upcoming 26.05.05 release. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org