From bcaa369cb824848aab901e1d9c1c0339bc78ed5c Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Tue, 29 Aug 2023 09:10:49 +0100 Subject: [PATCH] Issue #296693 by tedbow, omkar.podey, sun, ilya.no, JeroenT, tim.plunkett, catch, hooroomoo, ridhimaabrol24, boombatower, paulocs, Damien Tournoud, Xano, pwolanin, chx, webchick, stella, quietone, Bojhan, smustgrave, lauriii, benjifisher, gabesullice: Restrict access to empty top level administration pages --- .../ModeratedContentLocalTaskTest.php | 3 +- .../src/Functional/LanguageBreadcrumbTest.php | 2 +- .../SystemAdminMenuBlockAccessCheck.php | 107 ++++++++ .../src/Controller/SystemController.php | 3 + .../AccessRouteAlterSubscriber.php | 42 +++ core/modules/system/system.routing.yml | 3 +- core/modules/system/system.services.yml | 9 + core/modules/system/tests/http.php | 5 +- .../menu_test/menu_test.links.menu.yml | 75 ++++++ .../menu_test/menu_test.permissions.yml | 18 ++ .../modules/menu_test/menu_test.routing.yml | 80 ++++++ .../src/Functional/Menu/MenuAccessTest.php | 241 +++++++++++++++++- .../SecurityAdvisoryTest.php | 3 + .../Functional/Session/SessionHttpsTest.php | 2 +- .../src/Functional/ToolbarAdminMenuTest.php | 5 + .../Functional/ToolbarMenuTranslationTest.php | 2 + .../tests/src/Nightwatch/Tests/toolbarTest.js | 12 +- .../src/Functional/UpdateSemverCoreTest.php | 2 +- .../Drupal/Nightwatch/Tests/loginTest.js | 2 +- 19 files changed, 599 insertions(+), 17 deletions(-) create mode 100644 core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php create mode 100644 core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php create mode 100644 core/modules/system/tests/modules/menu_test/menu_test.permissions.yml diff --git a/core/modules/content_moderation/tests/src/Functional/ModeratedContentLocalTaskTest.php b/core/modules/content_moderation/tests/src/Functional/ModeratedContentLocalTaskTest.php index 64cc55f1b02e..7f1bd1acb5cb 100644 --- a/core/modules/content_moderation/tests/src/Functional/ModeratedContentLocalTaskTest.php +++ b/core/modules/content_moderation/tests/src/Functional/ModeratedContentLocalTaskTest.php @@ -64,8 +64,7 @@ public function testModeratedContentLocalTask() { // Verify the moderated content local task does not exist without the node // module installed. $this->drupalGet('admin/content'); - $this->assertSession()->statusCodeEquals(200); - $this->assertSession()->linkNotExists('Moderated content'); + $this->assertSession()->statusCodeEquals(403); } } diff --git a/core/modules/language/tests/src/Functional/LanguageBreadcrumbTest.php b/core/modules/language/tests/src/Functional/LanguageBreadcrumbTest.php index 21c9e5916b36..9c91eea834e3 100644 --- a/core/modules/language/tests/src/Functional/LanguageBreadcrumbTest.php +++ b/core/modules/language/tests/src/Functional/LanguageBreadcrumbTest.php @@ -48,7 +48,7 @@ public function testBreadCrumbs() { $this->assertBreadcrumb('de/user/login', []); $this->assertBreadcrumb('gsw-berne/user/login', []); - $admin_user = $this->drupalCreateUser(['access administration pages']); + $admin_user = $this->drupalCreateUser(['access administration pages', 'administer blocks']); $this->drupalLogin($admin_user); // Use administration routes to assert that breadcrumb is displayed diff --git a/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php b/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php new file mode 100644 index 000000000000..a527605ea737 --- /dev/null +++ b/core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php @@ -0,0 +1,107 @@ +<?php + +namespace Drupal\system\Access; + +use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Menu\MenuLinkInterface; +use Drupal\Core\Menu\MenuLinkManagerInterface; +use Drupal\Core\Menu\MenuLinkTreeInterface; +use Drupal\Core\Menu\MenuTreeParameters; +use Drupal\Core\Routing\Access\AccessInterface; +use Drupal\Core\Routing\AccessAwareRouter; +use Drupal\Core\Routing\RouteMatchInterface; +use Drupal\Core\Session\AccountInterface; + +/** + * Access check for routes implementing _access_admin_menu_block_page. + * + * @see \Drupal\system\EventSubscriber\AccessRouteAlterSubscriber + * @see \Drupal\system\Controller\SystemController::systemAdminMenuBlockPage() + */ +class SystemAdminMenuBlockAccessCheck implements AccessInterface { + + /** + * Constructs a new SystemAdminMenuBlockAccessCheck. + * + * @param \Drupal\Core\Access\AccessManagerInterface $accessManager + * The access manager. + * @param \Drupal\Core\Menu\MenuLinkTreeInterface $menuLinkTree + * The menu link tree service. + * @param \Drupal\Core\Routing\AccessAwareRouter $router + * The router service. + * @param \Drupal\Core\Menu\MenuLinkManagerInterface $menuLinkManager + * The menu link manager service. + */ + public function __construct( + private readonly AccessManagerInterface $accessManager, + private readonly MenuLinkTreeInterface $menuLinkTree, + private readonly AccessAwareRouter $router, + private readonly MenuLinkManagerInterface $menuLinkManager, + ) { + } + + /** + * Checks access. + * + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The cron key. + * @param \Drupal\Core\Session\AccountInterface $account + * The current user. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + */ + public function access(RouteMatchInterface $route_match, AccountInterface $account): AccessResultInterface { + $parameters = $route_match->getParameters()->all(); + // Load links in the 'admin' menu matching this route. + $links = $this->menuLinkManager->loadLinksByRoute($route_match->getRouteName(), $parameters, 'admin'); + if (empty($links)) { + // If we did not find a link then we have no opinion on access. + return AccessResult::neutral(); + } + return $this->hasAccessToChildMenuItems(reset($links), $account)->cachePerPermissions(); + } + + /** + * Check that the given route has access to one of it's child routes. + * + * @param \Drupal\Core\Menu\MenuLinkInterface $link + * The menu link. + * @param \Drupal\Core\Session\AccountInterface $account + * The account. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + */ + protected function hasAccessToChildMenuItems(MenuLinkInterface $link, AccountInterface $account): AccessResultInterface { + $parameters = new MenuTreeParameters(); + $parameters->setRoot($link->getPluginId()) + ->excludeRoot() + ->setTopLevelOnly() + ->onlyEnabledLinks(); + + $tree = $this->menuLinkTree->load(NULL, $parameters); + + 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::neutral(); + } + + foreach ($tree as $element) { + if (!$this->accessManager->checkNamedRoute($element->link->getRouteName(), $element->link->getRouteParameters(), $account)) { + continue; + } + + // If access is allowed to this element in the tree check for access to + // its own children. + return AccessResult::allowedIf($this->hasAccessToChildMenuItems($element->link, $account)->isAllowed()); + } + return AccessResult::neutral(); + } + +} diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index 8a9d3dbc45ae..afb715dad9a1 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -109,6 +109,9 @@ public static function create(ContainerInterface $container) { /** * Provide the administration overview page. * + * This will render child links two levels below the specified link ID, + * grouped by the child links one level below. + * * @param string $link_id * The ID of the administrative path link for which to display child links. * diff --git a/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php b/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php new file mode 100644 index 000000000000..5bc438442bc4 --- /dev/null +++ b/core/modules/system/src/EventSubscriber/AccessRouteAlterSubscriber.php @@ -0,0 +1,42 @@ +<?php + +namespace Drupal\system\EventSubscriber; + +use Drupal\Core\Routing\RouteBuildEvent; +use Drupal\Core\Routing\RoutingEvents; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * Alters routes to add necessary requirements. + * + * @see \Drupal\system\Access\SystemAdminMenuBlockAccessCheck + * @see \Drupal\system\Controller\SystemController::systemAdminMenuBlockPage() + */ +class AccessRouteAlterSubscriber implements EventSubscriberInterface { + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + $events[RoutingEvents::ALTER][] = 'accessAdminMenuBlockPage'; + return $events; + } + + /** + * Adds _access_admin_menu_block_page requirement to routes pointing to SystemController::systemAdminMenuBlockPage. + * + * @param \Drupal\Core\Routing\RouteBuildEvent $event + * The event to process. + */ + public function accessAdminMenuBlockPage(RouteBuildEvent $event) { + $routes = $event->getRouteCollection(); + 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'); + } + } + } + +} diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml index 07de084f0349..5a7c75e495f6 100644 --- a/core/modules/system/system.routing.yml +++ b/core/modules/system/system.routing.yml @@ -515,8 +515,7 @@ system.db_update: system.admin_content: path: '/admin/content' defaults: - _controller: '\Drupal\system\Controller\SystemController::overview' - link_id: 'system.admin_content' + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' _title: 'Content' requirements: _permission: 'access administration pages' diff --git a/core/modules/system/system.services.yml b/core/modules/system/system.services.yml index a10504025569..7754d1437285 100644 --- a/core/modules/system/system.services.yml +++ b/core/modules/system/system.services.yml @@ -7,6 +7,11 @@ services: class: Drupal\system\Access\DbUpdateAccessCheck tags: - { name: access_check, applies_to: _access_system_update } + access_check.admin_menu_block_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_menu_block_page } system.manager: class: Drupal\system\SystemManager arguments: ['@module_handler', '@request_stack', '@menu.link_tree', '@menu.active_trail'] @@ -77,3 +82,7 @@ services: arguments: ['@menu.link_tree', '@system.module_admin_links_memory_cache'] system.module_admin_links_memory_cache: class: Drupal\Core\Cache\MemoryCache\MemoryCache + system.access_route_alter_subscriber: + class: Drupal\system\EventSubscriber\AccessRouteAlterSubscriber + tags: + - { name: event_subscriber } diff --git a/core/modules/system/tests/http.php b/core/modules/system/tests/http.php index 6d822ec88f08..2eb23a9f3aba 100644 --- a/core/modules/system/tests/http.php +++ b/core/modules/system/tests/http.php @@ -16,8 +16,9 @@ $_SERVER['HTTPS'] = NULL; ini_set('session.cookie_secure', FALSE); foreach ($_SERVER as &$value) { - $value = str_replace('core/modules/system/tests/http.php', 'index.php', $value); - $value = str_replace('https://', 'http://', $value); + // Because HTTPS is null. + $value = $value ? str_replace('core/modules/system/tests/http.php', 'index.php', $value) : ""; + $value = $value ? str_replace('https://', 'http://', $value) : ""; } $kernel = new TestKernel('testing', $autoloader, TRUE); 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 9ba1b8f1bf4f..d694e419b85a 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 @@ -15,6 +15,81 @@ menu_test.menu_callback_description.description-plain: description: 'Menu item description text' route_name: menu_test.callback_description_plain parent: menu_test.menu_callback_description +menu_test.parent_test: + title: 'Menu Parent' + description: 'Menu item description parent' + route_name: menu_test.parent_test + parent: system.admin +menu_test.parent_test.child1_test: + title: 'Menu child1' + description: 'Menu item description child1' + route_name: menu_test.child1_test + parent: menu_test.parent_test +menu_test.parent_test.child2_test: + title: 'Menu child2' + description: 'Menu item description child2' + route_name: menu_test.child2_test + parent: menu_test.parent_test +menu_test.parent_test.child3_test: + title: 'Menu child3' + description: 'Menu item description child3' + route_name: menu_test.child3_test + 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 + 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 + 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 + parent: menu_test.parent_test.child2_test +menu_test.menu_parent_test_param: + title: 'Menu Parent Param' + description: 'Menu item description parent' + route_name: menu_test.parent_test_param + parent: system.admin + route_parameters: + param: 'param-in-menu' +menu_test.menu_parent_test.child_test_param: + title: 'Menu Child Param' + description: 'Menu item description child' + route_name: menu_test.child_test_param + parent: menu_test.menu_parent_test_param + route_parameters: + param: 'param-in-menu' +menu_test.menu_parent_test_param_default: + title: 'Menu Parent Param Default' + description: 'Menu item description parent' + route_name: menu_test.parent_test_param + parent: system.admin + route_parameters: + param: 'child_uses_default' +menu_test.menu_parent_test.child_test_param_default: + title: 'Menu Child Param Default' + description: 'Menu item description child' + route_name: menu_test.child_test_param + parent: menu_test.menu_parent_test_param_default +menu_test.menu_parent_test_param_default_explicit: + title: 'Menu Parent Param Default Explicit' + description: 'Menu item description parent' + route_name: menu_test.parent_test_param_explicit + parent: system.admin + route_parameters: + param: 'my_default' +menu_test.menu_parent_test.child_test_param_default_explicit: + title: 'Menu Child Param Default Explicit' + description: 'Menu item description child' + route_name: menu_test.child_test_param_explicit + parent: menu_test.menu_parent_test_param_default_explicit + route_parameters: + param: 'my_default' menu_test.menu_no_title_callback: title: 'A title with @placeholder' route_name: menu_test.menu_no_title_callback 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 new file mode 100644 index 000000000000..47c528f01254 --- /dev/null +++ b/core/modules/system/tests/modules/menu_test/menu_test.permissions.yml @@ -0,0 +1,18 @@ +access parent test page: + title: 'Access parent test page' + restrict access: true +access child1 test page: + title: 'Access child1 test page' + restrict access: true +access child2 test page: + title: 'Access child2 test page' + restrict access: true +access super child1 test page: + title: 'Access super child1 test page' + restrict access: true +access super child2 test page: + title: 'Access super child2 test page' + restrict access: true +access super child3 test page: + title: 'Access super child3 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 59086d156cbd..5b83aeb2af65 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 @@ -38,6 +38,86 @@ menu_test.callback_description_plain: requirements: _access: 'TRUE' +menu_test.parent_test: + path: '/parent_test' + defaults: + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' + requirements: + _permission: 'access parent test page' + +menu_test.child1_test: + path: '/parent_test/child1_test' + defaults: + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' + requirements: + _permission: 'access child1 test page' + +menu_test.child2_test: + path: '/parent_test/child2_test' + defaults: + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' + requirements: + _permission: 'access child2 test page' + +menu_test.child3_test: + 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' + 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' + defaults: + _controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage' + requirements: + _permission: 'access super child2 test page' + +menu_test.super_child3_test: + path: '/parent_test/child_test/super_child3_test' + defaults: + _controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage' + requirements: + _permission: 'access super child3 test page' + +menu_test.parent_test_param: + path: '/parent_test_param/{param}' + defaults: + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' + param: 'child_uses_default' + requirements: + _permission: 'access parent test page' + +menu_test.child_test_param: + path: '/parent_test_child_test_param/{param}' + defaults: + _controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage' + param: 'my_default' + requirements: + _permission: 'access child1 test page' + +menu_test.parent_test_param_explicit: + path: '/parent_test_param_explicit/{param}' + defaults: + _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage' + param: 'my_default' + requirements: + _permission: 'access parent test page' + +menu_test.child_test_param_explicit: + path: '/parent_test_child_test_param_explicit/{param}' + defaults: + _controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPage' + param: 'my_default' + requirements: + _permission: 'access child1 test page' + menu_test.menu_no_title_callback: path: '/menu_no_title_callback' defaults: diff --git a/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php b/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php index ac85028b51d0..307fbf361300 100644 --- a/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php +++ b/core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\system\Functional\Menu; +use Drupal\Core\Session\AccountInterface; use Drupal\Core\Url; use Drupal\Tests\BrowserTestBase; @@ -17,7 +18,7 @@ class MenuAccessTest extends BrowserTestBase { * * @var array */ - protected static $modules = ['block', 'menu_test']; + protected static $modules = ['block', 'filter', 'menu_test', 'toolbar']; /** * {@inheritdoc} @@ -71,4 +72,242 @@ public function testMenuBlockLinksAccessCheck() { $this->assertSession()->linkByHrefNotExists('foo/asdf/c'); } + /** + * Test routes implementing _access_admin_menu_block_page. + * + * @covers \Drupal\system\EventSubscriber\AccessRouteAlterSubscriber::accessAdminMenuBlockPage + * @covers \Drupal\system\Access\SystemAdminMenuBlockAccessCheck::access + */ + public function testSystemAdminMenuBlockAccessCheck(): void { + // Create an admin user. + $adminUser = $this->drupalCreateUser([], NULL, TRUE); + + // Create a user with 'administer menu' permission. + $menuAdmin = $this->drupalCreateUser([ + 'access administration pages', + 'administer menu', + ]); + + // Create a user with 'administer filters' permission. + $filterAdmin = $this->drupalCreateUser([ + 'access administration pages', + 'administer filters', + ]); + + // Create a user with 'access administration pages' permission. + $webUser = $this->drupalCreateUser([ + 'access administration pages', + ]); + + // An admin user has access to all parent pages. + $this->drupalLogin($adminUser); + $this->assertMenuItemRoutesAccess(200, 'admin/structure', 'admin/people'); + + // This user has access to administer menus so the structure parent page + // should be accessible. + $this->drupalLogin($menuAdmin); + $this->assertMenuItemRoutesAccess(200, 'admin/structure'); + $this->assertMenuItemRoutesAccess(403, 'admin/people'); + + // This user has access to administer filters so the config parent page + // should be accessible. + $this->drupalLogin($filterAdmin); + $this->assertMenuItemRoutesAccess(200, 'admin/config'); + $this->assertMenuItemRoutesAccess(403, 'admin/people'); + + // 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'); + + // 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.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 + // '_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 + // `_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', + ]; + + // Create a user with access to only the top level parent. + $parentUser = $this->drupalCreateUser([ + 'access parent test page', + ]); + // Create a user with access to the parent and child routes but none of the + // super 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 + // child route. + $superChild1User = $this->drupalCreateUser([ + 'access parent test page', + 'access child1 test page', + 'access child2 test page', + 'access super child1 test page', + ]); + $superChild2User = $this->drupalCreateUser([ + 'access parent test page', + 'access child1 test page', + 'access child2 test page', + 'access super child2 test page', + ]); + $superChild3User = $this->drupalCreateUser([ + 'access parent test page', + 'access child1 test page', + 'access child2 test page', + 'access super child3 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', + ]); + + // Users that do not have access to any of the 'super_child' routes will + // not have access to any of the routes in the tree. + $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. + $this->assertUserRoutesAccess( + $noParentAccessUser, + array_diff($tree_routes, ['menu_test.parent_test', 'menu_test.child3_test']), + ...$tree_routes + ); + // Users who have only access to one super child route should have access + // only to that route and its parents. + $this->assertUserRoutesAccess( + $superChild1User, + ['menu_test.parent_test', 'menu_test.child1_test', 'menu_test.super_child1_test'], + ...$tree_routes); + $this->assertUserRoutesAccess( + $superChild2User, + ['menu_test.parent_test', 'menu_test.child2_test', 'menu_test.super_child2_test'], + ...$tree_routes); + $this->assertUserRoutesAccess( + $superChild3User, + // The 'menu_test.super_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); + + // 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'])); + $this->drupalLogin($childOnlyUser); + $this->assertMenuItemRoutesAccess(200, Url::fromRoute('menu_test.parent_test_param', ['param' => 'param-in-menu'])); + + // Test a route that does not have a parameter defined in the menu item but + // uses the route default parameter. + // @todo Change the following test case to use a parent menu item that also + // uses the routes default parameter in https://drupal.org/i/3359511. + $this->drupalLogin($parentUser); + $this->assertMenuItemRoutesAccess( + 403, + Url::fromRoute('menu_test.parent_test_param', ['param' => 'child_uses_default']), + Url::fromRoute('menu_test.child_test_param', ['param' => 'child_uses_default']), + + ); + $this->drupalLogin($childOnlyUser); + $this->assertMenuItemRoutesAccess( + 200, + Url::fromRoute('menu_test.parent_test_param', ['param' => 'child_uses_default']), + Url::fromRoute('menu_test.child_test_param', ['param' => 'child_uses_default']), + ); + + // Test a route that does have a parameter defined in the menu item and that + // parameter value is equal to the default value specific in the route. + $this->drupalLogin($parentUser); + $this->assertMenuItemRoutesAccess( + 403, + Url::fromRoute('menu_test.parent_test_param_explicit', ['param' => 'my_default']), + Url::fromRoute('menu_test.child_test_param_explicit', ['param' => 'my_default']) + ); + $this->drupalLogin($childOnlyUser); + $this->assertMenuItemRoutesAccess( + 200, + Url::fromRoute('menu_test.parent_test_param_explicit', ['param' => 'my_default']), + Url::fromRoute('menu_test.child_test_param_explicit', ['param' => 'my_default']) + ); + + // If we try to access a route that takes a parameter but route is not in the + // with that parameter we should always be denied access because the sole + // purpose of \Drupal\system\Controller\SystemController::systemAdminMenuBlockPage + // is to display items in the menu. + $this->drupalLogin($parentUser); + $this->assertMenuItemRoutesAccess( + 403, + Url::fromRoute('menu_test.parent_test_param', ['param' => 'any-other']), + // $parentUser does not have the 'access child1 test page' permission. + Url::fromRoute('menu_test.child_test_param', ['param' => 'any-other']) + ); + $this->drupalLogin($childOnlyUser); + $this->assertMenuItemRoutesAccess(403, Url::fromRoute('menu_test.parent_test_param', ['param' => 'any-other'])); + // $childOnlyUser has the 'access child1 test page' permission. + $this->assertMenuItemRoutesAccess(200, Url::fromRoute('menu_test.child_test_param', ['param' => 'any-other'])); + } + + /** + * Asserts route requests connected to menu items have the expected access. + * + * @param int $expected_status + * The expected request status. + * @param string|\Drupal\Core\Url ...$paths + * The paths as passed to \Drupal\Tests\UiHelperTrait::drupalGet(). + */ + 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.'); + } + } + + /** + * 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 + * The routes the user should have access to. + * @param string ...$allRoutes + * The routes to check. + */ + private function assertUserRoutesAccess(AccountInterface $user, array $accessibleRoutes, string ...$allRoutes): void { + $this->drupalLogin($user); + foreach ($allRoutes as $route) { + $this->assertMenuItemRoutesAccess( + in_array($route, $accessibleRoutes, TRUE) ? 200 : 403, + Url::fromRoute($route) + ); + } + } + } diff --git a/core/modules/system/tests/src/Functional/SecurityAdvisories/SecurityAdvisoryTest.php b/core/modules/system/tests/src/Functional/SecurityAdvisories/SecurityAdvisoryTest.php index 9e0a91b29275..aeb9c507c83e 100644 --- a/core/modules/system/tests/src/Functional/SecurityAdvisories/SecurityAdvisoryTest.php +++ b/core/modules/system/tests/src/Functional/SecurityAdvisories/SecurityAdvisoryTest.php @@ -147,6 +147,9 @@ public function testPsa(): void { // advisories on admin pages. $this->drupalLogin($this->drupalCreateUser([ 'access administration pages', + // We have nothing under admin, so we need access to a child route to + // access the parent. + 'administer modules', ])); $this->assertAdvisoriesNotDisplayed($mixed_advisory_links, ['system.admin']); diff --git a/core/modules/system/tests/src/Functional/Session/SessionHttpsTest.php b/core/modules/system/tests/src/Functional/Session/SessionHttpsTest.php index a300f3c4436a..819a3fda94ec 100644 --- a/core/modules/system/tests/src/Functional/Session/SessionHttpsTest.php +++ b/core/modules/system/tests/src/Functional/Session/SessionHttpsTest.php @@ -64,7 +64,7 @@ protected function setUp(): void { * Tests HTTPS sessions. */ public function testHttpsSession() { - $user = $this->drupalCreateUser(['access administration pages']); + $user = $this->drupalCreateUser(['access administration pages', 'administer site configuration']); /** @var \Symfony\Component\BrowserKit\CookieJar $browser_kit_cookie_jar */ $browser_kit_cookie_jar = $this->getSession()->getDriver()->getClient()->getCookieJar(); diff --git a/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php b/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php index 2d69089d64ed..08a7644d1b3c 100644 --- a/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php +++ b/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php @@ -65,6 +65,7 @@ class ToolbarAdminMenuTest extends BrowserTestBase { 'language', 'test_page_test', 'locale', + 'search', ]; /** @@ -95,6 +96,7 @@ protected function setUp(): void { 'administer taxonomy', 'administer languages', 'translate interface', + 'administer search', ]; // Create an administrative user and log it in. @@ -329,6 +331,8 @@ public function testLocaleTranslationSubtreesHashCacheClear() { // should create a new menu hash if the toolbar subtrees cache is correctly // invalidated. $this->drupalLogin($translate_user); + // We need to visit the page to get the string to be translated. + $this->drupalGet($langcode . '/admin/config'); $search = [ 'string' => 'Search and metadata', 'langcode' => $langcode, @@ -358,6 +362,7 @@ public function testLocaleTranslationSubtreesHashCacheClear() { // of the link items in the Structure tree (Menus) has had its text // translated. $this->drupalLogin($admin_user); + $this->drupalGet('admin/config'); // Have the adminUser request a page in the new language. $this->drupalGet($langcode . '/test-page'); $this->assertSession()->statusCodeEquals(200); diff --git a/core/modules/toolbar/tests/src/Functional/ToolbarMenuTranslationTest.php b/core/modules/toolbar/tests/src/Functional/ToolbarMenuTranslationTest.php index 3079352f8e5d..be99b93902c4 100644 --- a/core/modules/toolbar/tests/src/Functional/ToolbarMenuTranslationTest.php +++ b/core/modules/toolbar/tests/src/Functional/ToolbarMenuTranslationTest.php @@ -28,6 +28,7 @@ class ToolbarMenuTranslationTest extends BrowserTestBase { 'toolbar_test', 'locale', 'locale_test', + 'block', ]; /** @@ -47,6 +48,7 @@ protected function setUp(): void { 'translate interface', 'administer languages', 'access administration pages', + 'administer blocks', ]); $this->drupalLogin($this->adminUser); } diff --git a/core/modules/toolbar/tests/src/Nightwatch/Tests/toolbarTest.js b/core/modules/toolbar/tests/src/Nightwatch/Tests/toolbarTest.js index e23488c900a8..91773a7dac9a 100644 --- a/core/modules/toolbar/tests/src/Nightwatch/Tests/toolbarTest.js +++ b/core/modules/toolbar/tests/src/Nightwatch/Tests/toolbarTest.js @@ -99,23 +99,23 @@ module.exports = { 'is-active toolbar-tray-vertical', ); browser.waitForElementPresent( - '#toolbar-item-administration-tray li:nth-child(4) button', + '#toolbar-item-administration-tray li:nth-child(2) button', ); browser.assert.not.hasClass( - '#toolbar-item-administration-tray li:nth-child(4)', + '#toolbar-item-administration-tray li:nth-child(2)', 'open', ); browser.assert.not.hasClass( - '#toolbar-item-administration-tray li:nth-child(4) button', + '#toolbar-item-administration-tray li:nth-child(2) button', 'open', ); - browser.click('#toolbar-item-administration-tray li:nth-child(4) button'); + browser.click('#toolbar-item-administration-tray li:nth-child(2) button'); browser.assert.hasClass( - '#toolbar-item-administration-tray li:nth-child(4)', + '#toolbar-item-administration-tray li:nth-child(2)', 'open', ); browser.assert.hasClass( - '#toolbar-item-administration-tray li:nth-child(4) button', + '#toolbar-item-administration-tray li:nth-child(2) button', 'open', ); browser.expect diff --git a/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php b/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php index cd87ee90bc80..8fe8fbe3f24c 100644 --- a/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php +++ b/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php @@ -523,7 +523,7 @@ public function testBrokenThenFixedUpdates() { \Drupal::keyValueExpirable('update_available_releases')->deleteAll(); // This cron run should retrieve fixed updates. $this->cronRun(); - $this->drupalGet('admin/structure'); + $this->drupalGet('admin/config'); $this->assertSession()->statusCodeEquals(200); $this->assertSession()->pageTextContains('There is a security update available for your version of Drupal.'); } diff --git a/core/tests/Drupal/Nightwatch/Tests/loginTest.js b/core/tests/Drupal/Nightwatch/Tests/loginTest.js index 3eb398f8a937..500ed7846c61 100644 --- a/core/tests/Drupal/Nightwatch/Tests/loginTest.js +++ b/core/tests/Drupal/Nightwatch/Tests/loginTest.js @@ -13,7 +13,7 @@ module.exports = { .drupalCreateUser({ name: 'user', password: '123', - permissions: ['access site reports'], + permissions: ['access site reports', 'administer site configuration'], }) .drupalLogin({ name: 'user', password: '123' }) .drupalRelativeURL('/admin/reports') -- GitLab