Change redirect after login form submit

[petrakohorst]

Use uri_for on redirect URI only if no params are attached.

(This problem only happens when submitting the login form! Prior redirects to shibboleth instances for example are exempt from this, because they don't submit the login form.)

When redirecting to a URL that has parameters attached, the sub uri_for does not recognize these parameters as such. The sub expects parameters to be given separately from the base URI.
Instead, in this case, the full URL is used as $path (expecting a clean URL path). This results in undecoded questionmarks in the actual redirect URI and the user lands on a 404 page.

Example:
Requested URL is http://localhost:5001/librecat/search/admin?sort=author.asc
User gets to the login form, logs in.

The redirect, however, will lead to: http://localhost:5001/librecat/search/admin%3Fsort=author.asc

[coveralls]

Coverage remained the same at 81.388% when pulling 7449365 on pr/fix_redirect_after_login into e46ee58 on dev.

[nicolasfranck]

Why is there even a call to uri_for when a full url is expected?
I would rather suggest to change the previous line from

my $return_url = params->{return_url} || '/librecat';

to

my $return_url = params->{return_url} || uri_for('/librecat');

[nicolasfranck]

mm, you're right: this route expects a local path in $return_url,
and that is why "external" links are filtered out.

[nicolasfranck]

I've look through the code, and it seems that return_url should be a full url, and just a path,
like /librecat on this line suggests.

But a check against external links cannot be done like this as it strips too much.
What if the application is served behind a proxy, and so has a prefix?

Proposal:

• default url should be full url

- my $return_url = params->{return_url} || '/librecat';
+ my $return_url = params->{return_url} || uri_for('/librecat')->as_string;

• return_url should be replaced by that default url when not starting with the full prefix uri_base (e.g. https://myhost/prefix)

- $return_url =~ s{^[a-zA-Z:]+(\/\/)[^\/]+}{};
+ $return_url = uri_for("/librecat")->as_string if index( $return_url, request->uri_base() ) != 0;

and then just redirect to that url

[nicolasfranck]

@petrakohorst I've made some changes. Can you confirm if it works?

[petrakohorst]

@nicolasfranck Yes, it works perfectly with your changes!