Scope of Domain Source module

There is a lot to unpack here. Let's start with a description of what the module is designed to do.

Core features:

* Allows nodes to be assigned to a 'canonical' domain.
* This canonical domain is used to rewrite all links to defined node paths, as declared by the Node entity type.
* Ensures that form submissions for those nodes are routed to the proper canonical URL.

Techniques:

* The module uses a standard entity reference field for the assignment of the canonical domain.
* The DomainSourcePathProcessor service extends core's OutboundPathProcessor service to rewrite links.
* Links are rewritten using the same technique as language-based domain handling, by setting the 'base_url' property of the url options.

Assumptions (which may not be true):

* All paths registered to a Node should point to the canonical URL.
* External URL handling is handled by core.
* JS should be loaded from the active domain, which may require setting a global 'url.site' cache context.

This issue is problematic, because you're dealing with about 4 separate issues and behaviors.

1. The behavior you describe is the desired behavior. Domain Source should force links to the canonical URL.

If we want to change this behavior, we would need to make the routes in DomainSourcePathProcessor::ProcessOutbound() configurable. (Or you would write a custom module to replace that service.)

2. The module doesn't handle comments at all. Ideally, a node should never be opened on it's non-canonical domain.

I don't really understand this error, because we only issue redirects on form submit, and those redirects (in domain_source_form_submit()) do use TrustedRedirect Response.

Ideally, we need to add the Comment URLs to the list of rewrites and we need tests for this behavior.

3. This problem is a JS cache issue that is entirely outside our control. Perhaps adding the 'url.site' cache context will fix it. This also sounds like you are using Inline Entity Form (or similar), which would require collaboration with that module to fix.

Again, however, you should be editing on the canonical domain. It may also be a problem caused by the fact that your hostnames don't share a common route (e.g. example.com). Using *.local:8888 for all domains may solve this issue.

4. I think your requirements run counter to the module design. I particularly disagree with item 2. That is the default behavior without using Domain Source. I also disagree with item 3. By design, the canonical URL should _always_ be used.

Why fight against the use of canonical URLs for all links?

As to the README, contributions are welcome. I simply haven't had time.

I think the SEO optimization you're talking about is more like what Global Redirect does: issue a server-level redirect rather than write the links.

It shouldn't be too hard to write a module that does that, using similar principles to Domain Source.

As to that core JS issue -- does having common base hostname (e.g. example.com, one.example.com, two.example.com) fix it?

I wonder if we can fix the JS issue by only changing the VIew link to canonical.

We could also make it configurable, so that only certain routes get rewritten.

Confirned the comment / reply bug.

Confirmed the AJAX error.

This quick fix corrects the AJAX issue by making the path rewrite only match the canonical route.

Here's a proof of concept patch that incorporates @vilepickle's work on #2894904: \Drupal\Core\Routing\TrustedRedirectResponse error is thrown excessively, especially with multilingual enabled to correct the Comment bug.

That new RedirectResponse subscriber should be moved to Domain Source, since Domain itself doesn't issue redirects except in the case of inactive domains.

I don't thi8nk we have to rewrite the entire RedirectSubscriber.

It looks like the key component is in the URLHelper::externalIsLocal() method. If we can extend or override that, we should be fine and with less code.

OK. Here's a big patch for review that covers both issues.

What I ended up doing is swapping in our own Redirect handler when Domain Source is in use. This in turns calls a new DomainRedirectResponse class that overrides two core Drupal classes that are hardcoded.

It seems an elegant solution to the issue.

This still needs tests, and I need to know if it actually solves the reported issues.

This patch has gotten large, but seems to fix the following issues:

* Redirects when translations are enabled. #2894691: Domain source does not correctly rewrite URLs created with the UrlGenerator core class
* Redirects from comments to canonical domain (original report)
* AJAX errors from editing forms and cross-domain requests (original report)

These still need tests, but the current behavior is as follows:

* Domain Source will only rewrite the canonical link for nodes (/node/1) and not sub-links (/node/1/edit and so on).
** This change fixes the AJAX issue on editing forms.
* Domain Source now validates Redirects against registered domains and aliases, replacing the core behavior of internal path checking.
** This change _might_ open a security hole, since open redirects would be allows by any alias matching path. We might need to restrict matches by environment to prevent exposure in cases where someone registers an alias like *.com.
* Language-based source differences are now respected.

With the CORS solution in core, I don't know that we need to fix the AJAX issue via brute force. https://www.drupal.org/node/2715637

We could certainly make the link rewrites configurable.

I'd also like to offer a "security mode" which restricts redirects / rewrites to domains in the $trusted_host_settings.

This has been corrected and tests provided. See also the new README that comes with Domain Source -- https://github.com/agentrickard/domain/tree/8.x-1.x/domain_source

See https://github.com/agentrickard/domain/pull/361

Note that you may need to disable link rewriting for the 'edit-form' route, which can be done in the new configuration. See the documentation.

If you are not submitting an edit form, Domain would only be issuing a redirect if you are trying to access an inactive domain without permission.

That code hasn't really changed -- it's in the DomainSubscriber. The only thing that code is doing that's new is ensuring that the redirect is to a trusted host.

There is nothing else in the code that would issue a redirect on a node view page. The only other change affects how Redirects are handled in cases where nodes are being saved that have a different canonical URL.

Now, if your custom module is issuing a redirect, it might be getting routed through DomainSourceRedirectResponseSubscriber. If that were redirecting you to the default domain, I would also expect it to return an error.

See also what we're testing for in the following:

* Functional\DomainSourceLanguageTest
* Functional\DomainSourceTrustedHostTest
* Functional\DomainSourceUrlTest
* DomainInactiveTest

You should be able to come up with a test case that duplicates the issue.

For reference -- this patch went in -- https://www.drupal.org/files/issues/2894904_domain-external-redirect-err... -- but was modified because LocalRedirectResponse wasn't working properly and needed additional context.

This patch did not -- https://www.drupal.org/files/issues/domain_source-url_generator-2894691_... -- because the test cases that I have (see the list above) don't show a need for it.

I've been hanging out in Drupal Slack @agentrickard, too. But will likely be offline for a few days.

[Wrong issue]

It is also possible that you're seeing behavior change based on #2896434: Multiple hooks no longer work with domain_config in some cases. There were documented cases where hook_domain_request_alter() was not firing when using modules that implement HTTPMiddleware (e.g. Shield module).

In that case, it could be that multiple modules are now issuing alias-based redirects, which is the other case in which the resolver might issue a redirect.

OK. If you run across anything actionable, please open a new ticket.