From 82d33fc20b359cec5661442691c61a5553cd8b30 Mon Sep 17 00:00:00 2001 From: Benni Mack 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 Tested-by: Jochen Tested-by: Stefan Bürk Tested-by: Oliver Bartsch Tested-by: Benni Mack Reviewed-by: Jochen Reviewed-by: Stefan Bürk Reviewed-by: Oliver Bartsch Reviewed-by: Benni Mack --- .../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 @@ protected function link($key, $altTarget, $typeOverride) $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 @@ protected function link($key, $altTarget, $typeOverride) $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 @@ protected function link($key, $altTarget, $typeOverride) $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 @@ protected function link($key, $altTarget, $typeOverride) $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 @@ protected function determineOriginalShortcutPage(array $page) 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 @@ protected function menuTypoLink($page, $oTarget, $addParams, $typeOverride, ?int 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 @@ public function build(array &$linkDetails, string $linkText, string $target, arr 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 @@ protected function resolveShortcutPage(array $page, PageRepository $pageReposito 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 @@ protected function resolveShortcutPage(array $page, PageRepository $pageReposito $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 @@ public function directoryMenuIsGenerated(string $hostPrefix, int $sourcePageId, 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 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'standard parameter without access protected setting' => [ [ 'parameter' => 1, - 'linkAccessRestrictedPages' => false, ], [ 'showAccessRestrictedPages' => false, @@ -450,7 +449,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'standard parameter with access protected setting' => [ [ 'parameter' => 10, - 'linkAccessRestrictedPages' => true, ], [ 'showAccessRestrictedPages' => true, @@ -463,7 +461,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): '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 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): '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 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): [ 'parameter' => 1, 'target' => '_blank', - 'linkAccessRestrictedPages' => false, ], [ 'showAccessRestrictedPages' => false, @@ -503,7 +498,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'parameter with typeOverride=10' => [ [ 'parameter' => '10,10', - 'linkAccessRestrictedPages' => false, ], [ 'showAccessRestrictedPages' => false, @@ -516,7 +510,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'parameter with target and typeOverride=10' => [ [ 'parameter' => '10,10', - 'linkAccessRestrictedPages' => false, 'target' => '_self', ], [ @@ -530,7 +523,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'parameter with invalid value in typeOverride=foobar ignores typeOverride' => [ [ 'parameter' => 20, - 'linkAccessRestrictedPages' => false, 'target' => '_self', ], [ @@ -546,7 +538,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): [ 'parameter' => 10, 'target' => '_blank', - 'linkAccessRestrictedPages' => false, 'section' => 'section-name', ], [ @@ -563,7 +554,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'standard parameter with additional parameters' => [ [ 'parameter' => 10, - 'linkAccessRestrictedPages' => false, 'section' => 'section-name', 'additionalParams' => '&test=foobar', ], @@ -581,7 +571,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider(): 'overridden page array uid value gets used as parameter' => [ [ 'parameter' => 99, - 'linkAccessRestrictedPages' => false, 'section' => 'section-name', ], [