[MULTISITE-48] When I modify the site evaluation rules I still get valid page links when added in the ckEditor Created: 24/Jun/15  Updated: 08/Feb/16  Resolved: 02/Feb/16

Status: Closed
Project: Magnolia Multisite Module
Component/s: None
Affects Version/s: 1.0.8, 1.1.3
Fix Version/s: 1.1.4

Type: Story Priority: Neutral
Reporter: Christian Ringele Assignee: Philip Mundt
Resolution: Fixed Votes: 0
Labels: support
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Relates
relates to MAGNOLIA-6243 Inline rich text links don't work on ... Closed
relates to MGNLETK-131 uri-starts-with-sitename rule should ... Closed
relates to MAGNOLIA-6176 Checking to see if prefix might have ... Closed
relates to MULTISITE-56 Inline rich text links don't necessar... Closed
dependency
depends upon MULTISITE-57 Investigate solution for MULTISITE-48 Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MULTISITE-55 Re-evaluate cross site access rules Sub-task Closed Philip Mundt  
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:
Sprint: Basel 29
Story Points: 3

 Description   

The underlying problem of this story has two parts:

  1. When modifying the rules that are used to evaluate which site should be served
  2. actual link generation in ckEditor (possibly in more use-cases) might generate links that cannot be served anymore.

To put it differently: Site evaluation rules are not taken into account for link generation

(One of) The "problematic" rules is uri-starts-with-sitename which will resolve a site when the URI is prefixed with the actual sitename. Due to SEO optimization, it might make sense to move this rule down in the chain (on a side note: content should actually only be accessible through one URI only; this rule enables accessing content of different sites over different domains).

This becomes an issue when adding links in ckEditor which are actually prefixed with this particual site-prefix, now resulting in an invalid link (the rule to "detect" this link was moved).

breun has supplied thorough descriptions, analysis and steps to reproduce in:
SUPPORT-4685: Link creation the rich text editor
SUPPORT-4535: Cross site access

Notes

  • uri-starts-with-sitename rules is mainly there to enable serving all sites when working in an admin instance (where access might indeed happen through one domain).
    • maybe it makes sense to only use this rule in the admin instance
    • and only generate link with this particual prefix on an admin instance too

Also an point to think about is:
In the context of link creation to other workspaces used by apps.
Linking to such items in combination that these workspaces are mapped in "URI2RepositoryMapping" and specifically added to a site definition's mapping.



 Comments   
Comment by Nils Breunese [ 25/Jun/15 ]

Our current solution to our linked multisite issues is a combination of a modified multisite request matcher rule order (putting uri-starts-with-sitename as the last child of /modules/extended-templating-kit/config/rules) and a modified implementation of URI2RepositoryMapping#getHandle (see SUPPORT-4685). We'll be happy to discuss these issues further and explain why we believe our changes improve Magnolia's multisite behavior.

Comment by Rico Jansen [ 25/Jun/15 ]

Another that thing that is in the same terroritory is the generation of public url's on the author instance. Which is something we use to push page information in a centralized search system. Currently that is some custom code built on the domain mappings that knows things about our deployment setup.

Comment by Jan Haderka [ 07/Sep/15 ]

breun rico.jansen how would you guys solve this? Because no matter which way I look at it, there doesn't seem to be solution that would work for all cases. I can understand why you want to shorten links because of SEO, but as you can see changing order of those rules screws up other things. When we originally exposed rules it was to allow you to add there more rules if necessary, but not really to change order (without also taking care of what you break that way).

Can you come up with a patch that would fix the editor irrespective of the order and at the same time pass all the current tests? Otherwise I'm afraid that we will be closing the issue as won't fix and adding big warning to the exposed rules to discourage anyone from changing their order.

Comment by Nils Breunese [ 07/Sep/15 ]

Our primary desire was not to change the order of the rules, but to solve these two issues with the default configuration (see SUPPORT-4685 as well):

  • All pages from al sites being available on all public domains (this is really horrible)
  • Pages having two different valid URI's on a site (that's bad for SEO as well)

Magnolia Support pointed us in the direction of the multisite rules and after studying those we found that changing the order and simplifying URI2RepositoryMapping fixed those issues. Of course we would prefer those issues not to be present out of the box, but it seemed you were hesitant to change anything in this area in the product.

The rich text editor should create UUID-based links, so those should always resolve to the correct URL, but because of a bug (a combination of our changes and assumptions made in the rich editor setup) links were stored as plain URL's instead of UUID-based links without our knowledge.

Comment by Jan Haderka [ 07/Sep/15 ]

All pages from al sites being available on all public domains (this is really horrible)

This is just for backward compatibility and you should be able to switch it off easily by reconfiguring rules or resolvers in crossSite filter. Why is this not possible for you?

Can you attach the changes you have made to the rules and to URI2RepositoryMapping? Do I understand it correctly that with those changes all works for you or do you still have issues w creditor links?

Comment by Nils Breunese [ 07/Sep/15 ]

As we wrote in SUPPORT-4535 we decided to not go for the crossSite solution because that means creating a custom resolver for every site and we have quite a few of them, so we'd rather have one definitive fix. Also we thought it was weird to first have the multisite determine the assigned site wrongly and then later in the filter chain have the crossSite filter make sure that wrongly determined site can't be accessed via that domain, which is already configured in the site definition in the first place, so the multisite filter could have used that information (but this isn't considered by the default order of the multisite rules). We figured that if the multisite filter would just directly correctly determine the assigned site we wouldn't need the crossSite filter as a backup.

The changes we have been running in production for a while now are as follows:

Comment by Jan Haderka [ 21/Sep/15 ]

The fact that you remove the maybeHandle() method is then causing this tickets. So to fix the ckEditor links you would need to reinstate this method.

Comment by Rico Jansen [ 22/Sep/15 ]

I have looked at it quickly and we are currently running with :

  • uri-starts-with-sitename in the last position
  • modified URI2RepositoryMapping without the maybeHandle check.

I just tested this by creating a page link in the ckEditor, and checked via the jcr browser what was put in.
It put in the uuid/handle construct as expected, and not the wrong 'calculated' url.

So the problem that it put in the wrong url is gone.

Putting the maybeHandle function back means we get the problem that sites appear on all others domains.
Solving that through crosssitesecurity is backwards, as that is not it's function, you would expect the multisite filter
to do that properly.

Comment by Jan Haderka [ 22/Sep/15 ]

So the problem that it put in the wrong url is gone.

Are you saying the issue is gone in the latest version?

The whole thing with appending and removing the part of handle was there originally because of old tree. And I totally agree that whole thing is very brittle.

There is concept for rewriting link handling from the scratch that would take care of the problem completely and would not require it to be 100% backward compatible. But we are not ready to make that move yet.

And changing current link handling is considered very dangerous since it can break many things and is very hard to test in all possible scenarios.

Comment by Rico Jansen [ 23/Sep/15 ]

I am not seeing it in our current deployed version, which is 4.5.25.

This is with our two changes.

I agree with that the link handling can be better, especially regarding the interaction with the multisite configuration. The multisite code itself also could use some cleanup.

Since it all ties together so closely it is indeed quite difficult to do, that is one of the reasons that this was turned in to a more general issue.

Testing it is also quite difficult due to all the places it impacts.

Generated at Mon Feb 12 06:05:54 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.