Skip to content
Snippets Groups Projects
Commit 719f1684 authored by catch's avatar catch
Browse files

Issue #2463753 by claudiu.cristea, acbramley, gg24, anairamzap,...

Issue #2463753 by claudiu.cristea, acbramley, gg24, anairamzap, dhirendra.mishra, andypost, dawehner, swatichouhan012, smustgrave, quietone, Hardik_Patel_12, douggreen, mohrerao, gcb, kalistos, Wim Leers, pfrenssen, dww, larowlan: [regression] Do not bypass route access with 'link to any page' permissions for menu links

(cherry picked from commit fa1d8ce3)
parent a16476ec
No related branches found
No related tags found
26 merge requests!11185Issue #3477324 by andypost, alexpott: Fix usage of str_getcsv() and fgetcsv() for PHP 8.4,!10602Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu: Sub workspace does not clear,!10301Issue #3469309 by mstrelan, smustgrave, moshe weitzman: Use one-time login...,!10187Issue #3487488 by dakwamine: ExtensionMimeTypeGuesser::guessMimeType must support file names with "0" (zero) like foo.0.zip,!9944Issue #3483353: Consider making the createCopy config action optionally fail...,!9929Issue #3445469 by pooja_sharma, smustgrave: Add additional test coverage for...,!9787Resolve issue 3479427 - bootstrap barrio issue under Windows,!9742Issue #3463908 by catch, quietone: Split OptionsFieldUiTest into two,!9526Issue #3458177 by mondrake, catch, quietone, godotislate, longwave, larowlan,...,!8738Issue #3424162 by camilledavis, dineshkumarbollu, smustgrave: Claro...,!8704Make greek characters available in ckeditor5,!8597Draft: Issue #3442259 by catch, quietone, dww: Reduce time of Migrate Upgrade tests...,!8533Issue #3446962 by kim.pepper: Remove incorrectly added...,!8517Issue #3443748 by NexusNovaz, smustgrave: Testcase creates false positive,!8325Update file Sort.php,!8095Expose document root on install,!7930Resolve #3427374 "Taxonomytid viewsargumentdefault plugin",!7627Issue #3439440 by nicxvan, Binoli Lalani, longwave: Remove country support from DateFormatter,!7445Issue #3440169: When using drupalGet(), provide an associative array for $headers,!7401#3271894 Fix documented StreamWrapperInterface return types for realpath() and dirname(),!7384Add constraints to system.advisories,!7078Issue #3320569 by Spokje, mondrake, smustgrave, longwave, quietone, Lendude,...,!6622Issue #2559833 by piggito, mohit_aghera, larowlan, guptahemant, vakulrai,...,!6502Draft: Resolve #2938524 "Plach testing issue",!38582585169-10.1.x,!3226Issue #2987537: Custom menu link entity type should not declare "bundle" entity key
Pipeline #104225 passed with warnings
Pipeline: drupal

#104236

    Pipeline: drupal

    #104228

      Showing with 183 additions and 50 deletions
      ......@@ -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();
      }
      /**
      ......
      services:
      menu_ui.menu_tree_manipulators:
      class: Drupal\menu_ui\Menu\MenuUiMenuTreeManipulators
      Drupal\menu_ui\Menu\MenuUiMenuTreeManipulators: '@menu_ui.menu_tree_manipulators'
      <?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;
      }
      }
      ......@@ -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);
      ......
      ......@@ -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');
      }
      }
      ......@@ -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'));
      ......
      ......@@ -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);
      }
      /**
      ......
      ......@@ -217,6 +217,7 @@ public function testDefaultMenuTabRegression() {
      'administer menu',
      'link to any page',
      'access toolbar',
      'access administration pages',
      ]);
      $this->drupalLogin($admin_user);
      ......
      ......@@ -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());
      }
      }
      ......@@ -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);
      }
      /**
      ......
      0% Loading or .
      You are about to add 0 people to the discussion. Proceed with caution.
      Finish editing this message first!
      Please register or to comment