Bug 19799 - Changing language on OPAC redirects back to homepage
Summary: Changing language on OPAC redirects back to homepage
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 10 votes (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-12 14:28 UTC by Claire Gravely
Modified: 2020-05-04 00:51 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19799: Add localReferer function to module, and test (5.61 KB, patch)
2018-07-30 11:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19799: Introduce localReferer in opac-changelanguage.pl (1.84 KB, patch)
2018-07-30 11:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Claire Gravely 2017-12-12 14:28:11 UTC
Whilst looking at an item detail page on the OPAC (opac-detail.pl) you are redirected back to the OPAC homepage instead of staying on opac-detail. The language is successfully changed.

This is not an issue on master. We have tested it on 16.11, am not sure if it is an issue anywhere inbetween.
Comment 1 Owen Leonard 2018-02-07 16:37:07 UTC
I cannot reproduce this problem in 16.11.15.
Comment 2 Katrin Fischer 2018-02-07 18:45:47 UTC
Hm, still seeing it in our 16.11.15 installation.

How did you test?

1) Select English
2) Do a search in the OPAC
3) Switch language
4) You are returned to the start page instead of remaining on the result list
Comment 3 Katrin Fischer 2018-05-31 16:03:37 UTC
Also happens in 17.11
Comment 4 Katrin Fischer 2018-06-19 21:54:49 UTC
This seems to work alright in my devbox on master. Could someone confirm for 18.05?
Comment 5 Pablo AB 2018-07-25 18:02:16 UTC
Still happen on 18.05, very annoying.
Not only on search result (opac-search.pl) but anywhere on OPAC, even on user account (opac-user.pl).

Maybe language should be part of the URL query using ISO 639-1 language code: ?hl=en.
Comment 6 Marcel de Rooy 2018-07-30 11:37:12 UTC
(In reply to Katrin Fischer from comment #4)
> This seems to work alright in my devbox on master. Could someone confirm for
> 18.05?

The bug can be reproduced on 16.11, but on master too.
Depends on factors as using https and Plack.
If you use both Plack and https (which you normally should), you should be able to reproduce it.
Comment 7 Marcel de Rooy 2018-07-30 11:39:20 UTC
Created attachment 77323 [details] [review]
Bug 19799: Add localReferer function to module, and test

Before implementing the change in opac-changelanguage, we define a new
function (in Koha/Util.pm) and add tests.

Test plan:
Run t/Koha/Util/localReferer.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2018-07-30 11:39:25 UTC
Created attachment 77324 [details] [review]
Bug 19799: Introduce localReferer in opac-changelanguage.pl

Nice and easy replace..

Test plan:
Test changing language from various OPAC locations. (Verify that you are not
redirected to the home page.)
Include language=[some_language] in the URL before changing language.
[Bonus] Test calling opac-changelanguage.pl from another server.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2018-07-30 11:41:45 UTC
Caused by bug 18946 on 17.11 and backported
Comment 10 Marcel de Rooy 2018-07-30 11:50:22 UTC
This might help you understand the problem:

From C4/Auth:
    # Set the using_https variable for templates
    # FIXME Under Plack the CGI->https method always returns 'OFF'
    my $https = $in->{query}->https();
    my $using_https = ( defined $https and $https ne 'OFF' ) ? 1 : 0;

If you call CGI->url under Plack, you will get http back which is obviously wrong if you used https.
Comment 11 Tomás Cohen Arazi 2018-08-18 13:43:20 UTC
The right way to make things work with our codebase is to uncomment the following line in both apache-shared-intranet-plack.conf and apache-shared-opac-plack.conf.

# RequestHeader set X-FORWARDED-PROTO "https"

This header would be used by Plack::Middleware::ReverseProxy to make $cgi->https work correctly [1]

The problem with that is we didn't manage to automate it for non-tech users. That's an area we could spend time on!

The approach I aimed for having time to implement, was to split like this:

OPACBaseURL => OPACProto (http/https/gopher) + OPACDomainName + OPACBasePath (/cgi-bin/koha, preparing the ground for removing the requirement to use that path). The problem with this is that we of course allow different domains to be used (a feature).

If we were to add code that workarounds the issue on the codebase, I think we should better add it as a Plack middleware as we did in Koha::Middleware::SetEnv.

[1] https://github.com/plack/Plack/wiki/How-to-detect-reverse-proxy-and-SSL-frontend
Comment 12 Marcel de Rooy 2018-08-20 11:39:57 UTC
This report needs a staff counterpart.
Comment 13 Katrin Fischer 2018-08-20 15:17:11 UTC
We tested the solution Tomas suggested and it works well!
Comment 14 Katrin Fischer 2018-08-20 15:45:10 UTC
We've also tested this line, but only with https:

RequestHeader set X_FORWARDED_PROTO "https" env=HTTPS

Maybe this could be a general solution?

https://httpd.apache.org/docs/2.4/mod/mod_headers.html#requestheader
The RequestHeader directive may be followed by an additional argument, which may be any of: 
early Specifies early processing.     
env=[!]varname The directive is applied if and only if the environment variable varname exists.        
A ! in front of varname reverses the test, so the directive applies only if varname is unset.
Comment 15 Ulrich Kleiber 2018-08-20 16:17:55 UTC
Tested 'RequestHeader set X_FORWARDED_PROTO "https" env=HTTPS' with http and https, everthing seemed to be ok.
Comment 16 Tomás Cohen Arazi 2018-08-21 13:17:40 UTC
(In reply to Ulrich Kleiber from comment #15)
> Tested 'RequestHeader set X_FORWARDED_PROTO "https" env=HTTPS' with http and
> https, everthing seemed to be ok.

Uli, the solution requires SSLOptions +StdEnvVars which I'm not sure is enabled by default. Did you have to tweak it yourself? What OS/version are you on?

https://httpd.apache.org/docs/2.4/mod/mod_ssl.html
Comment 17 Ulrich Kleiber 2018-08-21 13:33:39 UTC
Tomás, we are on debian jessie. 'SSLOptions +StdEnvVars' is only set in default-ssl.conf, but this site is not enabled. It looks like it is enabled by default.

/etc$ sudo grep -R StdEnvVars *
apache2/sites-available/default-ssl.conf:		#   o StdEnvVars:
apache2/sites-available/default-ssl.conf:				SSLOptions +StdEnvVars
apache2/sites-available/default-ssl.conf:				SSLOptions +StdEnvVars
Comment 18 Ulrich Kleiber 2018-08-21 15:38:47 UTC
Tomás, I think the mod_rewrite module is responsible for the setting of 'SSLOptions +StdEnvVars'. It is required for the RewriteCond Directive.

https://httpd.apache.org/docs/current/mod/mod_rewrite.html
...These variables are available even without setting the StdEnvVars option of the SSLOptions directive...

Using 'RequestHeader set X_FORWARDED_PROTO "https" env=HTTPS' with Koha shoudln't be a problem.
Comment 19 Tomás Cohen Arazi 2018-08-21 15:49:25 UTC
(In reply to Ulrich Kleiber from comment #18)
> Tomás, I think the mod_rewrite module is responsible for the setting of
> 'SSLOptions +StdEnvVars'. It is required for the RewriteCond Directive.
> 
> https://httpd.apache.org/docs/current/mod/mod_rewrite.html
> ...These variables are available even without setting the StdEnvVars option
> of the SSLOptions directive...
> 
> Using 'RequestHeader set X_FORWARDED_PROTO "https" env=HTTPS' with Koha
> shoudln't be a problem.

Awesome, Uli. I propose we file a new bug for setting what you propose, and we keep this one for Marcel's enhancement which is still a nice improvement.
Comment 20 Marcel de Rooy 2018-08-31 06:43:43 UTC
Fixed by the X-FORWARDED-PROTO approach on bug 21267.
Will resubmit the referer code under a new enhancement.
Comment 21 David Cook 2020-05-04 00:51:35 UTC
So interesting seeing the history of Bug 21267!