hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.74k stars 429 forks source link

Force a redirect of the entire window #203

Open agrobbin opened 3 years ago

agrobbin commented 3 years ago

We use an external Auth provider, and if someone gets signed out of our app, subsequent fetch requests result in CORS violations, because Turbo (Drive and Frames) try to fetch the location from the "you're now signed out, please sign in again" redirect. This applies both to GET requests and to form submissions.

Similar to the way Turbo Drive lets you trigger a whole page refresh via the turbo-visit-control meta tag, I'm wondering if there can be a way to tell Turbo to handle redirects via window.location.href = response.location, perhaps via a Turbo-Redirect: window response header.

agrobbin commented 3 years ago

Now that I have read more of the docs, I'm actually wondering if this is a bug:

By default, Turbo Drive only loads URLs with the same origin—i.e. the same protocol, domain name, and port—as the current document. A visit to any other URL falls back to a full page load.

This applies to links clicked, but doesn't seem to be applying to redirects coming from visit requests.

dillonhafer commented 2 years ago

Yeah, I face the same issue, this looks like a bug to me too.

Here's a quick example of the bug:

# new.html.erb
<%= button_to "Create", [:users] %>
class UsersController
  def create
    redirect_to "https://github.com", allow_other_host: true
  end
end

This will result in a CORS error, because turbo tries to load github.com with a fetch request

dhh commented 2 years ago

Turbo should detect this and do a full page redirect. If anyone wants to look into that, please go ahead!

dillonhafer commented 2 years ago

That's good know that it shouldn't be trying to fetch another host. I'll try to dig into more.

dillonhafer commented 2 years ago

I think this is an issue with the fetch API itself. Fetch does not give you the ability to inspect the Location header on a redirect, so there is no way to prevent a CORS request from happening if you wanted to visit another host. I can work around this in Rails using the turbo event listeners and a custom status code. The custom status code prevents the fetch api from trying to handle the redirect, allowing my turbo event listener to inspect the Location header to determine what to do:


document.addEventListener("turbo:before-fetch-response", function (e) {
  const locationHeader = e.detail.fetchResponse.response.headers.get("Location") || "";
  if (locationHeader.length > 0 && !locationHeader.startsWith(window.location.origin)) {
    e.preventDefault();
    window.Turbo.visit(locationHeader);
  }
});

And the rails response (I left the custom status code in the 3xx range to attempt a semantic meaning):

class BillingController
  def create
    sessions = StripeAdapter.create_billing_session(current_user)
    redirect_to session.url, allow_other_host: true, status: 309
  end
end
dhh commented 2 years ago

Hmm, yeah, that's not easy. Could consider something like a custom parameter that could trigger the full redirect and then be stripped? Like ?_turbo_full_redirect=true.

bilogic commented 2 years ago

Looking for a solution to this too, I'm using data-turbo="false" for now.

iamFIREcracker commented 1 year ago

I think this is an issue with the fetch API itself. Fetch does not give you the ability to inspect the Location header on a redirect, so there is no way to prevent a CORS request from happening if you wanted to visit another host. I can work around this in Rails using the turbo event listeners and a custom status code. The custom status code prevents the fetch api from trying to handle the redirect, allowing my turbo event listener to inspect the Location header to determine what to do:

I implemented the same workaround but I have noticed it's not working with Safari, i.e. the browser is following the redirect irrespective of the status code used (309).

devaniljr commented 10 months ago

Has anyone found a solution to this bug? The suggested workaround (disabling turbo) doesn't work in my case because I'm using Turbo in case of failure.

ajgarlag commented 10 months ago

We use a workaround in a Symfony project with a custom turbo stream action.

We have a listener that checks if the request is made with turbo and the response is an external redirect.

In that case, we override the given response with a new one with the status code set to 300, removing the Location header and rendering the following turbo-stream:

<turbo-stream
    action="redirect"
    data-location="{{ location }}"
    {% if timeout is defined %} data-timeout="{{ timeout }}" {% endif %}
></turbo-stream>

The custom stream action is defined in a controller connected to the page body.

import { Controller } from '@hotwired/stimulus';
import { StreamActions, visit } from "@hotwired/turbo";
export default class extends Controller
{
    connect()
    {
        StreamActions.redirect = function () {
            setTimeout(() => visit(this.getAttribute('data-location')) , parseInt(this.getAttribute('data-timeout') ?? 0));
        }
    }
}
Crovitche-1623 commented 1 week ago

Hi @ajgarlag, I'd also be interested, as I'm also using Hotwired Turbo within a Symfony project. However, I didn't really understand your workaround. The HTTP code 300 is Multiple Choices; what does this have to do with a redirect? If I understand well, your workaround force the redirect as expected ?

Would you be willing to share your listener here?

ajgarlag commented 1 week ago

@Crovitche-1623 I chose the 300 HTTP status code because I wanted to represent the redirection intent, but I didn't want the browser to follow the redirection automatically.

The listener is available at https://gist.github.com/ajgarlag/69e3586a754728ff52e4470a61382eec#file-redirectresponsetoturbostreamlistener-php

ajgarlag commented 1 week ago

@Crovitche-1623 Please, send me your feedback about the listener in the gist comments to avoid spamming this issue.