Created attachment 51912 [details] [review] Patch: Open Library API via SSL When running Koha in a full-site-SSL configuration accessing external resources like Open Library cover images via HTTP doesn't work (mixed SSL/non-SSL content). It thus breaks a core feature of Koha. Referencing such resources always via HTTPS resolves the problem and also enhances privacy as a side-effect. Previous tickets took care of Amazon resources. This ticket is supposed to address the remaining external resource providers, starting with Open Library.
Hi Oliver, just found this bug: 1) Could you reattach the file as a complete patch file? It's only the diff, but it doesn't have the information about the authorship etc. as you can see on other bugs. There are at least 2 ways to do this: - git format-patch (https://wiki.koha-community.org/wiki/Version_Control_Using_Git) - git bz (https://wiki.koha-community.org/wiki/Git_bz_configuration) In both cases, make sure you set up your git config before you create the file so it contains your name and e-mail address. 2) After attaching a patch file, don't forget to change the status to "Needs Sign-off". This will make the bug show up in the lists the testers look at - otherwise your work can be easily missed. 3) If you provide a patch, don't forget to assign the bug to yourself, changing the 'Assignee' in the bug report. Please let us know if we can help you with any of that or if there are other questions about the development process.
Created attachment 54440 [details] [review] Bug 16615: Open Library API fails for full-site-SSL When running Koha in a full-site-SSL configuration accessing external resouces like Open Library cover images via HTTP doesn't work (mixed SSL/non-SSL content). It thus breaks a core feature of Koha. Referencing such resources always via HTTPS resolves the problem and also enhances privacy as a side-effect. Previous tickets took care of Amazon resources. This ticket is supposed to address the remaining external resource providers, starting with Open Library. TEST PLAN --------- 1) Over https access your OPAC with the Enhanced Content system preferences 'OpenLibraryCovers' set to 'Add', and 'OpenLibrarySearch' set to 'Show'. 2) Search for something (e.g. Mo Willems) -- Your browser console log will show messages about mixed content. 3) Apply the patch 4) Making sure to reload the java script, attempt steps 1 & 2 again. -- no mixed content messages should be given 5) run koha qa test tools
Created attachment 54441 [details] [review] Bug 16615: Follow up - missed a spot Also, it is recommended to apply bug 17103 before this.
Created attachment 54442 [details] [review] Bug 16615: Open Library API fails for full-site-SSL When running Koha in a full-site-SSL configuration accessing external resouces like Open Library cover images via HTTP doesn't work (mixed SSL/non-SSL content). It thus breaks a core feature of Koha. Referencing such resources always via HTTPS resolves the problem and also enhances privacy as a side-effect. Previous tickets took care of Amazon resources. This ticket is supposed to address the remaining external resource providers, starting with Open Library. TEST PLAN --------- 1) Over https access your OPAC with the Enhanced Content system preferences 'OpenLibraryCovers' set to 'Add', and 'OpenLibrarySearch' set to 'Show'. 2) Search for something (e.g. Mo Willems) -- Your browser console log will show messages about mixed content. 3) Apply the patch 4) Making sure to reload the java script, attempt steps 1 & 2 again. -- no mixed content messages should be given 5) run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 54443 [details] [review] Bug 16615: Follow up - missed a spot Also, it is recommended to apply bug 17103 before this. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 54444 [details] [review] Bug 17103 - Google API Loader jsapi called over http This patch replaces the hardcoded protocol ("http") from the URI with https since that is what Google defacto uses to serve JS resources Prevents MIXED CONTENT failure, allowing GoogleIndicTransliteration to function correctly when used in SSLized OPACs. Test plan --------- 1/ Make sure your OPAC is being served both over HTTP and HTTPS 2/ Set GoogleIndicTransliteration syspref to "Show" 3/ Access the OPAC over http, Google transliteration element will show up in masthead searchbox. 4/ Access the OPAC over https. The element will not show. Console will show MIXED CONTENT error and failure of google loader. 5/ Apply patch. Repeat steps #3 and #4 again. Now in both cases GoogleIndicTransliteration will work. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Oops... Sorry, I was trying to set the signed off status.
Comment on attachment 54444 [details] [review] Bug 17103 - Google API Loader jsapi called over http My mistake. Sorry.
I've signed off the first, and added the second. A single 's' shouldn't really need a secondary sign off should it?
Now this moved faster than I expected :) Oliver, would you be able to sign-off Mark's follow-up?
Hi guys, I would sign it off code-wise but I'm not able to sign it off based on the test plan just because I can't test it that way (no HTTP/non-SSL access possible). That said, in principle I'm fine with the patch. Best, Oliver PS: FYI, I stripped my local git details from the original patch as I didn't remember that you use git at all ;-)
Feel free to sign off - I or someone else will probably test without HTTPS in QA... :) Take a look at the git bz link - I think it will explain some about our workflow with git and Bugzilla.
Created attachment 54456 [details] [review] Bug 16615: Follow up - missed a spot (signed-off) Attaching the signed-off patch manually as "git bz attach" failed because of some missing default. Maybe the wiki defaults list is incomplete? Also, please try to get rid of putting the clear-text PW into any gitconfig. git bz should really prompt for it.
Created attachment 54462 [details] [review] Bug 16615: Open Library API fails for full-site-SSL When running Koha in a full-site-SSL configuration accessing external resouces like Open Library cover images via HTTP doesn't work (mixed SSL/non-SSL content). It thus breaks a core feature of Koha. Referencing such resources always via HTTPS resolves the problem and also enhances privacy as a side-effect. Previous tickets took care of Amazon resources. This ticket is supposed to address the remaining external resource providers, starting with Open Library. TEST PLAN --------- 1) Over https access your OPAC with the Enhanced Content system preferences 'OpenLibraryCovers' set to 'Add', and 'OpenLibrarySearch' set to 'Show'. 2) Search for something (e.g. Mo Willems) -- Your browser console log will show messages about mixed content. 3) Apply the patch 4) Making sure to reload the java script, attempt steps 1 & 2 again. -- no mixed content messages should be given 5) run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54463 [details] [review] Bug 16615: Follow up - missed a spot Also, it is recommended to apply bug 17103 before this. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Oliver Bock <oliver.bock@aei.mpg.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54464 [details] [review] Bug 16615: Update OPACSearchForTitleIn to access openlibrary.org via https Note that we could also update the syspref for existing installs Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The previous patch only updates the OpenLibrary URL. Why not change all of them (after testing they support it)?
(In reply to Oliver Bock from comment #17) > The previous patch only updates the OpenLibrary URL. Why not change all of > them (after testing they support it)? Because I did not test them :) Feel free to submit a patch to replace it.
Since the Passed QA scope is OpenLibrary only. I figure another bug is better suited to fixing other issues. That's why I renamed this.
Too bad. I intentionally named this bug without a specific service because all previous fixes so far typically only covered a single service. That means all the rest doesn't get fixed. My hope was to kick off a general fixing round, starting things of myself with OpenLibrary. This (intended) effort depends of course on others stepping in to fix the rest. Changing this back to the old scheme seems like a missed opportunity. Best, Oliver
It's just a workflow thing. Once patches have been pushed we don't 'reuse' the bug for more patches - it causes too much confusion with the status and also our release notes are created from the bug titles. We use one Omnibus bug with linked bugs for the separate issues. I will create an Omnibus bug and links this one to show how it works.
https://bugs.koha-community.org/bugzilla3/showdependencytree.cgi?id=17137&hide_resolved=1
Ok, sounds good. This should also ensure that completed patches can already be rolled out with the next release. Thanks, Oliver
Pushed to master for 16.11, thanks Oliver, Mark and Jonathan!
Sure, no problem. Hopefully my other tickets will also be processed soon... One final question: is this going to be backported to 3.22 as well? We're still on that release. Thanks, Oliver
It's marked as a bug (not enh) so the RMaints will take a look at this and decide.
Pushed in 16.05. Will be in 16.05.03.
Hm, this bug is still assigned to me although there's no pending task. How to deal with this? Should it just be closed? Thanks.
(In reply to Oliver Bock from comment #28) > Hm, this bug is still assigned to me although there's no pending task. How > to deal with this? Should it just be closed? > > Thanks. Status is "pushed to stable" and should be marked as closed soon.