Allow hosts redirects from hosts Rails configuration

[Kevinrob]

Summary

We use Rails with multiple domains (and subdomains). All these domains are configured with config.hosts.
Sometimes we have dynamic redirect from one domain to an other.
And ActionController::Redirecting::UnsafeRedirectError is raised.

It would be great to allow redirections to domains that the app accept in config.hosts.
This should not be considered as unsafe.

Other Information

[p8]

As this accessor is a collection of hosts, what do you think about renaming the accessor?

Suggested change

	mattr_accessor :redirect_hosts_allowed, default: []
	mattr_accessor :allowed_redirect_hosts, default: []

[Kevinrob]

Yes, it's better! I changed that 👍🏻

[Kevinrob]

@p8 Is there anything else I can do about this PR?

[fiestacasey]

I have a similar setup and in particular one domain is in charge of authentication and redirecting to the other domain after auth. This is further complicated because the redirect is within Devise and allow_other_host isn't controllable without monkey patching either Rails or Devise methods. I'm sure this is the case within a lot of different gems that call redirect_to internally so it would be great to be able to define allowed_redirect_hosts in this way.

[zzak]

I think providing an example in the changelog would be nice

[Kevinrob]

What example would you like to view here?
This PR only fixes the raise of UnsafeRedirectError when redirecting to an allowed hosts from config.hosts.

[zzak]

This looks like an anti-pattern to me, like the list of allowed hosts should be known before we get here or evaluated lazily. Might just be me though 🤔

[tom-lord]

Any update on this feature?

We've run into the same issue, where we frequently want to redirect between two subdomains. Currently the only option is to disable this security feature and verify manually that the URL is safe.

Additionally, what's odd is that the url_from helper automatically considers other subdomains "safe" meanwhile UnsafeRedirectError will still be raised when redirecting to another subdomain.

[owst]

Hi @Kevinrob, thanks for getting started with this PR, would you be able to get it across the line? There are a couple of sensible comments from @zzak and a build failure - hopefully they are both straightforward to address?

We'd like to see this added to Rails as we are also hitting an unsafe redirect during our authentication flow in the same way as apokalipto/devise_saml_authenticatable#237 - adding our identity provider's hostname to allowed_redirect_hosts would be a preferred fix. There is some discussion (but no conclusion) on a Devise issue (heartcombo/devise#5462) and it seems quite a few people are similarly affected, with a handful of alternative approaches/monkey patches being proposed - it would be great if there was an official Rails fix.

[Kevinrob]

Hi @Kevinrob, thanks for getting started with this PR, would you be able to get it across the line? There are a couple of sensible comments from @zzak and a build failure - hopefully they are both straightforward to address?

We'd like to see this added to Rails as we are also hitting an unsafe redirect during our authentication flow in the same way as apokalipto/devise_saml_authenticatable#237 - adding our identity provider's hostname to allowed_redirect_hosts would be a preferred fix. There is some discussion (but no conclusion) on a Devise issue (heartcombo/devise#5462) and it seems quite a few people are similarly affected, with a handful of alternative approaches/monkey patches being proposed - it would be great if there was an official Rails fix.

Hi @owst, I just rebased the PR against main and the tests is 🟢 (except one strange fail that I thing is not related).
I don't know how to bring more visibility to this PR.