Add return_to behaviour to 404 -> login flow

[ukutaht]

Changes

It's quite annoying at the moment to:

1. Receive a link to a dashboard (maybe with filters)
2. Get 404
3. Go to login
4. Enter credentials
5. Be redirected to /sites instead of the original link

This PR implements a return_to query param used in login flow, including when 2fa challenge is presented. This way the user lands on the intended page.

We currently have a session-based login_dest variable but it only works for known routes which dashboards are not. It could have been extended for this use-case but I felt like using query params is better when session state isn't strictly necessary.

[github-actions]

Preview environment👷🏼‍♀️🏗️
PR-4916

[zoldar]

Looks good - left some minor suggestions. Before the final 🦭 I'll take it for a short spin on preview app though.

[apata]

@ukutaht I'm a bit concerned that as-is this can be used to craft malicious links.
https://pr-4916.review.plausible.io/login?return_to=https:%2F%2Fexample.com%2Fhack%2Fplausible.io
If on the return URL is a copy of the Plausible login page, the user may naively enter their details again, inadvertently sharing their details with a hacker.

[zoldar]

@apata good point. The return_to URL could be sanitized before final redirect, scraping any with non-matching host part.

[ukutaht]

I'm a bit concerned that as-is this can be used to craft malicious links.

Thanks, that's a good point! I've taken a stab at it here. A few points about this commit:

• It changes the Phoenix.Controller.redirect function call to use to: argument over external: which only allows local redirects
• When an external URL is provided to redirect it will crash with a 500. Should be rare but still bad UX that I wanted to avoid. So I added a small validation our side as well which will ignore return_to unless it starts with a "/"
• Note that it's still possible to craft a malicious URL that starts with a double slash "//", and in that case we will 500 but I don't expect this to happen often if at all
• @zoldar is there any reason we've been using external over `to? I'm not sure why we would need to redirect to an external URL here...

[zoldar]

@ukutaht

Ad 5. this was originally a workaround for issue with internal redirects failing when there's a url encoded UTF8 character in the path (the domain of the site, in particular): #3560 - phoenix was validating the "to" URLs a bit overzealously. The phoenix version we currently are on (1.7.14) has this behavior changed via phoenixframework/phoenix@4ca6ca2. This should no longer be an issue and we should be able to get back to using internal "to" redirects.

[apata]

I'm happy with that, @ukutaht! But because this feature allows redirecting to a page that has itself url search params (https://pr-4916.review.plausible.io/login?return_to=/sites%3Ffoo%3Dbar), any other open redirect vulnerability that we might have would still be exploitable. Before we release it, we should double check that we don't have any.

Edit: From my side, I couldn't find any other redirects that take user input.