Skip to content

Commit

Permalink
[BUGFIX] Resolve shortcuts in HMENU to access restricted pages
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@bmack
bmack committed Dec 23, 2021
1 parent f8d021c commit 82d33fc
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 45 deletions.
49 changes: 17 additions & 32 deletions typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
View file
Expand Up @@ -1201,14 +1201,23 @@ 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;
}

$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']);
Expand Down Expand Up @@ -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'];
Expand All @@ -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;
}

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 10 additions & 2 deletions typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
View file
Expand Up @@ -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' }
Expand Down
90 changes: 90 additions & 0 deletions typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
View file
Expand Up @@ -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 [
Expand Down
11 changes: 0 additions & 11 deletions typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php
View file
Expand Up @@ -437,7 +437,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'standard parameter without access protected setting' => [
[
'parameter' => 1,
'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
Expand All @@ -450,7 +449,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'standard parameter with access protected setting' => [
[
'parameter' => 10,
'linkAccessRestrictedPages' => true,
],
[
'showAccessRestrictedPages' => true,
Expand All @@ -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',
Expand All @@ -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,
Expand All @@ -490,7 +486,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
[
'parameter' => 1,
'target' => '_blank',
'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
Expand All @@ -503,7 +498,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'parameter with typeOverride=10' => [
[
'parameter' => '10,10',
'linkAccessRestrictedPages' => false,
],
[
'showAccessRestrictedPages' => false,
Expand All @@ -516,7 +510,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'parameter with target and typeOverride=10' => [
[
'parameter' => '10,10',
'linkAccessRestrictedPages' => false,
'target' => '_self',
],
[
Expand All @@ -530,7 +523,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'parameter with invalid value in typeOverride=foobar ignores typeOverride' => [
[
'parameter' => 20,
'linkAccessRestrictedPages' => false,
'target' => '_self',
],
[
Expand All @@ -546,7 +538,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
[
'parameter' => 10,
'target' => '_blank',
'linkAccessRestrictedPages' => false,
'section' => 'section-name',
],
[
Expand All @@ -563,7 +554,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'standard parameter with additional parameters' => [
[
'parameter' => 10,
'linkAccessRestrictedPages' => false,
'section' => 'section-name',
'additionalParams' => '&test=foobar',
],
Expand All @@ -581,7 +571,6 @@ public function menuTypoLinkCreatesExpectedTypoLinkConfigurationDataProvider():
'overridden page array uid value gets used as parameter' => [
[
'parameter' => 99,
'linkAccessRestrictedPages' => false,
'section' => 'section-name',
],
[
Expand Down

0 comments on commit 82d33fc

Please sign in to comment.