In many points now, in particular in CAS authentication, we use the system preference staffClientBaseURL as OPACBaseURL. The preference should be a complete URL, starting with http:// or https://. So we need to change svc/import_bib line 101. From my $url = 'http://'. C4::Context->preference('staffClientBaseURL') .'/cgi-bin/koha/catalogue/detail.pl?biblionumber='. $biblionumber; To my $url = C4::Context->preference('staffClientBaseURL') .'/cgi-bin/koha/catalogue/detail.pl?biblionumber='. $biblionumber; It is probably a good idea to change also koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_client.pref. Like OPACBaseURL: " . This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)
Created attachment 66025 [details] [review] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t
Glad to see this patch - When using CAS for example it's already required that you add https:// in order to make things work right.
Created attachment 66143 [details] [review] [SIGNED OFF] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66712 [details] [review] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
What about the following query instead? UPDATE systempreferences SET value = CONCAT('http://', value) WHERE variable = 'staffClientBaseURL' AND value <> "" AND value NOT LIKE '^http%'; staffClientBaseURL is not supposed to be empty, but it will avoid to have it set to "http://"
Created attachment 67268 [details] [review] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67269 [details] [review] Bug 16401: (follow-up) Avoid changing empty staffClientBaseURL preference Adds a condition to check that the staffClientBaseUrl is not empty before updating it with 'http://'.
Created attachment 67270 [details] [review] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67271 [details] [review] Bug 16401: (follow-up) Avoid changing empty staffClientBaseURL preference Adds a condition to check that the staffClientBaseUrl is not empty before updating it with 'http://'.
I like Jonathan's idea and have written a follow-up for it. The change is tiny, so pushing it back to QA.
C4::Context has a OPACBaseURL change to make sure a protocol is prepended. Probably should do something similar for staffBaseUrl. Also, are there any letters/noticed which might be affected?
I don't see how this change would affect notices or letters. When I tested this initially I checked all occurrences of the preference had been taken into account.
Created attachment 67918 [details] [review] Bug 16401: System preference staffClientBaseURL hardcoded to 'http://' Test plan: 1) Insert some value to staffBaseURL preference, without starting http:// 2) Apply the patch 3) Update database 4) Go to system preferences adminsitration, find the staffBaseURL preference, the inserted value should be prepended with 'http://' and the comment should be "This should be a complete URL, starting with http:// or https://. Do not include a trailing slash in the URL. (This must be filled in correctly for CAS, svc, and load_testing to work.)" 5) prove t/db_dependent/Auth_with_cas.t 6) prove t/db_dependent/selenium/basic_workflow.t 7) prove t/db_dependent/check_sysprefs.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 67919 [details] [review] Bug 16401: (follow-up) Avoid changing empty staffClientBaseURL preference Adds a condition to check that the staffClientBaseUrl is not empty before updating it with 'http://'. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Katrin Fischer from comment #12) > I don't see how this change would affect notices or letters. When I tested > this initially I checked all occurrences of the preference had been taken > into account. Are there any letters with something like <<staffClientBaseURL>>? Because OPACBaseURL had some with http:// in front for some letters/notices. I vaguely remember checking and not finding anything, but that is how. So Passed QA is fine with me.
(In reply to M. Tompsett from comment #15) > (In reply to Katrin Fischer from comment #12) > Are there any letters with something like <<staffClientBaseURL>>? I didn't see anything in my check, doesn't mean a library isn't using it in their custom notices, but I think it is a minor local adjustment if needed
Created attachment 67967 [details] [review] Bug 16401: Add a warning if letter.content matches staffClientBaseURL
Pushed to master for 17.11, thanks to everybody involved!