Commit 18b65882 authored by Benni Mack's avatar Benni Mack
Browse files

[BUGFIX] Resolve shortcuts in HMENU to access restricted pages

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/+/72785


Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent 90817ca9
......@@ -1204,7 +1204,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;
......@@ -1212,6 +1212,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']);
......@@ -1245,8 +1254,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'];
......@@ -1299,6 +1306,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;
}
......@@ -1332,34 +1346,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.
*
......@@ -1719,7 +1705,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;
......
......@@ -270,7 +270,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);
......@@ -342,7 +342,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,
......@@ -357,7 +362,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;
}
......
......@@ -113,6 +113,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' }
......
......@@ -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 [
......
......@@ -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',
],
[
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment