Unmasquerade link appears for users with the permission: 'link to any page', regardless of masquerade status.

Patch #2 works for me on D8.3.2, thanks.

+++ b/src/MasqueradeMenuLinkTree.php
@@ -0,0 +1,31 @@
+    foreach ($tree as $index => $item) {
+      if (strstr($index, 'masquerade.unmasquerade')) {

I think we should check a route name (masquerade.unmasquerade) here rather than the index. This way, we could remove all custom menu links (if there are any) as well.

Providing a patch with the mentioned change.

Nice trick but it needs clean-up and a kind of container param to enable (or somehow other way to configure)

1. +++ b/src/MasqueradeMenuLinkTree.php
   @@ -0,0 +1,31 @@
   + * @file
   + * Class MasqueradeMenuLinkTree.
   
   +++ b/src/MasqueradeServiceProvider.php
   @@ -0,0 +1,21 @@
   + * @file
   + * MasqueradeServiceProvider class.
   

   @file should be removed

2. +++ b/src/MasqueradeMenuLinkTree.php
   @@ -0,0 +1,31 @@
   +use Drupal;
   ...
   +        $masquerade = Drupal::service('masquerade');
   

   Use `\Drupal::` as inline

3. +++ b/src/MasqueradeServiceProvider.php
   @@ -0,0 +1,21 @@
   +    $definition->setClass('Drupal\masquerade\MasqueradeMenuLinkTree');
   

   MasqueradeMenuLinkTree::class

4. +++ b/src/ProxyClass/MasqueradeMenuLinkTree.php
   @@ -0,0 +1,132 @@
   + * @file
   + * Contains \Drupal\masquerade\ProxyClass\MasqueradeMenuLinkTree.
   ...
   + * This file was generated via php core/scripts/generate-proxy-class.php 'Drupal\masquerade\MasqueradeMenuLinkTree' "modules/contrib/masquerade/src".
   

   Looks it need to be regenerated to remove "@file" header

Addressed feedback from #9 and fixed code style issues.

I confirm that the patch #10 solve this issue on 8.4
Many thanks!!

Also I confirm that the patch #10 solve this issue.
Thanks.

This approach works but is very hackish cos overrides default core behavior so will not be commited until core decide a way ahead

Different approach for this issue.

Fixed new approach: menu link enabled/disabled

This patch fixes the following error:

Exception: Error: Class 'Drupal\masquerade\Plugin\Menu\Drupal' not found
Drupal\masquerade\Plugin\Menu\UnmasqueradeMenuLink::create()() (Line: 46)

(Patch #17 working for me.)

I can also confirm that the patch works as expected and also the implementation looks sensible.

Yep, looks much better now but require test coverage

I think that there is a cache issue with the current patch. To repoduce:

- Make sure caches are enabled etc.
- Log in as the user you will masquerade as later
- "Unmasquerade" link obvioulsy won't appear in the menu, but its disabled state gets cached
- Now log in as another user and masquerade as the first one
- "Unmasquerade" link is not displayed in the menu but it should be
- Now clear the caches and reload the page
- "Unmasquerade" link will appear.

I can reproduce @slashrsm's bug in #22.
Have looked at code, but nothing is jumping out at me.

(Unrelated novice cache context question – given UnmasqueradeMenuLink::getCacheContexts() returns one context only 'session.is_masquerading' - what happens if more than one user is masquerading at the same time?)

@wturrell 'session. context prefix means that this link varies per user's session flag

The root of this problem is that the menu links don't respect route access for user 1. Personally, I'd say this is a core bug, but I need to work around this in my module for several items that the administrator should not see. Unmasquerade is one of these, since I don't allow users to masquerade as administrators (including user 1).

Using #17 as inspiration, I am simply adding this class in MYMODULE.links.menu.yml to any route that should not be visible to user 1.

/**
 * @file
 * Contains \Drupal\MYMODULE\Plugin\Menu\NoAdminMenuLink.
 */

namespace Drupal\MYMODULE\Plugin\Menu;

use Drupal\Core\Menu\MenuLinkDefault;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\Session\AccountProxyInterface;

/**
 * A menu link that can be hidden from user 1.
 */
class NoAdminMenuLink extends MenuLinkDefault {

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    $plugin_definition['enabled'] = $container->get('current_user')->id() != 1;
    return parent::create($container, $configuration, $plugin_id, $plugin_definition);
  }

  /**
   * {@inheritdoc}
   */
  public function getCacheContexts() {
    return ['user.is_super_user'];
  }

}

This would need a little adapting for masquerade. Specifically, the menu should be enabled if uid != 1 || $is_masquerading.

Note that you can continue to use the yml for the title and description, and that there is no reason to override the constructor to save the masquerading status since it is never used. I am not getting caching problems with the context I'm using above and unless uid 1 can masquerade as uid 1 (and I don't think you can masquerade as yourself), I would think that context would work for masquerade.

I'm new to D8, so I may be wrong here. Thanks for the great module. I've been using the D7 version for years.

I'm running v2.0@beta, and I can confirm the cache bug of #22 - WITHOUT any patches from here installed.

Adding a menu link plugin patch, shortly. The problem is you cannot alter the definition in the constructor as it'll become serialized that way.

Modified a test to show `Unmasquerade` should not show unless you are masquerading.

Thank you for patch! Looks nice workaround
Minor nit is missing class description with todo reference to core issue about permission

👍 thanks, @andypost. I'll make a follow up later. And I will actually run the tests locally and fix the failure.

Okay, so in my testing, locally:

// Permission 'masquerade as super user' granted by default.
$this->assertCanMasqueradeAs($this->rootUser);

`masquerade as super user` is broken, somehow, with this patch.

---

So this patch just uncovered that the tests have been false-positive on masquerading as a root user. The page has "You are not authorized to access this page.", which means that never worked.

---

Edit 2: you always see access denied on /masquerade after submitting the form 🤷‍♂️

I can't figure out why masquerading as uid1 breaks the menu link. I've tried everything in the tests. Even using messenger::addStatus in isEnabled. It just doesn't even run when switching to uid1.

Looks it needs to fix related first

This is more a workaround. We should focus on the core issue that, magically, fixes also this issue. Closing as duplicate of #2463753: [regression] Do not bypass route access with 'link to any page' permissions for menu links.

I confirm that patch on "https://www.drupal.org/project/drupal/issues/2463753" fix the issue on D9.3.6