diff --git a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php
index 1ccee03f856bec33a85f60c9a459c683b6a9e678..ba268af126595d6c29af6dd26d12ebfcb1455003 100644
--- a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php
+++ b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php
@@ -212,22 +212,14 @@ protected function collectNodeLinks(array &$tree, array &$node_links) {
* The access result.
*/
protected function menuLinkCheckAccess(MenuLinkInterface $instance) {
- $access_result = NULL;
- if ($this->account->hasPermission('link to any page')) {
- $access_result = AccessResult::allowed();
- }
- else {
- $url = $instance->getUrlObject();
+ $url = $instance->getUrlObject();
- // When no route name is specified, this must be an external link.
- if (!$url->isRouted()) {
- $access_result = AccessResult::allowed();
- }
- else {
- $access_result = $this->accessManager->checkNamedRoute($url->getRouteName(), $url->getRouteParameters(), $this->account, TRUE);
- }
+ if ($url->isRouted()) {
+ return $this->accessManager->checkNamedRoute($url->getRouteName(), $url->getRouteParameters(), $this->account, TRUE);
}
- return $access_result->cachePerPermissions();
+
+ // Must be an external link.
+ return AccessResult::allowed();
}
/**
diff --git a/core/modules/menu_ui/menu_ui.services.yml b/core/modules/menu_ui/menu_ui.services.yml
new file mode 100644
index 0000000000000000000000000000000000000000..79c75b803d4a6abb14f882c2ba084aa9e4c7d6b6
--- /dev/null
+++ b/core/modules/menu_ui/menu_ui.services.yml
@@ -0,0 +1,4 @@
+services:
+ menu_ui.menu_tree_manipulators:
+ class: Drupal\menu_ui\Menu\MenuUiMenuTreeManipulators
+ Drupal\menu_ui\Menu\MenuUiMenuTreeManipulators: '@menu_ui.menu_tree_manipulators'
diff --git a/core/modules/menu_ui/src/Menu/MenuUiMenuTreeManipulators.php b/core/modules/menu_ui/src/Menu/MenuUiMenuTreeManipulators.php
new file mode 100644
index 0000000000000000000000000000000000000000..0fbba4a803c99f3a813904b993a052f3f091adaa
--- /dev/null
+++ b/core/modules/menu_ui/src/Menu/MenuUiMenuTreeManipulators.php
@@ -0,0 +1,49 @@
+<?php
+
+namespace Drupal\menu_ui\Menu;
+
+use Drupal\Core\Access\AccessResult;
+
+/**
+ * Provides menu tree manipulators to be used when managing menu links.
+ */
+class MenuUiMenuTreeManipulators {
+
+ /**
+ * Grants access to a menu tree when used in the menu management form.
+ *
+ * This manipulator allows access to menu links with inaccessible routes.
+ *
+ * Example use cases:
+ * - A login menu link, using the `user.login` route, is not accessible to a
+ * logged-in user, but the site builder still needs to configure the menu
+ * link.
+ * - A site builder wants to create a menu item for a Views page that has not
+ * been created. In this case, there is no access to the route because it
+ * does not exist.
+ *
+ * @param \Drupal\Core\Menu\MenuLinkTreeElement[] $tree
+ * The menu link tree to manipulate.
+ *
+ * @return \Drupal\Core\Menu\MenuLinkTreeElement[]
+ * The manipulated menu link tree.
+ *
+ * @internal
+ * This menu tree manipulator is intended for use only in the context of
+ * MenuForm because the user permissions to administer links is already
+ * checked. Don't use this manipulator in other places.
+ *
+ * @see \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess()
+ * @see \Drupal\menu_ui\MenuForm
+ */
+ public function checkAccess(array $tree): array {
+ foreach ($tree as $element) {
+ $element->access = AccessResult::allowed();
+ if ($element->subtree) {
+ $element->subtree = $this->checkAccess($element->subtree);
+ }
+ }
+ return $tree;
+ }
+
+}
diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php
index 9d90ac5a607d90f27b5776b722ed9f4feb60fefb..9b4561f4da01c4949495e9a63919a6d4e6cc35a2 100644
--- a/core/modules/menu_ui/src/MenuForm.php
+++ b/core/modules/menu_ui/src/MenuForm.php
@@ -231,7 +231,13 @@ protected function buildOverviewForm(array &$form, FormStateInterface $form_stat
// We indicate that a menu administrator is running the menu access check.
$this->getRequest()->attributes->set('_menu_admin', TRUE);
$manipulators = [
- ['callable' => 'menu.default_tree_manipulators:checkAccess'],
+ // Use a dedicated menu tree access check manipulator as users editing
+ // this form, granted with 'administer menu' permission, should be able to
+ // access menu links with inaccessible routes. The default menu tree
+ // manipulator only allows the access to menu links with accessible routes.
+ // @see \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess()
+ // @see \Drupal\menu_ui\Menu\MenuUiMenuTreeManipulators::checkAccess()
+ ['callable' => 'menu_ui.menu_tree_manipulators:checkAccess'],
['callable' => 'menu.default_tree_manipulators:generateIndexAndSort'],
];
$tree = $this->menuTree->transform($tree, $manipulators);
diff --git a/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php b/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
index 97b31371cb758f37db2df3965bb675e2e3371e93..c7f653098abfc4ec538c448be052ffd998b9a4d3 100644
--- a/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
+++ b/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
@@ -727,10 +727,15 @@ public function testUnpublishedNodeMenuItem() {
$this->drupalLogout();
$this->drupalLogin($this->adminUser);
$this->drupalGet('admin/structure/menu/manage/' . $item->getMenuName());
- $this->assertSession()->pageTextNotContains($item->getTitle());
+ $this->assertSession()->linkExists($item->getTitle());
// The cache contexts associated with the (in)accessible menu links are
// bubbled. See DefaultMenuLinkTreeManipulators::menuLinkCheckAccess().
$this->assertSession()->responseHeaderContains('X-Drupal-Cache-Contexts', 'user.permissions');
+
+ // The menu link admin is able to administer the link but cannot access the
+ // route as is not granted with 'bypass node access' permission.
+ $this->clickLink($item->getTitle());
+ $this->assertSession()->statusCodeEquals(403);
}
/**
@@ -1229,4 +1234,47 @@ public function testMenuUiWithPendingRevisions() {
$this->assertSession()->elementNotExists('xpath', '//div[contains(@class, "messages--error")]');
}
+ /**
+ * Tests the user login/logout links.
+ */
+ public function testUserLoginUserLogoutLinks() {
+ MenuLinkContent::create([
+ 'menu' => 'tools',
+ 'link' => [
+ 'uri' => 'internal:/user/login',
+ ],
+ 'title' => 'Login',
+ ])->save();
+ MenuLinkContent::create([
+ 'menu' => 'tools',
+ 'link' => [
+ 'uri' => 'internal:/user/logout',
+ ],
+ 'title' => 'Logout',
+ ])->save();
+
+ $assert = $this->assertSession();
+
+ $block = $this->drupalPlaceBlock('system_menu_block:tools');
+ $this->drupalGet('<front>');
+ $assert->linkExists('Login');
+ $assert->linkNotExists('Logout');
+
+ $this->drupalLogin($this->createUser());
+ $this->drupalGet('<front>');
+ $assert->linkNotExists('Login');
+ $assert->linkExists('Logout');
+
+ // Delete the block, we're now checking the Menu UI form.
+ $block->delete();
+
+ $this->drupalLogin($this->createUser(['administer menu']));
+ $this->drupalGet('admin/structure/menu/manage/tools');
+
+ $assert->linkExists('Logout');
+ // Check that the login link is accessible even the route is not.
+ $this->assertFalse(Url::fromRoute('user.login')->access($this->loggedInUser));
+ $assert->linkExists('Login');
+ }
+
}
diff --git a/core/modules/system/tests/src/Functional/Menu/LinksetControllerTest.php b/core/modules/system/tests/src/Functional/Menu/LinksetControllerTest.php
index ed8d0863554b06772ab855388520d242879790da..05a1bc4e1a0e9c2b30dffc183fc304fc9f2705d9 100644
--- a/core/modules/system/tests/src/Functional/Menu/LinksetControllerTest.php
+++ b/core/modules/system/tests/src/Functional/Menu/LinksetControllerTest.php
@@ -280,12 +280,10 @@ public function testUserAccountMenu() {
$this->enableEndpoint(TRUE);
$expected_cacheability = new CacheableMetadata();
$expected_cacheability->addCacheContexts([
- 'user.permissions',
'user.roles:authenticated',
]);
$expected_cacheability->addCacheTags([
'config:system.menu.account',
- 'config:user.role.anonymous',
'http_response',
]);
$response = $this->doRequest('GET', Url::fromUri('base:/system/menu/account/linkset'));
diff --git a/core/modules/system/tests/src/Functional/System/AdminTest.php b/core/modules/system/tests/src/Functional/System/AdminTest.php
index 8f72a98ec74065b550f757b431f244c19aec068f..cf65b1ac2731c517346836fd60cff4f4b9c87988 100644
--- a/core/modules/system/tests/src/Functional/System/AdminTest.php
+++ b/core/modules/system/tests/src/Functional/System/AdminTest.php
@@ -65,7 +65,14 @@ public function testAdminPages() {
// Verify that all visible, top-level administration links are listed on
// the main administration page.
- foreach ($this->getTopLevelMenuLinks() as $item) {
+ foreach ($this->getTopLevelMenuLinks() as $element) {
+ $item = $element->link;
+ if (!$element->access->isAllowed()) {
+ // If the link is not accessible, it should not be rendered.
+ // @see \Drupal\Core\Menu\MenuLinkTree::buildItems().
+ $this->assertSession()->linkNotExists($item->getTitle());
+ continue;
+ }
$this->assertSession()->linkExists($item->getTitle());
$this->assertSession()->linkByHrefExists($item->getUrlObject()->toString());
// The description should appear below the link.
@@ -124,7 +131,7 @@ public function testAdminPages() {
/**
* Returns all top level menu links.
*
- * @return \Drupal\Core\Menu\MenuLinkInterface[]
+ * @return \Drupal\Core\Menu\MenuLinkTreeElement[]
*/
protected function getTopLevelMenuLinks() {
$menu_tree = \Drupal::menuTree();
@@ -137,15 +144,7 @@ protected function getTopLevelMenuLinks() {
['callable' => 'menu.default_tree_manipulators:checkAccess'],
['callable' => 'menu.default_tree_manipulators:flatten'],
];
- $tree = $menu_tree->transform($tree, $manipulators);
-
- // Transform the tree to a list of menu links.
- $menu_links = [];
- foreach ($tree as $element) {
- $menu_links[] = $element->link;
- }
-
- return $menu_links;
+ return $menu_tree->transform($tree, $manipulators);
}
/**
diff --git a/core/modules/views_ui/tests/src/Functional/DisplayPathTest.php b/core/modules/views_ui/tests/src/Functional/DisplayPathTest.php
index 07d0178ff73fe29558a519105fd4ec63850b336a..f5c5c5d40b88904912878cd74da51d3d76d7e055 100644
--- a/core/modules/views_ui/tests/src/Functional/DisplayPathTest.php
+++ b/core/modules/views_ui/tests/src/Functional/DisplayPathTest.php
@@ -217,6 +217,7 @@ public function testDefaultMenuTabRegression() {
'administer menu',
'link to any page',
'access toolbar',
+ 'access administration pages',
]);
$this->drupalLogin($admin_user);
diff --git a/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php b/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php
index c36c37fea913c345a8ea53db0751f4faee2bad81..43ad9d4784c47af68af282e207b6865b74847471 100644
--- a/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Menu/MenuLinkTreeTest.php
@@ -2,8 +2,11 @@
namespace Drupal\KernelTests\Core\Menu;
+use Drupal\Core\Menu\InaccessibleMenuLink;
use Drupal\Core\Menu\MenuLinkTreeElement;
use Drupal\Core\Menu\MenuTreeParameters;
+use Drupal\Core\Session\AnonymousUserSession;
+use Drupal\Core\Session\UserSession;
use Drupal\KernelTests\KernelTestBase;
use Drupal\Tests\Core\Menu\MenuLinkMock;
@@ -133,4 +136,42 @@ public function testCreateLinksInMenu() {
$this->assertEquals(3, $height);
}
+ /**
+ * Tests user/login and user/logout links.
+ */
+ public function testUserLoginAndUserLogoutLinks() {
+ $account_switcher = $this->container->get('account_switcher');
+
+ $login_menu_link = MenuLinkMock::create(['id' => 'user_login_example', 'route_name' => 'user.login']);
+ $logout_menu_link = MenuLinkMock::create(['id' => 'user_logout_example', 'route_name' => 'user.logout']);
+
+ $this->menuLinkManager->addDefinition($login_menu_link->getPluginId(), $login_menu_link->getPluginDefinition());
+ $this->menuLinkManager->addDefinition($logout_menu_link->getPluginId(), $logout_menu_link->getPluginDefinition());
+
+ // Returns the accessible links from transformed 'mock' menu tree.
+ $get_accessible_links = function () {
+ $parameters = new MenuTreeParameters();
+ $manipulators = [
+ ['callable' => 'menu.default_tree_manipulators:checkAccess'],
+ ];
+
+ $tree = $this->linkTree->load('mock', $parameters);
+ $this->linkTree->transform($tree, $manipulators);
+
+ return array_keys(
+ array_filter($tree, function (MenuLinkTreeElement $element) {
+ return !$element->link instanceof InaccessibleMenuLink;
+ })
+ );
+ };
+
+ // Check that anonymous can only access the login link.
+ $account_switcher->switchTo(new AnonymousUserSession());
+ $this->assertSame(['user_login_example'], $get_accessible_links());
+
+ // Ensure that also user 1 does not see the login link.
+ $account_switcher->switchTo(new UserSession(['uid' => 1]));
+ $this->assertSame(['user_logout_example'], $get_accessible_links());
+ }
+
}
diff --git a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php
index 896699b805176222ab6c0d57e3af1e439661df2e..d24e0b7eaf39deb409c26dcf3d7dead5e8256883 100644
--- a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php
+++ b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php
@@ -207,15 +207,12 @@ public function testCheckAccess() {
$this->cacheContextManager->assertValidTokens(['user'])->shouldBeCalled()->willReturn(TRUE);
$this->originalTree[8]->access = AccessResult::allowed()->cachePerUser();
- // Since \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess()
- // allows access to any link if the user has the 'link to any page'
- // permission, *every* single access result is varied by permissions.
$tree = $this->defaultMenuTreeManipulators->checkAccess($this->originalTree);
// Menu link 1: route without parameters, access forbidden, but at level 0,
// hence kept.
$element = $tree[1];
- $this->assertEquals(AccessResult::forbidden()->cachePerPermissions(), $element->access);
+ $this->assertEquals(AccessResult::forbidden(), $element->access);
$this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
// Menu link 2: route with parameters, access granted.
$element = $tree[2];
@@ -223,21 +220,19 @@ public function testCheckAccess() {
$this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
// Menu link 3: route with parameters, AccessResult::neutral(), top-level
// inaccessible link, hence kept for its cacheability metadata.
- // Note that the permissions cache context is added automatically, because
- // we always check the "link to any page" permission.
$element = $tree[2]->subtree[3];
- $this->assertEquals(AccessResult::neutral()->cachePerPermissions(), $element->access);
+ $this->assertEquals(AccessResult::neutral(), $element->access);
$this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
// Menu link 4: child of menu link 3, which was AccessResult::neutral(),
// hence menu link 3's subtree is removed, of which this menu link is one.
$this->assertArrayNotHasKey(4, $tree[2]->subtree[3]->subtree);
// Menu link 5: no route name, treated as external, hence access granted.
$element = $tree[5];
- $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $element->access);
+ $this->assertEquals(AccessResult::allowed(), $element->access);
$this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
// Menu link 6: external URL, hence access granted.
$element = $tree[6];
- $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $element->access);
+ $this->assertEquals(AccessResult::allowed(), $element->access);
$this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
// Menu link 7: 'access' already set: AccessResult::neutral(), top-level
// inaccessible link, hence kept for its cacheability metadata.
@@ -247,31 +242,31 @@ public function testCheckAccess() {
$element = $tree[5]->subtree[7];
$this->assertEquals(AccessResult::neutral(), $element->access);
$this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
- // Menu link 8: 'access' already set, note that 'per permissions' caching
- // is not added.
+ // Menu link 8: 'access' already set.
$element = $tree[8];
$this->assertEquals(AccessResult::allowed()->cachePerUser(), $element->access);
$this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link);
}
/**
- * Tests checkAccess() tree manipulator with 'link to any page' permission.
+ * Tests checkAccess() tree manipulator.
*
* @covers ::checkAccess
* @covers ::menuLinkCheckAccess
*/
- public function testCheckAccessWithLinkToAnyPagePermission() {
+ public function testCheckAccessTreeManipulator(): void {
$this->mockTree();
- $this->currentUser->expects($this->exactly(9))
- ->method('hasPermission')
- ->with('link to any page')
- ->willReturn(TRUE);
+ // There are 9 checks but one is on an external link, so the route access
+ // checker should be called only 8 times.
+ // @see \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::menuLinkCheckAccess()
+ $this->accessManager->expects($this->exactly(8))
+ ->method('checkNamedRoute')
+ ->willReturn(AccessResult::allowed());
$this->mockTree();
- $this->cacheContextManager->assertValidTokens(['user.permissions'])->shouldBeCalled()->willReturn(TRUE);
$this->defaultMenuTreeManipulators->checkAccess($this->originalTree);
- $expected_access_result = AccessResult::allowed()->cachePerPermissions();
+ $expected_access_result = AccessResult::allowed();
$this->assertEquals($expected_access_result, $this->originalTree[1]->access);
$this->assertEquals($expected_access_result, $this->originalTree[2]->access);
$this->assertEquals($expected_access_result, $this->originalTree[2]->subtree[3]->access);
@@ -366,8 +361,8 @@ public function testCheckNodeAccess() {
$this->assertEquals($node_access_result, $tree[1]->access);
$this->assertEquals($node_access_result, $tree[2]->access);
$this->assertEquals(AccessResult::neutral(), $tree[2]->subtree[3]->access);
- $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $tree[5]->access);
- $this->assertEquals(AccessResult::neutral()->cachePerPermissions(), $tree[5]->subtree[6]->access);
+ $this->assertEquals(AccessResult::allowed(), $tree[5]->access);
+ $this->assertEquals(AccessResult::neutral(), $tree[5]->subtree[6]->access);
}
/**