From 82d33fc20b359cec5661442691c61a5553cd8b30 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Wed, 22 Dec 2021 23:45:07 +0100
Subject: [PATCH] [BUGFIX] Resolve shortcuts in HMENU to access restricted
pages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
There is a difference between
> typolink.linkAccessRestrictedPages = 1 (only a toggle)
and
> HMENU.showAccessRestrictedPages = [pageId]|NONE
HMENU.showAccessRestrictedPages behaves like
the global option "config.typolinkLinkAccessRestrictedPages"
Basically, if a page is access restricted, link
to a different {pageId}.
This change explains the behavior:
-- https://review.typo3.org/c/Packages/TYPO3.CMS/+/35908/
"typolink.linkAccessRestrictedPages" in contrast
still links to the actual disallowed page, which
is also nice in case you have a 403 error page in place.
This change fixes the behaviour of HMENU to behave
EXACTLY like the global config.typolinkLinkAccessRestrictedPages,
previously HMENU did some magic PLUS it set
"typolink.linkAccessRestrictedPages"
With this change, shortcuts to access restricted pages
now get properly transformed and linked for menus.
Resolves: #60258
Resolves: #65118
Related: #63804
Releases: main, 11.5
Change-Id: Ifd975243fe4b024b3fcbd4e356430d809cc0f429
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/72796
Tested-by: core-ci <typo3@b13.com>
Tested-by: Jochen <rothjochen@gmail.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Jochen <rothjochen@gmail.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../Menu/AbstractMenuContentObject.php | 49 ++++------
.../Classes/Typolink/PageLinkBuilder.php | 12 ++-
.../SiteHandling/Fixtures/SlugScenario.yaml | 3 +
.../SiteHandling/SlugLinkGeneratorTest.php | 90 +++++++++++++++++++
.../Menu/AbstractMenuContentObjectTest.php | 11 ---
5 files changed, 120 insertions(+), 45 deletions(-)
diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
index 07cbca8e66b5..d28dc8276323 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
@@ -1201,7 +1201,7 @@ abstract class AbstractMenuContentObject
$attrs = [];
$runtimeCache = $this->getRuntimeCache();
$MP_var = $this->getMPvar($key);
- $cacheId = 'menu-generated-links-' . md5($key . $altTarget . $typeOverride . $MP_var . json_encode($this->menuArr[$key]));
+ $cacheId = 'menu-generated-links-' . md5($key . $altTarget . $typeOverride . $MP_var . ((string)($this->mconf['showAccessRestrictedPages'] ?? '_')) . json_encode($this->menuArr[$key]));
$runtimeCachedLink = $runtimeCache->get($cacheId);
if ($runtimeCachedLink !== false) {
return $runtimeCachedLink;
@@ -1209,6 +1209,15 @@ abstract class AbstractMenuContentObject
$tsfe = $this->getTypoScriptFrontendController();
+ $SAVED_link_to_restricted_pages = '';
+ $SAVED_link_to_restricted_pages_additional_params = '';
+ // links to a specific page
+ if ($this->mconf['showAccessRestrictedPages'] ?? false) {
+ $SAVED_link_to_restricted_pages = $tsfe->config['config']['typolinkLinkAccessRestrictedPages'] ?? false;
+ $SAVED_link_to_restricted_pages_additional_params = $tsfe->config['config']['typolinkLinkAccessRestrictedPages_addParams'] ?? null;
+ $tsfe->config['config']['typolinkLinkAccessRestrictedPages'] = $this->mconf['showAccessRestrictedPages'];
+ $tsfe->config['config']['typolinkLinkAccessRestrictedPages_addParams'] = $this->mconf['showAccessRestrictedPages.']['addParams'] ?? '';
+ }
// If a user script returned the value overrideId in the menu array we use that as page id
if (($this->mconf['overrideId'] ?? false) || ($this->menuArr[$key]['overrideId'] ?? false)) {
$overrideId = (int)($this->mconf['overrideId'] ?: $this->menuArr[$key]['overrideId']);
@@ -1242,8 +1251,6 @@ abstract class AbstractMenuContentObject
$LD['target'] = $this->menuArr[$key]['target'];
}
- // Manipulation in case of access restricted pages:
- $this->changeLinksForAccessRestrictedPages($LD, $this->menuArr[$key], $mainTarget, $typeOverride);
// Overriding URL / Target if set to do so:
if ($this->menuArr[$key]['_OVERRIDE_HREF'] ?? false) {
$LD['totalURL'] = $this->menuArr[$key]['_OVERRIDE_HREF'];
@@ -1257,6 +1264,13 @@ abstract class AbstractMenuContentObject
$attrs['HREF'] = (string)$LD['totalURL'] !== '' ? $LD['totalURL'] : $tsfe->baseUrl;
$attrs['TARGET'] = $LD['target'] ?? '';
$runtimeCache->set($cacheId, $attrs);
+
+ // End showAccessRestrictedPages
+ if ($this->mconf['showAccessRestrictedPages'] ?? false) {
+ $tsfe->config['config']['typolinkLinkAccessRestrictedPages'] = $SAVED_link_to_restricted_pages;
+ $tsfe->config['config']['typolinkLinkAccessRestrictedPages_addParams'] = $SAVED_link_to_restricted_pages_additional_params;
+ }
+
return $attrs;
}
@@ -1290,34 +1304,6 @@ abstract class AbstractMenuContentObject
return $page;
}
- /**
- * Will change $LD (passed by reference) if the page is access restricted
- *
- * @param array $LD The array from the linkData() function
- * @param array $page Page array
- * @param string $mainTarget Main target value
- * @param string $typeOverride Type number override if any
- */
- protected function changeLinksForAccessRestrictedPages(&$LD, $page, $mainTarget, $typeOverride)
- {
- // If access restricted pages should be shown in menus, change the link of such pages to link to a redirection page:
- if (($this->mconf['showAccessRestrictedPages'] ?? false) && $this->mconf['showAccessRestrictedPages'] !== 'NONE' && !$this->getTypoScriptFrontendController()->checkPageGroupAccess($page)) {
- $thePage = $this->sys_page->getPage($this->mconf['showAccessRestrictedPages']);
- $addParams = str_replace(
- [
- '###RETURN_URL###',
- '###PAGE_ID###',
- ],
- [
- rawurlencode($LD['totalURL']),
- $page['uid'],
- ],
- $this->mconf['showAccessRestrictedPages.']['addParams']
- );
- $LD = $this->menuTypoLink($thePage, $mainTarget, $addParams, $typeOverride);
- }
- }
-
/**
* Creates a submenu level to the current level - if configured for.
*
@@ -1677,7 +1663,6 @@ abstract class AbstractMenuContentObject
if ($page['sectionIndex_uid'] ?? false) {
$conf['section'] = $page['sectionIndex_uid'];
}
- $conf['linkAccessRestrictedPages'] = !empty($this->mconf['showAccessRestrictedPages']);
$this->parent_cObj->typoLink('|', $conf);
$LD = $this->parent_cObj->lastTypoLinkLD;
$LD['totalURL'] = $this->parent_cObj->lastTypoLinkUrl;
diff --git a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
index 8f723276fdbe..19cd52d752ee 100644
--- a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
+++ b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
@@ -266,7 +266,7 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
rawurlencode($url),
$page['uid'],
],
- $tsfe->config['config']['typolinkLinkAccessRestrictedPages_addParams']
+ $tsfe->config['config']['typolinkLinkAccessRestrictedPages_addParams'] ?? ''
);
$url = $this->contentObjectRenderer->getTypoLink_URL($thePage['uid'] . ($pageType ? ',' . $pageType : ''), $addParams, $target);
$url = $this->forceAbsoluteUrl($url, $conf);
@@ -338,7 +338,12 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
return $page;
}
$shortcutMode = (int)($page['shortcut_mode'] ?? PageRepository::SHORTCUT_MODE_NONE);
+ $savedWhereGroupAccess = '';
try {
+ if ($disableGroupAccessCheck) {
+ $savedWhereGroupAccess = $pageRepository->where_groupAccess;
+ $pageRepository->where_groupAccess = '';
+ }
$shortcut = $pageRepository->getPageShortcut(
$page['shortcut'] ?? '',
$shortcutMode,
@@ -353,7 +358,10 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
$page['_SHORTCUT_PAGE_UID'] = $page['uid'];
}
} catch (\Exception $e) {
- return $page;
+ // Keep the existing page record if shortcut could not be resolved
+ }
+ if ($disableGroupAccessCheck) {
+ $pageRepository->where_groupAccess = $savedWhereGroupAccess;
}
return $page;
}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
index 0dc0e85dee08..cf8f21ff26f0 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
@@ -107,6 +107,9 @@ entities:
- self: {id: 1521, title: 'Current Year', slug: '/my-acme/forecasts/current-year'}
- self: {id: 1522, title: 'Next Year', slug: '/my-acme/forecasts/next-year'}
- self: {id: 1523, title: 'Five Years', slug: '/my-acme/forecasts/five-years'}
+ - self: {id: 1530, title: 'Employees', slug: '/my-acme/employees', type: *pageShortcut, shortcut: 'first' }
+ children:
+ - self: {id: 1531, title: 'Employee of the year', visitorGroups: -2, slug: '/my-acme/employees/employee-of-the-year'}
- self: {id: 1600, title: 'About us', slug: '/about'}
- self: {id: 1700, title: 'Announcements & News', type: *pageMount, mount: 7100, slug: '/news'}
- self: {id: 1800, title: 'Work in progress', hidden: 1, slug: '/never-visible-working-on-it' }
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
index c58bc5c223d9..9f05d25fb0cc 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
@@ -964,6 +964,96 @@ class SlugLinkGeneratorTest extends AbstractTestCase
self::assertSame($expectation, $json);
}
+ public function directoryMenuToAccessRestrictedPagesIsGeneratedDataProvider(): array
+ {
+ return [
+ 'All restricted pages are linked to welcome page' => [
+ 'https://acme.us/',
+ 1100,
+ 1500,
+ 1100,
+ 0,
+ 0,
+ [
+ [
+ 'title' => 'Whitepapers',
+ 'link' => '/welcome',
+ ],
+ [
+ 'title' => 'Forecasts',
+ 'link' => '/welcome',
+ ],
+ [
+ // Shortcut page, which resolves the shortcut and then the next page
+ 'title' => 'Employees',
+ 'link' => '/welcome',
+ ],
+ ],
+ ],
+ 'Inherited restricted pages are linked' => [
+ 'https://acme.us/',
+ 1100,
+ 1520,
+ 1100,
+ 0,
+ 0,
+ [
+ [
+ 'title' => 'Current Year',
+ // Should be
+ // 'link' => '/welcome',
+ // see https://forge.typo3.org/issues/16561
+ 'link' => '/my-acme/forecasts/current-year',
+ ],
+ [
+ 'title' => 'Next Year',
+ // Should be
+ // 'link' => '/welcome',
+ // see https://forge.typo3.org/issues/16561
+ 'link' => '/my-acme/forecasts/next-year',
+ ],
+ [
+ 'title' => 'Five Years',
+ // Should be
+ // 'link' => '/welcome',
+ // see https://forge.typo3.org/issues/16561
+ 'link' => '/my-acme/forecasts/five-years',
+ ],
+ ],
+ ],
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider directoryMenuToAccessRestrictedPagesIsGeneratedDataProvider
+ */
+ public function directoryMenuToAccessRestrictedPagesIsGenerated(string $hostPrefix, int $sourcePageId, int $directoryMenuParentPage, int $loginPageId, int $backendUserId, int $workspaceId, array $expectation): void
+ {
+ $response = $this->executeFrontendSubRequest(
+ (new InternalRequest($hostPrefix))
+ ->withPageId($sourcePageId)
+ ->withInstructions([
+ $this->createHierarchicalMenuProcessorInstruction([
+ 'special' => 'directory',
+ 'special.' => [
+ 'value' => $directoryMenuParentPage,
+ ],
+ 'levels' => 1,
+ 'showAccessRestrictedPages' => $loginPageId,
+ ]),
+ ]),
+ (new InternalRequestContext())
+ ->withWorkspaceId($backendUserId !== 0 ? $workspaceId : 0)
+ ->withBackendUserId($backendUserId)
+ );
+
+ $json = json_decode((string)$response->getBody(), true);
+ $json = $this->filterMenu($json);
+
+ self::assertSame($expectation, $json);
+ }
+
public function listMenuIsGeneratedDataProvider(): array
{
return [
diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php
index f1d4d0b94829..a5b8638f7978 100644
--- a/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php
@@ -437,7 +437,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'standard parameter without access protected setting' => [
[
'parameter' => 1,
- 'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
@@ -450,7 +449,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'standard parameter with access protected setting' => [
[
'parameter' => 10,
- 'linkAccessRestrictedPages' => true,
],
[
'showAccessRestrictedPages' => true,
@@ -463,7 +461,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'standard parameter with access protected setting "NONE" casts to boolean linkAccessRestrictedPages (delegates resolving to typoLink method internals)' => [
[
'parameter' => 10,
- 'linkAccessRestrictedPages' => true,
],
[
'showAccessRestrictedPages' => 'NONE',
@@ -476,7 +473,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'standard parameter with access protected setting (int)67 casts to boolean linkAccessRestrictedPages (delegates resolving to typoLink method internals)' => [
[
'parameter' => 10,
- 'linkAccessRestrictedPages' => true,
],
[
'showAccessRestrictedPages' => 67,
@@ -490,7 +486,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
[
'parameter' => 1,
'target' => '_blank',
- 'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
@@ -503,7 +498,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'parameter with typeOverride=10' => [
[
'parameter' => '10,10',
- 'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
@@ -516,7 +510,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'parameter with target and typeOverride=10' => [
[
'parameter' => '10,10',
- 'linkAccessRestrictedPages' => false,
'target' => '_self',
],
[
@@ -530,7 +523,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'parameter with invalid value in typeOverride=foobar ignores typeOverride' => [
[
'parameter' => 20,
- 'linkAccessRestrictedPages' => false,
'target' => '_self',
],
[
@@ -546,7 +538,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
[
'parameter' => 10,
'target' => '_blank',
- 'linkAccessRestrictedPages' => false,
'section' => 'section-name',
],
[
@@ -563,7 +554,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'standard parameter with additional parameters' => [
[
'parameter' => 10,
- 'linkAccessRestrictedPages' => false,
'section' => 'section-name',
'additionalParams' => '&test=foobar',
],
@@ -581,7 +571,6 @@ class AbstractMenuContentObjectTest extends UnitTestCase
'overridden page array uid value gets used as parameter' => [
[
'parameter' => 99,
- 'linkAccessRestrictedPages' => false,
'section' => 'section-name',
],
[
--
GitLab