search

silverstripe / silverstripe-subsites

Subsites module for Silverstripe CMS
http://addons.silverstripe.org/add-ons/silverstripe/subsites
BSD 3-Clause "New" or "Revised" License
65 stars 106 forks source link

TreeDropdownField in SiteConfig CMS fields can cause unexpected subsite switching #477

Open samthejarvis opened 2 years ago

samthejarvis commented 2 years ago

When loading child pages in a TreeDropdownField with an ID that matches any SiteConfig ID associated with a subsite, the user is automatically switched back to this subsite when loading other subsite's /admin/settings.

Example: Load the child pages for a page with ID 8 via a TreeDropdownField named PageID.

This fetches: https://localhost/admin/settings/EditForm/field/PageID/tree?**ID=8**&format=json In this request the session variable SilverStripe\SiteConfig\SiteConfigLeftAndMain is set to {currentPage: 8} (not expected).

From now on, the following code runs on all actions under /admin/settings: https://github.com/silverstripe/silverstripe-subsites/blob/acf9715c3b8165ad567eaec8f85dfb21d9a364ce/src/Extensions/LeftAndMainSubsites.php#L314-L339

In this case, $record is now the Subsite that relates to SiteConfig#8. It's ID is 3 in this case.

As the currentPage is the ID of an existing SiteConfig and matches that of a subsite, the current subsite ID is set to the subsite for SiteConfig#8 via a redirect to ?SubsiteID=3.

From now on, accessing /admin/settings on other subsites automatically switches you back to this subsite (if it includes any components that generate requests like TreeDropdownField's tree action GridFieldFilterHeader SearchForm action etc.). Effectively the user can no longer access /admin/settings/ on other subsites.

Clearing the erroneous SiteConfigLeftAndMain entry from the session stops this behaviour.

While the impact is on subsites functionality, the cause is likely in core (to confirm) (the SiteConfigLeftAndMain currentPage session variable should not be set by the above request).

samthejarvis commented 2 years ago

Root of the issue seems to be that LeftAndMain::currentPageID() assumes any ID request var is the current page ID. This means that for the the TreeDropdownField tree request, LeftAndMain::currentPage() returns a SiteConfig of the ID of the current page requested in the TreeDropdownField. This allows the snippet above to run which switches the current subsite.

A quick fix is to just add an is_int($this->owner->urlParams['ID']) to the condition in the snippet above, as $this->owner->urlParams['ID'] is equal to 'field' in the above request.