Commit 3ed2ee36 authored by catch's avatar catch
Browse files

Issue #3522754 by hfernandes, catch, joaopauloc.dev, alexpott: Access to... 

Issue #3522754 by hfernandes, catch, joaopauloc.dev, alexpott: Access to systemAdminMenuBlockPage is denied if it contains links using the url attribute

(cherry picked from commit fb06e64f)
parent 39235c94

core/modules/system/src/Access/SystemAdminMenuBlockAccessCheck.php

+9 −6
Original line number Diff line number Diff line
@@ -2,7 +2,6 @@ 


namespace Drupal\system\Access;



use Drupal\Core\Access\AccessManagerInterface;

use Drupal\Core\Access\AccessResult;

use Drupal\Core\Access\AccessResultInterface;

use Drupal\Core\Menu\MenuLinkInterface;
@@ -25,8 +24,6 @@ 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
@@ -35,7 +32,6 @@ class SystemAdminMenuBlockAccessCheck implements AccessInterface { 
   *   The menu link manager service.

   */

  public function __construct(

    private readonly AccessManagerInterface $accessManager,

    private readonly MenuLinkTreeInterface $menuLinkTree,

    private readonly AccessAwareRouter $router,

    private readonly MenuLinkManagerInterface $menuLinkManager,
@@ -97,13 +93,20 @@ protected function hasAccessToChildMenuItems(MenuLinkInterface $link, AccountInt 
      ->setTopLevelOnly()

      ->onlyEnabledLinks();



    $route = $this->router->getRouteCollection()->get($link->getRouteName());

    $link_url = $link->getUrlObject();

    if (!$link_url->isRouted()) {

      // If the link is not routed, we cannot check access to it.

      return AccessResult::neutral();

    }



    $route = $this->router->getRouteCollection()->get($link_url->getRouteName());

    if ($route && empty($route->getRequirement('_access_admin_menu_block_page')) && empty($route->getRequirement('_access_admin_overview_page'))) {

      return AccessResult::allowed();

    }



    foreach ($this->menuLinkTree->load(NULL, $parameters) as $element) {

      if (!$this->accessManager->checkNamedRoute($element->link->getRouteName(), $element->link->getRouteParameters(), $account)) {

      // Skip the link if the user does not have access.

      if (!$element->link->getUrlObject()->access($account)) {

        continue;

      }

core/modules/system/system.services.yml

+2 −2
Original line number Diff line number Diff line
@@ -11,12 +11,12 @@ services: 
      - { 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']

    arguments: ['@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' ]

    arguments: ['@menu.link_tree', '@router', '@plugin.manager.menu.link' ]

    tags:

      - { name: access_check, applies_to: _access_admin_overview_page }

  system.manager:

core/modules/system/tests/modules/menu_test/menu_test.links.menu.yml

+10 −0
Original line number Diff line number Diff line
@@ -110,6 +110,16 @@ menu_test.menu_parent_test.child_test_param_default_explicit: 
  parent: menu_test.menu_parent_test_param_default_explicit

  route_parameters:

    param: 'my_default'

menu_test.parent_url_test:

  title: 'Menu URL Parent'

  description: 'Menu item description parent'

  route_name: menu_test.parent_url_test

  parent: system.admin

menu_test.parent_url_test.child1_test:

  title: 'Menu URL child1'

  description: 'Menu URL child1: uses a URL path'

  url: "internal:#test"

  parent: menu_test.parent_url_test

menu_test.menu_no_title_callback:

  title: 'A title with @placeholder'

  route_name: menu_test.menu_no_title_callback

core/modules/system/tests/modules/menu_test/menu_test.routing.yml

+7 −0
Original line number Diff line number Diff line
@@ -151,6 +151,13 @@ menu_test.child_test_param_explicit: 
  requirements:

    _permission: 'access child1 test page'



menu_test.parent_url_test:

  path: '/parent_url_test'

  defaults:

    _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'

  requirements:

    _permission: 'access child1 test page'



menu_test.menu_no_title_callback:

  path: '/menu_no_title_callback'

  defaults:

core/modules/system/tests/src/Functional/Menu/MenuAccessTest.php

+6 −0
Original line number Diff line number Diff line
@@ -343,6 +343,12 @@ public function testSystemAdminMenuBlockAccessCheck(): void { 
    $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']));



    // User that has access to the child route must also have access to the

    // parent route if it's a systemAdminMenuBlockPage page.

    // This test validates the cases where the child link has a url attribute.

    $this->drupalLogin($noParentAccessUser);

    $this->assertMenuItemRoutesAccess(200, Url::fromRoute('menu_test.parent_url_test'));

  }



  /**