[Bug 31699] New: Add a generic way to redirect back to the page you were on at login for modal logins
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Bug ID: 31699 Summary: Add a generic way to redirect back to the page you were on at login for modal logins Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Currently, we have a few extra buttons in the OPAC that trigger the login modal.. however, in most cases those don't do anything clever to keep the users context and instead they result with the jarring shift to the opac-user page once logged in. We should implement a general redirect option to allow such modal trigger to specify that context should be maintained. Example - Trying to add a comment on the opac-detail page when not logged in. It'll trigger a login modal and then redirect you to opac-user upon success losing the context of the item you were trying to comment on. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 141420 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141420&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 141421 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141421&action=edit Bug 31699: Impliment modal context return on opac-detail. This patch utilises the generic return option introduced in the previous patch to prevent lose of context when clicking to login to add a comment to a biblio. Test plan 1/ Test that prior to this patch, upon clicking 'Log in to your account to post a comment' you are redirected to opac-user.pl and thus loose your context. 2/ Apply the patch 3/ Confirm that a login using the 'Log in to your account to post a comment' link now redirect immediatly back the correct record and thus maintains context. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.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=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |31028 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31028 [Bug 31028] Add 'Report a concern' feature for patrons to report concerns about catalog records -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #3 from David Nind <david@davidnind.com> --- Hi Martin. This isn't working for me: - I tried in Firefox and Google Chrome - I applied the patch, flush_memcached, and restart_all - For browsers, I tried clearing the cache and opening in ingognito mode/private window - Visited http://127.0.0.1:8080/cgi-bin/koha/opac-detail.pl?biblionumber=262 from both the search results page and going to the URL directly - Link for "Log in to your account to post a comment." is showing the changes in the patch (<a class="login-link loginModal-trigger" role="button" data-toggle="modal" data-return="true" href="/cgi-bin/koha/opac-user.pl">Log in to your account</a> to post a comment.) David -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141420|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142200 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142200&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141421|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142201&action=edit Bug 31699: Impliment modal context return on opac-detail. This patch utilises the generic return option introduced in the previous patch to prevent lose of context when clicking to login to add a comment to a biblio. Test plan 1/ Test that prior to this patch, upon clicking 'Log in to your account to post a comment' you are redirected to opac-user.pl and thus loose your context. 2/ Apply the patch 3/ Confirm that a login using the 'Log in to your account to post a comment' link now redirect immediatly back the correct record and thus maintains context. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for testing David.. something wierd had crept in.. I've amended the patch (checking for boolean truthyness instead of comparing to a string) and now it appears to be working again for me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 David Nind <david@davidnind.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=31699 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142200|0 |1 is obsolete| | --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 142204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142204&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142201|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 142205 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142205&action=edit Bug 31699: Impliment modal context return on opac-detail. This patch utilises the generic return option introduced in the previous patch to prevent lose of context when clicking to login to add a comment to a biblio. Test plan 1/ Test that prior to this patch, upon clicking 'Log in to your account to post a comment' you are redirected to opac-user.pl and thus loose your context. 2/ Apply the patch 3/ Confirm that a login using the 'Log in to your account to post a comment' link now redirect immediatly back the correct record and thus maintains context. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #9 from David Nind <david@davidnind.com> --- (In reply to Martin Renvoize from comment #6)
Thanks for testing David.. something wierd had crept in.. I've amended the patch (checking for boolean truthyness instead of comparing to a string) and now it appears to be working again for me.
Thanks Martin - this now works. I'm redirected back to the details page for the record I was viewing (but not the comments tab). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |This patch adds the ability released in| |to redirect users back to | |where they were when using | |the modal type logins in | |place of an action that | |requires login on the OPAC. | | | |Example: On the | |opac-detail page one can | |add comments if logged in. | |Prior to this patch, | |clicking the link to add a | |comment prior to being | |logged in would expose the | |login modal and then | |re-direct you to your | |opac-user page and thus | |loose the context of your | |action. With this patch | |you are redirected back to | |the biblio you were looking | |at so you can post your | |comment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This works well and is an improvement. Tiny glitch: We return to the same page, but the active tab is 'holdings', instead of 'comments' now. I was wondering: Maybe it would be nice to be able to set the return URL in the data attribute? Then we could also put things like the anchor for the tab. I don't think this is worth failing this tho :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142204|0 |1 is obsolete| | Attachment #142205|0 |1 is obsolete| | --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142733&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142734&action=edit Bug 31699: Implement modal context return on opac-detail This patch utilises the generic return option introduced in the previous patch to prevent lose of context when clicking to login to add a comment to a biblio. Test plan 1/ Test that prior to this patch, upon clicking 'Log in to your account to post a comment' you are redirected to opac-user.pl and thus loose your context. 2/ Apply the patch 3/ Confirm that a login using the 'Log in to your account to post a comment' link now redirect immediatly back the correct record and thus maintains context. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32028 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32028 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #10)
This works well and is an improvement.
Tiny glitch: We return to the same page, but the active tab is 'holdings', instead of 'comments' now. I was wondering: Maybe it would be nice to be able to set the return URL in the data attribute? Then we could also put things like the anchor for the tab.
I don't think this is worth failing this tho :)
So this isn't actually about setting the return url.. we already do that.. it's that the # values in the URI are local to the browser and not taken into account on return. I actually added code to deal with this for my catalog concerns bug, but for re-triggering the modal rather than switching tab.. It will need custom JS per case though I believe. Example of what I did for catalog concerns: let urlParams = new URLSearchParams(window.location.search); if ( urlParams.has('modal') ) { let modal = urlParams.get('modal'); if ( modal == 'concern' ) { $("#addConcernModal").modal('show'); } } We'd need to do something similar for 'tab' to trigger the JS to select the right tab after page load. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32125 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32125 [Bug 32125] Implement contextual return on opac comments -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142733|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143378&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142734|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=31699 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I decided to split this into two bugs.. one for the generic return code and one for the use of it in the biblio details comments tab. I also added a minor follow-up on the subsequent bug to ensure the correct tab is selected too. Keeping the QA stamp here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Do we need some form of validation for $return? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm, probably a good idea... You thinking something like a test that at the base matches OpacBaseURL? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #17)
Hmm, probably a good idea... You thinking something like a test that at the base matches OpacBaseURL?
At least the domain used to retrieve the page (maybe calculated). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #16)
Do we need some form of validation for $return?
100% The "return" param could either be window.location.pathname + window.location.search so the Perl could append that to OpacBaseURL, or "return" could be a full URL, and the Perl replaces the hostname with OpacBaseURL. This is the approach we use in C4::Auth_with_shibboleth, and it's something I've used elsewhere as well. The only downside I see is that it locks you into using the particular OpacBaseURL you have defined for that virtual host. Either way, we definitely need to validate/construct the URL, or else we're creating this problem: https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_For... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- I'd be tempted to mark this as Failed QA until it addresses the security hole, but I'll leave it up to the RM whether we do that or add a follow-up -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |ASSIGNED --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry chaps, it was late last night when this discussion took place. I've marked it assigned and will work on it today. Bit disappointed with myself that I didn't spot this at initial development. Still, it should be a trivial follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143463 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143463&action=edit Bug 31699: (follow-up) Protect against unauthorized redirects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143378|0 |1 is obsolete| | --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143464&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143463|0 |1 is obsolete| | --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143465&action=edit Bug 31699: (follow-up) Protect against unauthorized redirects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143464|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143468 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143468&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143465|0 |1 is obsolete| | --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143469 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143469&action=edit Bug 31699: (follow-up) Protect against unauthorized redirects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #27 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #21)
Sorry chaps, it was late last night when this discussion took place. I've marked it assigned and will work on it today.
Bit disappointed with myself that I didn't spot this at initial development. Still, it should be a trivial follow-up.
No worries. We're all human and it's why it's always good to have another set of eyes on things! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #28 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 143469 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143469 Bug 31699: (follow-up) Protect against unauthorized redirects Review of attachment 143469: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=31699&attachment=143469) ----------------------------------------------------------------- ::: opac/opac-user.pl @@ +428,5 @@
# back to the page we triggered the login from my $return = $query->param('return'); if ( $return ) { + my $uri = C4::Context->preference('OPACBaseURL'); + $uri .= $return;
If OPACBaseURL isn't set, which happens more often than I'd like, this would still be vulnerable to open redirects. I was thinking that we'd re-write the URL using the URI module. Today, I thought we could potentially get away with nuking the "scheme" and "authority", but then I figured out a URL trick to get it to redirect to a malicious URL. So I think we need to do a bit more validation. I'll write up a quick patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #29 from David Nind <david@davidnind.com> --- The last follow-up has "broken" the redirect used for bug 31028 - Catalog concerns (see comment 323 (!!)), so it may pay to test with that bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #30 from David Cook <dcook@prosentient.com.au> --- (In reply to David Nind from comment #29)
The last follow-up has "broken" the redirect used for bug 31028 - Catalog concerns (see comment 323 (!!)), so it may pay to test with that bug.
That's interesting. I don't think it would've been the follow-up, so much as the whole feature. How were you setting the data-return attribute? I think that might be part of your problem? (Interestingly, it looks like opac-reserve.pl might be broken in master as well on an unrelated note...) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #31 from David Cook <dcook@prosentient.com.au> --- At the moment, with Martin's patches, the following will generate a "ERR_INVALID_REDIRECT": http://localhost:8080/cgi-bin/koha/opac-user.pl?return=http://koha-community... If I erase OPACBaseURL, then I get the open redirect vulnerability again. So now I'll add a patch... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #32 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #31)
If I erase OPACBaseURL, then I get the open redirect vulnerability again.
Worth noting that this affects only logged in users. The redirect code won't trigger for anonymous users, since we short-circuit opac-user.pl for anonymous users and prompt with a regular login box. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #33 from David Cook <dcook@prosentient.com.au> --- With my upcoming patch, Without OPACBaseURL, no redirection happens for the following: http://localhost:8080/cgi-bin/koha/opac-user.pl?return=http://koha-community... With OPACBaseURL, the return URL is rewritten to use the scheme and authority of OPACBaseURL, so that the following: http://localhost:8080/cgi-bin/koha/opac-user.pl?return=http://koha-community... Redirects to: http://localhost:8080/test?test=test#test -- So it shouldn't be possible to send a user anywhere other than http://localhost:8080. (An alternative would be to set the page visited in the user's database session and then juggle that around and try to return to that, but that would be a more difficult code change than Martin's patches here.) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #34 from David Cook <dcook@prosentient.com.au> --- Created attachment 143608 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143608&action=edit Bug 31699: (follow-up) Protect more against open redirects This change checks that the OPACBaseURL exists, and uses its scheme and authority to rewrite the URL passed through the "return" param. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- Note that while the code checks for URI::http as the URI class type, this includes URI::https since it subclasses URI::http, so it works for OPACBaseURL using either HTTP or HTTPS URLs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #36 from David Cook <dcook@prosentient.com.au> --- I was doing some more thinking... On one hand, I was thinking maybe we should make a more consistent login box. For instance, if you go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add_form, the opac-auth.tt template creates a login box that works as expected. But the modal sends you off to opac-user.pl. There's no reason the masthead.inc couldn't create the same login box as opac-auth.tt. On the other hand, I like the idea of login endpoints (like with SSO), which need to redirect you back, so maybe this redirect is fine (so long as we're rewriting/validating the redirect URLs) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143608|0 |1 is obsolete| | --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143643 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143643&action=edit Bug 31699: (follow-up) Protect more against open redirects This change checks that the OPACBaseURL exists, and uses its scheme and authority to rewrite the URL passed through the "return" param. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #38 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for working on this David, your follow-up is indeed a much more thorough way of doing things.. my approach was a little lazy. Works great, -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #39 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, I'm still not convinced, to be honest. In production it is common to use several domain names for the same Koha instance, and I don't think sticking to OpacBaseURL is enough for that use case. In my opinion, using OpacBaseURL as a fixed value (instead of an allow-list) is a design mistake because of what I mentioned above. We should somehow be using x-forwarded-for or similar header. I know solving it is not this bug's responsability. But I think we should be able to find a nicer approach. That said, this is a bugfix and we can still think about it and if it doesn't make it to the release date it will be backported for sure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We use SetEnvIf to overcome the OpacBaseUrl thing already here.. your already setting up a host to get the different domains.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_05_candidate --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #40)
We use SetEnvIf to overcome the OpacBaseUrl thing already here.. your already setting up a host to get the different domains..
I'm sorry, I'm really overwhelmed by the amount of stuffs that need to be done/fixed for the release. This can be clearly considered a bugfix and pushed right after the release and backported if required. I can commit to that. I'm really sorry, but my brain bandwidth is exhausted right now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #42 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #39)
In my opinion, using OpacBaseURL as a fixed value (instead of an allow-list) is a design mistake because of what I mentioned above.
I was thinking about that a bit as well. With the OIDC server, they use an allow list to great effect. I figure Koha would possibly benefit in a few different places from URL redirect allow lists. (In reply to Martin Renvoize from comment #40)
We use SetEnvIf to overcome the OpacBaseUrl thing already here.. your already setting up a host to get the different domains..
But I also agree with this in terms of addressing the practical problem. (In reply to Tomás Cohen Arazi from comment #41)
I'm really sorry, but my brain bandwidth is exhausted right now.
You've been doing so much! We all understand! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143468|0 |1 is obsolete| | --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 144644 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144644&action=edit Bug 31699: Add 'return' option to opac modal login This patch adds the option to trigger a redirect back to the current context after successful login using modal logins triggered from various places in the OPAC. To make use of this, simply add 'data-return="true"' to the modal trigger link. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143469|0 |1 is obsolete| | --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 144645 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144645&action=edit Bug 31699: (follow-up) Protect against unauthorized redirects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143643|0 |1 is obsolete| | --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 144646 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144646&action=edit Bug 31699: (follow-up) Protect more against open redirects This change checks that the OPACBaseURL exists, and uses its scheme and authority to rewrite the URL passed through the "return" param. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #46 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30979 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30979 [Bug 30979] Add ability for OPAC users to checkout to themselves -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #47 from Tomás Cohen Arazi <tomascohen@gmail.com> --- No more rebases needed :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)|This patch adds the ability |This patch adds the ability released in|to redirect users back to |to redirect users back to |where they were when using |where they were when using |the modal type logins in |the modal type logins in |place of an action that |place of an action that |requires login on the OPAC. |requires login on the OPAC. | | |Example: On the |Example: On the |opac-detail page one can |opac-detail page one can |add comments if logged in. |add comments if logged in. |Prior to this patch, |Prior to this |clicking the link to add a |patch,23.05.00, clicking |comment prior to being |the link to add a comment |logged in would expose the |prior to being logged in |login modal and then |would expose the login |re-direct you to your |modal and then re-direct |opac-user page and thus |you to your opac-user page |loose the context of your |and thus loose the context |action. With this patch |of your action. With this |you are redirected back to |patch you are redirected |the biblio you were looking |back to the biblio you were |at so you can post your |looking at so you can post |comment. |your comment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #48 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement adds the release notes| |ability to redirect users | |back to where they were | |when using the modal type | |logins in place of an | |action that requires login | |on the OPAC. | | | |Example: On | |the OPAC detail page you | |can add comments if logged | |in. Prior to this patch, | |clicking the link to add a | |comment prior to being | |logged in would expose the | |login modal and then | |re-direct you to your OPAC | |user page, and thus lose | |the context of your action. | | With this enhancement, you | |are redirected back to the | |record you were looking at | |and can then post your | |comment. Version(s)|This patch adds the ability |23.05.00 released in|to redirect users back to | |where they were when using | |the modal type logins in | |place of an action that | |requires login on the OPAC. | | | |Example: On the | |opac-detail page one can | |add comments if logged in. | |Prior to this | |patch,23.05.00, clicking | |the link to add a comment | |prior to being logged in | |would expose the login | |modal and then re-direct | |you to your opac-user page | |and thus loose the context | |of your action. With this | |patch you are redirected | |back to the biblio you were | |looking at so you can post | |your comment. | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.03 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 --- Comment #49 from Jacob O'Mara <jacob.omara@ptfs-europe.com> --- Nice work, thanks everyone! Pushed to 22.11.x for the next release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED CC| |lucas@bywatersolutions.com --- Comment #50 from Lucas Gass <lucas@bywatersolutions.com> --- Enhancement will not be backported to 22.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21135 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21135 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21135 [Bug 21135] Upon log in, user is redirected to account page instead of the page where they logged in from -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #51 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Could this be used to solve bug 33175? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33175 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Keywords|rel_23_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31699 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement adds the |**Sponsored by** *The release notes|ability to redirect users |European Southern |back to where they were |Observatory* |when using the modal type | |logins in place of an |This |action that requires login |enhancement adds the |on the OPAC. |ability to redirect users | |back to where they were |Example: On |when using the modal type |the OPAC detail page you |logins in place of an |can add comments if logged |action that requires login |in. Prior to this patch, |on the OPAC. |clicking the link to add a | |comment prior to being |Example: On |logged in would expose the |the OPAC detail page you |login modal and then |can add comments if logged |re-direct you to your OPAC |in. Prior to this patch, |user page, and thus lose |clicking the link to add a |the context of your action. |comment prior to being | With this enhancement, you |logged in would expose the |are redirected back to the |login modal and then |record you were looking at |re-direct you to your OPAC |and can then post your |user page, and thus lose |comment. |the context of your action. | | With this enhancement, you | |are redirected back to the | |record you were looking at | |and can then post your | |comment. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org