I received the following error while setting up a multilingual website:
Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.
Here is the sequence to create this error:
- Install new 8.0.1 website using English
- Enable Language Translation and Language Modules
- Allow Trusted Hosts: localhost, www.localhost & es.localhost in settings.php
- Add Spanish language
- Set Detection & selection > Domain to www for English and es for Spanish & Save Configuration
I've done this several times in beta.
As a side note. On the "Detection & selection" screen, the user is entering "sub-domains" not "Domains" as the screen says. In this example: Example: Specifying "de.example.com" as language domain for German will result in an URL like "http://de.example.com/contact".
, de is a sub-domain of example.com/. This screen should likely be changed.
Remaining tasks
- Review #46 and determine if a different approach is needed.
Comment | File | Size | Author |
---|---|---|---|
#46 | 2643466-46-interdiff.txt | 885 bytes | Berdir |
#46 | 2643466-46.patch | 4.75 KB | Berdir |
#39 | 2643466-39.patch | 4.75 KB | Matthijs |
#37 | 2643466-37-interdiff.txt | 1004 bytes | Berdir |
#37 | 2643466-37.patch | 4.74 KB | Berdir |
Comments
Comment #2
Greg Sims CreditAttribution: Greg Sims commentedI tried a fresh install this morning with the following in settings.php:
The result was the same.
Comment #3
Greg Sims CreditAttribution: Greg Sims commentedThis could be related to:
Redirects being built as http://localhost:8888/drupal8 and not considered safe by RedirectResponseSubscriber
but I am not sure.
Comment #4
Gábor HojtsyRe your side note, you personally may be entering subdomains, but it is equally possible to enter example.es, example.de, etc. or even beispiel.de and ejemplo.es, there is no requirement for them being subdomains.
Comment #5
Gábor HojtsySaving the config does the redirect to the new domain. However in other cases, there should not be domain redirect, should there? I mean once you saved the configuration, do you have issues later on using the different domains?
Comment #6
Greg Sims CreditAttribution: Greg Sims commented@Gábor Hojtsy
I tried the sequence in #1 above with 8.0.2 -- same result.
I then went to "Content" which seemed to go OK. I tried to create an Article and received: "The website encountered an unexpected error. Please try again later."
I then went back to: http://www.localhost/admin/config/regional/language and received: "The website encountered an unexpected error. Please try again later."
I conclude that the website is unusable at this point and agree with Gábor assessment of "Major" Priority. This issue would be "Critical" for anyone that needs to use domains for languages now.
Comment #7
swentel CreditAttribution: swentel commentedSo there are two problems:
- trusted url redirect (without or with trusted urls configured)
- WSOD pages for pages you don't have access to on the domain.
So at first, when configuring, you get the url trusted redirect response. Now, authenticated on my 'base' domain, I refresh the page, all links are rewritten to go to en.drupal8 which is fine, however, you get the WSOD when you then click any link or manually go to a URL for which you need a permission. (The website encountered an unexpected error. Please try again later.)
You can find 3 entries in the logs (note, the difference between 2 and 3 is very subtle: 'system/403' vs 'system/404')
1:
type: access denied
location: http://en.drupal8/admin/config/regional/language
referrer: http://drupal8/admin/config/regional/language/detection/url
message: /admin/config/regional/language
2.
type: page not found
location: http://en.drupal8/http://en.drupal8/system/403?_exception_statuscode=403...
referrer: http://drupal8/admin/config/regional/language/detection/url
message: /http://en.drupal8/system/403?destination=http%3A%2F%2Fen.drupal8%2Fadmin...
3.
type: page not found
location: http://en.drupal8/http://en.drupal8/system/403?_exception_statuscode=404...
referrer: http://drupal8/admin/config/regional/language/detection/url
message: /http://en.drupal8/system/404?_exception_statuscode=403&destination=http%...
If you are authenticated on all domains (and with the same permissions of course), all pages and submissions are fine as along as you have access.
Haven't looked deeper, but the problem seems to be a combination in building/creating the 403/404 page and redirecting of some sort.
Comment #8
swentel CreditAttribution: swentel commentedSo this fixes the redirect, but then I get into trouble with not being authenticated on that domain and not having access to the URL configure form.
Comment #9
Leksat CreditAttribution: Leksat at Amazee Labs commentedProbably this can be fixed if we extend
\Drupal\Core\Routing\LocalAwareRedirectResponseTrait::isLocal()
so it also checks language domains.UPD: or maybe it should check
$settings[trusted_host_patterns]
?I'm not sure what is better.
Comment #10
Leksat CreditAttribution: Leksat at Amazee Labs commentedThis patch fixes the issue by adding new
hook_local_domains()
hook.I could not think out a better solution... I'm not even sure if
\Drupal::moduleHandler()
is always available in\Drupal\Core\Routing\LocalAwareRedirectResponseTrait::isLocal()
. Let's see what tests will say.Comment #11
Leksat CreditAttribution: Leksat at Amazee Labs commentedOMG, it works! :D
Comment #14
brianV CreditAttribution: brianV as a volunteer commentedI re-tested this patch, and it's no longer failing:
https://www.drupal.org/pift-ci-job/351901
Moving back to needs review. I'd love to see it in core.
Comment #16
drupalninja99 CreditAttribution: drupalninja99 at Mediacurrent commentedAny updates on this issue?
Comment #17
Fidelix CreditAttribution: Fidelix as a volunteer commentedThe patch in #10 solved the problem for us.
I'd like to see better documentation and coding standards on the code though.
Thanks for the excellent work!
Comment #19
pawel_r CreditAttribution: pawel_r commented#10 worked for me perfectly
Comment #20
dawehnerWe should add some test coverage if possible :)
Comment #21
pixeliteThe patch in #10 applies to Drupal 8.4.x-dev, and fixes the issue documented in #6, so I'm marking as 'Triaged for D8 major current state'.
Comment #22
alexpottDiscussed with @catch, @cilefen, @cottser, @laurii, and @xjm. We agreed that this is a major issue - there is no workaround and you get an unexpected TrustedRedirectResponse Error whilst using the core UI.
The patch needs tests and documentation (ie hook documentation) and a change record.
One thought is that maybe we should just produce a warning a tell people to update their $settings[trusted_host_patterns] in settings.php instead of a new hook? And use it in the trait.
Comment #24
hsponner CreditAttribution: hsponner commentedThanks, #10 patch helped me on 8.4.0 website
Comment #26
markus_petrux CreditAttribution: markus_petrux commentedHi all! ...patch works and fixes the issue... tested on 8.4.5.
Attached is a patch for 8.4.5, it's the same as the one in #10, but offsets fixed.
Comment #27
5n00py CreditAttribution: 5n00py commentedThanks for working patch, it fix my problem.
Not sure what is better, new hook or trusted hosts patterns.
Comment #29
mirsoft CreditAttribution: mirsoft commentedThanks, I confirm the fix #26 worked for me and fixed an annoying issue we faced a long time (and did not know the cause).
For us it appeared after adding a menu link from the language-specific subdomain (e.g. de.example.com), pointing for example to <front> and attempting to save that link. (surprisingly enough, editing that link afterwards worked without an error)
It would be great if someone who has bandwidth could implement the rest of tasks (documentation and tests) so it can proceed further.
To the question whether or not to use trusted host patterns instead - I'm not sure I'm here the right one to tell my opinion but if we treat domains like de.example.com as local domains, then I think the implementation is correct as local domains are not being added to trusted hosts.
Comment #30
keats76 CreditAttribution: keats76 commentedThis was still a problem for us on Drupal 8.7.6.
Our scenario:
1) We use custom sub-domains for our languages (e.g. fr.example.com, de.example.com with www.example.com as our source)
2) Add a translation for a piece of content (example: German). Save the translation.
3) Attempt to delete the translation. The deletion fails with the message: "Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it."
4) Refresh the original page and note that the translation has now been removed.
5) It would appear that this was an annoyance, but if you try to add a new German translation for this content, the system will error because Drupal thinks that some German content remains. I think this is an artifact of our use of the Paragraphs module.
In any case, I can confirm that applying this patch against Drupal 8.7.6 core worked and we have incorporated it into our composer flow.
Considering how old this is, I would suggest adding it to core.
Comment #31
mbovan CreditAttribution: mbovan at MD Systems GmbH commentedI assume the target should be 8.9.x branch here. Rerolled #10 and added documentation for
hook_local_domains()
.Comment #32
BarisW CreditAttribution: BarisW at LimoenGroen commentedAnother thing, not sure if its related.. I have a website using 2 language domains (domain.com and domain.nl).
When I go to the overview of menu links of my main menu in the English interface (/domain.com/admin/structure/menu/manage/main-navigation) I see that the Edit links (the action dropdown) next to each menu link point to /domain.nl/menu/item/123/edit (instead of .com).
Also, when I create a menu link using the .com interface, I get the "Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it." error after submitting the form.
This only happens with domain detection, not with path prefix.
Comment #36
Berdir> One thought is that maybe we should just produce a warning a tell people to update their $settings[trusted_host_patterns] in settings.php instead of a new hook? And use it in the trait.
Interesting idea. Attaching a patch that does that. Seems to work well in unit tests, didn't do any real tests with it yet.
Would be nice to finally get rid of this, this is the one of the oldest patch in our project.
No interdiff as it is a completely new approach.
Comment #37
BerdirAdjusting the error message a bit. How we do that seems a bit weird to me, the trigger error and the assumption that it is a "client error". Would be nice to be able to actually put the domain in the message or log it. But I hope that's out of scope ;)
Comment #39
MatthijsUpdated the patch from #37, I added the missing delimiters to the preg_match in isLocal().
Comment #42
xjmComment #44
stefank CreditAttribution: stefank commentedHi all,
Tested on 9.4.5 and patch in #31 still applies cleanly and works as expected, prevent the error message "Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it". Also, I have tested using the new approach from #37, but still getting the same issue.
Tested on site which is multilingual, and the detection is done using url (Domain url). When adding for example a menu item and the domain url is set for example in DE and the select list field for language is set for example in FR, then Im getting the error message. The menu is added, but the screen just displays the error, which is confusing for the end user.
Trusted hosts patterns in setting.php is set as $settings['trusted_host_patterns'][] = '.*';
Comment #46
BerdirFixing the new tests.
#44: Did you use the patch from #37 or or #39? #39 should work with .* I tested that with the unit test.
That said, this shows a problem with this approach. Maybe you just did that local or for testing, but you should _not_ use that, because it completely subverts the protection that this is supposed to provided, as any URL is then trusted and will be redirected to.
I don't think this needs a change record, it's a bug and it's now fixed. Only thing would be a reminder that you should property define the trusted host patterns :)
Comment #47
smustgrave CreditAttribution: smustgrave at Mobomo commentedRemoving CR tag per #46
Verified the test fails without the fix
Data set #8 Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.
Verified following the steps in the IS
Patch #46 seems to solve it.
Comment #48
BerdirThanks as well.
I am a bit concerned about what I said in #46. This means that not having the trusted host patterns configured properly (which gives you a warning/error on status page but works just fine) now also completely bypasses the outgoing projection. We might need a security review here.
The implementation is based on #22, two options I can think of is explicitly check for the full/default wildcard and then disallow this, or going back to an earlier implementation with its own hook/extension point.
Comment #53
xjmAdding credits for the triage discussion mentioned in #22 (note: Cottser has changed his username since the comment was posted.)
Comment #54
xjm#48 also sounds like this is not actually RTBC. I added #46 to the remaining tasks in the IS.
Comment #55
BerdirYes, I'm more and more convinced that this is a bad idea. See for example the template for platform.sh projects: https://github.com/platformsh-templates/drupal10/blob/master/web/sites/d.... That will completely disable the protection for redirects to external hosts.
Comment #56
alexpottJust wondering what was actually wrong with the approach in #8? Ah #32 points out the redirect can happen in other places too.
I agree with @Berdir - the security implications of falling back on the trusted hosts setting are large when you consider the default config of platform.sh. Also re-using trusted hosts here repurposes/extends its meaning which, on reflection, doesn't feel like a good idea given its role in securing your site and the role of LocalRedirectResponse.
So this brings us back to #31 as the best (or maybe least worst) solution we have.
Comment #57
joseph.olstad#31 applies cleanly to D10.0.11.
Comment #58
johnzzonI encountered a similar issue, but when deleting a translation of a node. I've detailed reproduction steps in another issue before I found this. Adding that issue as related.
https://www.drupal.org/project/drupal/issues/3255337
EDIT: I see now that #30 has this scenario documented already.