diff --git a/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php b/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php
index a527605ea737cfdac56e58992459ac1e2d0f644c..f9dda50ca62e2e17b178ed93909269104f17021d 100644
--- a/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php
+++ b/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php
@@ -55,8 +55,23 @@ public function __construct(
*/
public function access(RouteMatchInterface $route_match, AccountInterface $account): AccessResultInterface {
$parameters = $route_match->getParameters()->all();
- // Load links in the 'admin' menu matching this route.
+ $route = $route_match->getRouteObject();
+ // Load links in the 'admin' menu matching this route. First, try to find
+ // the menu link using all specified parameters.
$links = $this->menuLinkManager->loadLinksByRoute($route_match->getRouteName(), $parameters, 'admin');
+ // If the menu link was not found, try finding it without the parameters
+ // that match the route defaults. Depending on whether the parameter is
+ // specified in the menu item with a value matching the default, or not
+ // specified at all, will change how it is stored in the menu_tree table. In
+ // both cases the route match parameters will always include the default
+ // parameters. This fallback method of finding the menu item is needed so
+ // that menu items will work in either case.
+ // @todo Remove this fallback in https://drupal.org/i/3359511.
+ if (empty($links)) {
+
+ $parameters_without_defaults = array_filter($parameters, fn ($key) => !$route->hasDefault($key) || $route->getDefault($key) !== $parameters[$key], ARRAY_FILTER_USE_KEY);
+ $links = $this->menuLinkManager->loadLinksByRoute($route_match->getRouteName(), $parameters_without_defaults, 'admin');
+ }
if (empty($links)) {
// If we did not find a link then we have no opinion on access.
return AccessResult::neutral();
@@ -65,7 +80,7 @@ public function access(RouteMatchInterface $route_match, AccountInterface $accou
}
/**
- * Check that the given route has access to one of it's child routes.
+ * Check that the given route has access to child routes.
*
* @param \Drupal\Core\Menu\MenuLinkInterface $link
* The menu link.
@@ -87,7 +102,10 @@ protected function hasAccessToChildMenuItems(MenuLinkInterface $link, AccountInt
if (empty($tree)) {
$route = $this->router->getRouteCollection()->get($link->getRouteName());
if ($route) {
- return AccessResult::allowedIf(empty($route->getRequirement('_access_admin_menu_block_page')));
+ return AccessResult::allowedIf(
+ empty($route->getRequirement('_access_admin_menu_block_page'))
+ && empty($route->getRequirement('_access_admin_overview_page'))
+ );
}
return AccessResult::neutral();
}
diff --git a/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php b/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php
index 5bc438442bc45fc311aa27cc40bde27afdf835e7..7de3bab0bb5104145d70c33c8f0ad1a1a785c3c8 100644
--- a/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php
+++ b/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php
@@ -33,8 +33,16 @@ public function accessAdminMenuBlockPage(RouteBuildEvent $event) {
foreach ($routes as $route) {
// Do not use a leading slash when comparing to the _controller string
// because the leading slash in a fully-qualified method name is optional.
- if ($route->hasDefault('_controller') && ltrim($route->getDefault('_controller'), '\\') === 'Drupal\system\Controller\SystemController::systemAdminMenuBlockPage') {
- $route->setRequirement('_access_admin_menu_block_page', 'TRUE');
+ if ($route->hasDefault('_controller')) {
+ switch (ltrim($route->getDefault('_controller'), '\\')) {
+ case 'Drupal\system\Controller\SystemController::systemAdminMenuBlockPage':
+ $route->setRequirement('_access_admin_menu_block_page', 'TRUE');
+ break;
+
+ case 'Drupal\system\Controller\SystemController::overview':
+ $route->setRequirement('_access_admin_overview_page', 'TRUE');
+ break;
+ }
}
}
}
diff --git a/core/modules/system/system.services.yml b/core/modules/system/system.services.yml
index e6380fbf1380f4b4f12e2b1fb766a2dc91fb98bd..b56f401f9f050133c49ed204613530f7abb683e1 100644
--- a/core/modules/system/system.services.yml
+++ b/core/modules/system/system.services.yml
@@ -14,6 +14,11 @@ services:
arguments: ['@access_manager', '@menu.link_tree', '@router', '@plugin.manager.menu.link']
tags:
- { name: access_check, applies_to: _access_admin_menu_block_page }
+ access_check.admin_overview_page:
+ class: Drupal\system\Access\SystemAdminMenuBlockAccessCheck
+ arguments: [ '@access_manager', '@menu.link_tree', '@router', '@plugin.manager.menu.link' ]
+ tags:
+ - { name: access_check, applies_to: _access_admin_overview_page }
system.manager:
class: Drupal\system\SystemManager
arguments: ['@module_handler', '@request_stack', '@menu.link_tree', '@menu.active_trail']
diff --git a/core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml b/core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml
index d694e419b85aca6e7693d7dadf070d553d75c8d2..503cbb6622ae4c19a045ccbe384085071f57421d 100644
--- a/core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml
+++ b/core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml
@@ -22,7 +22,7 @@ menu_test.parent_test:
parent: system.admin
menu_test.parent_test.child1_test:
title: 'Menu child1'
- description: 'Menu item description child1'
+ description: 'Menu child1: uses SystemController::overview'
route_name: menu_test.child1_test
parent: menu_test.parent_test
menu_test.parent_test.child2_test:
@@ -33,23 +33,33 @@ menu_test.parent_test.child2_test:
menu_test.parent_test.child3_test:
title: 'Menu child3'
description: 'Menu item description child3'
- route_name: menu_test.child3_test
+ route_name: menu_test.child3_test_block
parent: menu_test.parent_test
-menu_test.parent_test.child_test.super_child1_test:
- title: 'Menu super child1'
- description: 'Menu item description super child1'
- route_name: menu_test.super_child1_test
+menu_test.parent_test.child4_test:
+ title: 'Menu child4'
+ description: 'Menu item description child4'
+ route_name: menu_test.child4_test_overview
+ parent: menu_test.parent_test
+menu_test.parent_test.child_test.grand_child1_test:
+ title: 'Menu grand child1'
+ description: 'Menu grand child1: direct parent uses SystemController::overview'
+ route_name: menu_test.grand_child1_test
parent: menu_test.parent_test.child1_test
-menu_test.parent_test.child_test.super_child2_test:
- title: 'Menu super child2'
- description: 'Menu item description super child2'
- route_name: menu_test.super_child2_test
+menu_test.parent_test.child_test.grand_child2_test:
+ title: 'Menu grand child2'
+ description: 'Menu item description grand child2'
+ route_name: menu_test.grand_child2_test
parent: menu_test.parent_test.child2_test
-menu_test.parent_test.child_test.super_child3_test:
- title: 'Menu super child3'
- description: 'Menu item description super child3'
- route_name: menu_test.super_child3_test
+menu_test.parent_test.child_test.grand_child3_test:
+ title: 'Menu grand child3'
+ description: 'Menu item description grand child3'
+ route_name: menu_test.grand_child3_test
parent: menu_test.parent_test.child2_test
+menu_test.parent_test.child_test.great_grand_child1_test:
+ title: 'Menu great grand child1'
+ description: 'Menu great grand child1: grand parent uses SystemController::overview'
+ route_name: menu_test.great_grand_child1_test
+ parent: menu_test.parent_test.child_test.grand_child1_test
menu_test.menu_parent_test_param:
title: 'Menu Parent Param'
description: 'Menu item description parent'
diff --git a/core/modules/system/tests/modules/menu_test/menu_test.permissions.yml b/core/modules/system/tests/modules/menu_test/menu_test.permissions.yml
index 47c528f01254e9ad05d751bc07039884e0bc1da1..a175145d3b7c7028a7c7a4596459594b768ac673 100644
--- a/core/modules/system/tests/modules/menu_test/menu_test.permissions.yml
+++ b/core/modules/system/tests/modules/menu_test/menu_test.permissions.yml
@@ -7,12 +7,15 @@ access child1 test page:
access child2 test page:
title: 'Access child2 test page'
restrict access: true
-access super child1 test page:
- title: 'Access super child1 test page'
+access grand child1 test page:
+ title: 'Access grand child1 test page'
restrict access: true
-access super child2 test page:
- title: 'Access super child2 test page'
+access grand child2 test page:
+ title: 'Access grand child2 test page'
restrict access: true
-access super child3 test page:
- title: 'Access super child3 test page'
+access grand child3 test page:
+ title: 'Access grand child3 test page'
+ restrict access: true
+access great grand child1 test page:
+ title: 'Access great grand child1 test page'
restrict access: true
diff --git a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
index 5b83aeb2af65962d60f158962395c1c38a25138e..d88f9f21774e12fa09911460f03593c5f8092592 100644
--- a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
+++ b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
@@ -48,7 +48,9 @@ menu_test.parent_test:
menu_test.child1_test:
path: '/parent_test/child1_test'
defaults:
- _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'
+ _controller: '\Drupal\system\Controller\SystemController::overview'
+ link_id: 'menu_test.parent_test.child1_test'
+ _title: 'Configuration'
requirements:
_permission: 'access child1 test page'
@@ -59,32 +61,49 @@ menu_test.child2_test:
requirements:
_permission: 'access child2 test page'
-menu_test.child3_test:
+menu_test.child3_test_block:
path: '/parent_test/child3_test'
defaults:
_controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'
requirements:
_access: 'TRUE'
-menu_test.super_child1_test:
- path: '/parent_test/child_test/super_child1_test'
+menu_test.child4_test_overview:
+ path: '/parent_test/child4_test'
+ defaults:
+ _controller: '\Drupal\system\Controller\SystemController::overview'
+ link_id: 'menu_test.parent_test.child4_test'
+ _title: 'Configuration'
+ requirements:
+ _access: 'TRUE'
+
+menu_test.grand_child1_test:
+ path: '/parent_test/child_test/grand_child1_test'
+ defaults:
+ _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'
+ requirements:
+ _permission: 'access grand child1 test page'
+
+menu_test.great_grand_child1_test:
+ path: '/parent_test/child_test/great_grand_child1_test'
defaults:
_controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage'
requirements:
- _permission: 'access super child1 test page'
-menu_test.super_child2_test:
- path: '/parent_test/child_test/super_child2_test'
+ _permission: 'access great grand child1 test page'
+
+menu_test.grand_child2_test:
+ path: '/parent_test/child_test/grand_child2_test'
defaults:
_controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage'
requirements:
- _permission: 'access super child2 test page'
+ _permission: 'access grand child2 test page'
-menu_test.super_child3_test:
- path: '/parent_test/child_test/super_child3_test'
+menu_test.grand_child3_test:
+ path: '/parent_test/child_test/grand_child3_test'
defaults:
_controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage'
requirements:
- _permission: 'access super child3 test page'
+ _permission: 'access grand child3 test page'
menu_test.parent_test_param:
path: '/parent_test_param/{param}'
diff --git a/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php b/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php
index 307fbf3613002db5dd45fe43ebc8ea40735ffc52..b1a83af3cb7c2ce4a2f9d62a57a2107075733f91 100644
--- a/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php
+++ b/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php
@@ -18,7 +18,7 @@ class MenuAccessTest extends BrowserTestBase {
*
* @var array
*/
- protected static $modules = ['block', 'filter', 'menu_test', 'toolbar'];
+ protected static $modules = ['block', 'filter', 'toolbar', 'menu_ui'];
/**
* {@inheritdoc}
@@ -40,6 +40,7 @@ protected function setUp(): void {
* @see \Drupal\menu_test\Access\AccessCheck::access()
*/
public function testMenuBlockLinksAccessCheck() {
+ $this->container->get('module_installer')->install(['menu_test']);
$this->drupalPlaceBlock('system_menu_block:account');
// Test that there's link rendered on the route.
$this->drupalGet('menu_test_access_check_session');
@@ -118,33 +119,40 @@ public function testSystemAdminMenuBlockAccessCheck(): void {
// This user doesn't have access to any of the child pages, so the parent
// pages should not be accessible.
$this->drupalLogin($webUser);
- $this->assertMenuItemRoutesAccess(403, 'admin/structure', 'admin/people');
- // As menu_test adds a menu link under config.
- $this->assertMenuItemRoutesAccess(200, 'admin/config');
+ $this->assertMenuItemRoutesAccess(403, 'admin/structure', 'admin/people', 'admin/config');
+ // The test cases below depend on routes, menu items and permissions added
+ // by the menu_test module. It is not enabled before this to ensure that any
+ // other configuration it provides that we don't need for these test cases
+ // does not affect the assertions above.
+ $this->container->get('module_installer')->install(['menu_test']);
// Test access to routes in the admin menu. The routes are in a menu tree
// of the hierarchy:
// menu_test.parent_test
// -menu_test.child1_test
- // --menu_test.super_child1_test
+ // --menu_test.grand_child1_test
// -menu_test.child2_test
- // --menu_test.super_child2_test
- // --menu_test.super_child3_test
- // -menu_test.child3_test
- // All routes in this tree except the "super_child" routes should have the
+ // --menu_test.grand_child2_test
+ // --menu_test.grand_child3_test
+ // -menu_test.child3_test_block
+ // -menu_test.child4_test_overview
+ // All routes in this tree except the "grand_child" routes should have the
// '_access_admin_menu_block_page' requirement which denies access unless
// the user has access to a menu item under that route. Route
- // 'menu_test.child3_test' has no menu items underneath it so no user should
- // have access to this route even though it has the requirement
+ // 'menu_test.child3_test_block' and 'menu_test.child4_test_overview' have
+ // no menu items underneath it so no user should have access to these routes
+ // even though they have the requirement:
// `_access: 'TRUE'`.
$tree_routes = [
'menu_test.parent_test',
'menu_test.child1_test',
'menu_test.child2_test',
- 'menu_test.child3_test',
- 'menu_test.super_child1_test',
- 'menu_test.super_child2_test',
- 'menu_test.super_child3_test',
+ 'menu_test.child3_test_block',
+ 'menu_test.child4_test_overview',
+ 'menu_test.grand_child1_test',
+ 'menu_test.grand_child2_test',
+ 'menu_test.grand_child3_test',
+ 'menu_test.great_grand_child1_test',
];
// Create a user with access to only the top level parent.
@@ -152,72 +160,92 @@ public function testSystemAdminMenuBlockAccessCheck(): void {
'access parent test page',
]);
// Create a user with access to the parent and child routes but none of the
- // super child routes.
+ // grand child routes.
$childOnlyUser = $this->drupalCreateUser([
'access parent test page',
'access child1 test page',
'access child2 test page',
]);
- // Create 3 users all with access the parent and child but only 1 super
+ // Create 3 users all with access the parent and child but only 1 grand
// child route.
- $superChild1User = $this->drupalCreateUser([
+ $grandChild1User = $this->drupalCreateUser([
'access parent test page',
'access child1 test page',
'access child2 test page',
- 'access super child1 test page',
+ 'access grand child1 test page',
]);
- $superChild2User = $this->drupalCreateUser([
+ $grandChild2User = $this->drupalCreateUser([
'access parent test page',
'access child1 test page',
'access child2 test page',
- 'access super child2 test page',
+ 'access grand child2 test page',
]);
- $superChild3User = $this->drupalCreateUser([
+ $grandChild3User = $this->drupalCreateUser([
'access parent test page',
'access child1 test page',
'access child2 test page',
- 'access super child3 test page',
+ 'access grand child3 test page',
+ ]);
+ $greatGrandChild1User = $this->drupalCreateUser([
+ 'access parent test page',
+ 'access child1 test page',
+ 'access grand child1 test page',
+ 'access great grand child1 test page',
]);
$noParentAccessUser = $this->drupalCreateUser([
'access child1 test page',
'access child2 test page',
- 'access super child1 test page',
- 'access super child2 test page',
- 'access super child3 test page',
+ 'access grand child1 test page',
+ 'access grand child2 test page',
+ 'access grand child3 test page',
+ 'access great grand child1 test page',
]);
- // Users that do not have access to any of the 'super_child' routes will
+ // Users that do not have access to any of the 'grand_child' routes will
// not have access to any of the routes in the tree.
- $this->assertUserRoutesAccess($parentUser, [], ...$tree_routes);
- $this->assertUserRoutesAccess($childOnlyUser, [], ...$tree_routes);
-
+ $this->assertUserRoutesAccess($parentUser, [], $tree_routes);
+ $this->assertUserRoutesAccess($childOnlyUser, [], $tree_routes);
// A user that does not have access to the top level parent but has access
// to all the other routes will have access to all routes except the parent
- // and 'menu_test.child3_test', because it has no items underneath in the
- // menu.
+ // and 'menu_test.child3_test_block', because it has no items underneath in
+ // the menu.
$this->assertUserRoutesAccess(
$noParentAccessUser,
- array_diff($tree_routes, ['menu_test.parent_test', 'menu_test.child3_test']),
- ...$tree_routes
+ array_diff($tree_routes, [
+ 'menu_test.parent_test',
+ 'menu_test.child3_test_block',
+ 'menu_test.child4_test_overview',
+ ]),
+ $tree_routes
);
- // Users who have only access to one super child route should have access
- // only to that route and its parents.
+
+ // Route using overview should have access to the grand child to access the
+ // current route.
$this->assertUserRoutesAccess(
- $superChild1User,
- ['menu_test.parent_test', 'menu_test.child1_test', 'menu_test.super_child1_test'],
- ...$tree_routes);
+ $grandChild1User,
+ [],
+ $tree_routes);
+ $this->assertUserRoutesAccess(
+ $greatGrandChild1User, [
+ 'menu_test.parent_test',
+ 'menu_test.child1_test',
+ 'menu_test.grand_child1_test',
+ 'menu_test.great_grand_child1_test',
+ ],
+ $tree_routes);
+ // Users who have only access to one grand child route should have access
+ // only to that route and its parents.
$this->assertUserRoutesAccess(
- $superChild2User,
- ['menu_test.parent_test', 'menu_test.child2_test', 'menu_test.super_child2_test'],
- ...$tree_routes);
+ $grandChild2User,
+ ['menu_test.parent_test', 'menu_test.child2_test', 'menu_test.grand_child2_test'],
+ $tree_routes);
$this->assertUserRoutesAccess(
- $superChild3User,
- // The 'menu_test.super_child3_test' menu item is nested under
+ $grandChild3User,
+ // The 'menu_test.grand_child3_test' menu item is nested under
// 'menu_test.child2_test' to ensure access is correct when there are
// multiple items nested at the same level.
- ['menu_test.parent_test', 'menu_test.child2_test', 'menu_test.super_child3_test'],
- ...$tree_routes);
-
+ ['menu_test.parent_test', 'menu_test.child2_test', 'menu_test.grand_child3_test'],
+ $tree_routes);
// Test a route that has parameter defined in the menu item.
$this->drupalLogin($parentUser);
$this->assertMenuItemRoutesAccess(403, Url::fromRoute('menu_test.parent_test_param', ['param' => 'param-in-menu']));
@@ -285,29 +313,60 @@ public function testSystemAdminMenuBlockAccessCheck(): void {
private function assertMenuItemRoutesAccess(int $expected_status, string|Url ...$paths): void {
foreach ($paths as $path) {
$this->drupalGet($path);
- $this->assertSession()->statusCodeEquals($expected_status);
- $this->assertSession()->pageTextNotContains('You do not have any administrative items.');
+ if (!is_string($path)) {
+ $path = $path->toString();
+ }
+ // We don't use \Behat\Mink\WebAssert::statusCodeEquals() here because it
+ // would not allow us to know which path failed.
+ $this->assertSame($expected_status, $this->getSession()->getStatusCode(), "Route $path has expected status code");
}
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function drupalGet($path, array $options = [], array $headers = []) {
+ $return = parent::drupalGet($path, $options, $headers);
+ $this->assertSession()->pageTextNotContains('You do not have any administrative items.');
+ return $return;
+ }
+
/**
* Asserts which routes a user has access to.
*
* @param \Drupal\Core\Session\AccountInterface $user
* The user account for which to check access.
- * @param array $accessibleRoutes
+ * @param array $expectedAccessibleRoutes
* The routes the user should have access to.
- * @param string ...$allRoutes
+ * @param array $allRoutes
* The routes to check.
*/
- private function assertUserRoutesAccess(AccountInterface $user, array $accessibleRoutes, string ...$allRoutes): void {
+ private function assertUserRoutesAccess(AccountInterface $user, array $expectedAccessibleRoutes, array $allRoutes): void {
$this->drupalLogin($user);
+ $expectedInaccessibleRoutes = array_diff($allRoutes, $expectedAccessibleRoutes);
+ $this->assertEmpty(array_diff($expectedAccessibleRoutes, $allRoutes));
+ $actualAccessibleRoutes = [];
+ $actualInaccessibleRoutes = [];
foreach ($allRoutes as $route) {
- $this->assertMenuItemRoutesAccess(
- in_array($route, $accessibleRoutes, TRUE) ? 200 : 403,
- Url::fromRoute($route)
- );
+ $this->drupalGet(Url::fromRoute($route));
+ switch ($this->getSession()->getStatusCode()) {
+ case 200:
+ $actualAccessibleRoutes[] = $route;
+ break;
+
+ case 403:
+ $actualInaccessibleRoutes[] = $route;
+ break;
+
+ default:
+ throw new \UnexpectedValueException("Unexpected status code {$this->getStatus()} for route $route");
+
+ }
}
+ $debug = fn($accessibleRoutes, $inaccessibleRoutes) => "\nAccessible routes: " . implode(', ', $accessibleRoutes) . "\nInaccessible routes: " . implode(', ', $inaccessibleRoutes);
+ $expected = $debug($expectedAccessibleRoutes, $expectedInaccessibleRoutes);
+ $actual = $debug($actualAccessibleRoutes, $actualInaccessibleRoutes);
+ $this->assertSession()->assert($expected === $actual, "Routes do not match. \nExpected routes:$expected\nActual routes: $actual");
}
}