Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UHF-8522: Check target entity's view access for each menu link #36

Merged
merged 15 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions helfi_navigation.module
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ declare(strict_types = 1);
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\menu_link_content\Entity\MenuLinkContent;
use Drupal\menu_link_content\MenuLinkContentInterface;
Expand Down Expand Up @@ -156,10 +155,6 @@ function helfi_navigation_cron() : void {
* Implements hook_page_attachments().
*/
function helfi_navigation_page_attachments(array &$attachments) : void {
$langcode = Drupal::languageManager()
->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)
->getId();

/** @var \Drupal\helfi_api_base\Language\DefaultLanguageResolver $defaultLanguageResolver */
$defaultLanguageResolver = Drupal::service('helfi_api_base.default_language_resolver');

Expand Down
9 changes: 1 addition & 8 deletions helfi_navigation.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ services:
- '@config.factory'
- '@helfi_navigation.api_manager'
- '@helfi_navigation.menu_tree_builder'
- '@account_switcher'
helfi_navigation.menu_tree_builder:
class: Drupal\helfi_navigation\Menu\MenuTreeBuilder
arguments:
Expand All @@ -31,14 +32,6 @@ services:
- '@menu.link_tree'
- '@plugin.manager.menu.link'
- '@event_dispatcher'
helfi_navigation.anonymous_user:
class: Drupal\Core\Session\AnonymousUserSession
helfi_navigation.menu_tree_manipulators:
class: Drupal\helfi_navigation\Menu\MenuTreeManipulator
arguments:
- '@access_manager'
- '@helfi_navigation.anonymous_user'
- '@entity_type.manager'
helfi_navigation.cache_warmer:
class: Drupal\helfi_navigation\CacheWarmer
arguments:
Expand Down
9 changes: 6 additions & 3 deletions src/EventSubscriber/RedirectEventSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ final class RedirectEventSubscriber implements EventSubscriberInterface {
* The config factory service.
*/
public function __construct(
private RedirectRepository $repository,
private ConfigFactoryInterface $configFactory,
private readonly RedirectRepository $repository,
private readonly ConfigFactoryInterface $configFactory,
) {
}

Expand All @@ -35,7 +35,10 @@ public function __construct(
* Gets all available redirects for given link and updates the URL
* to use the redirect destination.
*
* This is required by javascript navigation to build the active trail.
* Mobile navigation uses path to figure out the active trail, something
* like "if (path in menu link === current url path in browser)". Override
* the menu link path to match the path where user actually lands after
* clicking the link.
*
* @param \Drupal\helfi_navigation\Event\MenuTreeBuilderLink $event
* The event to respond to.
Expand Down
22 changes: 15 additions & 7 deletions src/MainMenuManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
namespace Drupal\helfi_navigation;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Session\AccountSwitcherInterface;
use Drupal\Core\Session\AnonymousUserSession;
use Drupal\Core\Url;
use Drupal\helfi_navigation\Menu\MenuTreeBuilder;
use Drupal\language\ConfigurableLanguageManagerInterface;
Expand All @@ -26,12 +27,15 @@ class MainMenuManager {
* The api manager.
* @param \Drupal\helfi_navigation\Menu\MenuTreeBuilder $menuTreeBuilder
* The menu tree builder.
* @param \Drupal\Core\Session\AccountSwitcherInterface $accountSwitcher
* The account switcher service.
*/
public function __construct(
private ConfigurableLanguageManagerInterface $languageManager,
private ConfigFactoryInterface $config,
private ApiManager $apiManager,
private MenuTreeBuilder $menuTreeBuilder,
private readonly ConfigurableLanguageManagerInterface $languageManager,
private readonly ConfigFactoryInterface $config,
private readonly ApiManager $apiManager,
private readonly MenuTreeBuilder $menuTreeBuilder,
private readonly AccountSwitcherInterface $accountSwitcher,
) {
}

Expand All @@ -46,6 +50,9 @@ public function __construct(
* @throws \GuzzleHttp\Exception\GuzzleException
*/
public function sync(string $langcode): bool {
// Sync menu as an anonymous user to make sure no sensitive
// links are synced.
$this->accountSwitcher->switchTo(new AnonymousUserSession());
$response = $this->apiManager->update(
$langcode,
[
Expand All @@ -54,6 +61,8 @@ public function sync(string $langcode): bool {
'menu_tree' => $this->build($langcode),
]
);
$this->accountSwitcher->switchBack();

if (!isset($response->data->status)) {
throw new \InvalidArgumentException('Failed to parse entity published state.');
}
Expand Down Expand Up @@ -96,8 +105,7 @@ public function getSiteName(string $langcode) : string {
* @return mixed
* Menu tree.
*/
public function build(string $langcode = NULL): array {
$langcode = $langcode ?: $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_INTERFACE)->getId();
public function build(string $langcode): array {
$siteName = $this->getSiteName($langcode);

$instanceUri = Url::fromRoute('<front>', options: [
Expand Down
97 changes: 66 additions & 31 deletions src/Menu/MenuTreeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

namespace Drupal\helfi_navigation\Menu;

use Drupal\Core\Access\AccessResult;
use Drupal\Core\Access\AccessResultInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Menu\MenuLinkInterface;
use Drupal\Core\Menu\MenuLinkManagerInterface;
use Drupal\Core\Menu\MenuLinkTreeElement;
use Drupal\Core\Menu\MenuLinkTreeInterface;
use Drupal\Core\Menu\MenuTreeParameters;
use Drupal\helfi_api_base\Link\InternalDomainResolver;
Expand Down Expand Up @@ -35,11 +37,11 @@ final class MenuTreeBuilder {
* The event dispatcher.
*/
public function __construct(
private EntityTypeManagerInterface $entityTypeManager,
private InternalDomainResolver $domainResolver,
private MenuLinkTreeInterface $menuTree,
private MenuLinkManagerInterface $menuLinkManager,
private EventDispatcherInterface $eventDispatcher
private readonly EntityTypeManagerInterface $entityTypeManager,
private readonly InternalDomainResolver $domainResolver,
private readonly MenuLinkTreeInterface $menuTree,
private readonly MenuLinkManagerInterface $menuLinkManager,
private readonly EventDispatcherInterface $eventDispatcher
) {
}

Expand Down Expand Up @@ -76,7 +78,8 @@ public function build(
$tree = $this->menuTree->transform($tree, [
// Sync menu links accessible to anonymous users and sort them
// the same way core does.
['callable' => 'helfi_navigation.menu_tree_manipulators:checkAccess'],
['callable' => 'menu.default_tree_manipulators:checkNodeAccess'],
['callable' => 'menu.default_tree_manipulators:checkAccess'],
['callable' => 'menu.default_tree_manipulators:generateIndexAndSort'],
]);

Expand Down Expand Up @@ -121,27 +124,13 @@ private function transform(array $menuItems, string $langcode, string $rootId =

foreach ($menuItems as $element) {
/** @var \Drupal\menu_link_content\MenuLinkContentInterface $link */
// @todo Do we want to show links other than MenuLinkContent?
if (!$link = $this->getEntity($element->link)) {
if (!$link = $this->getEntity($element->link, $langcode)) {
continue;
}
$this->evaluateEntityAccess($element);

// Handle only menu links with translations.
if (
!$link->hasTranslation($langcode) ||
!$link->isTranslatable()
) {
continue;
}

/** @var \Drupal\menu_link_content\MenuLinkContentInterface $link */
$link = $link->getTranslation($langcode);

// Only show accessible links (and published).
if (
!$link->get('content_translation_status')->value ||
($element->access instanceof AccessResultInterface && !$element->access->isAllowed())
) {
// Only show accessible links.
if ($element->access instanceof AccessResultInterface && !$element->access->isAllowed()) {
continue;
}

Expand Down Expand Up @@ -233,29 +222,75 @@ private function processItem(MenuTreeBuilderLink $link) : array {
}

/**
* Load entity with given menu link.
* Load MenuLinkContent entity for given menu link.
*
* @param \Drupal\Core\Menu\MenuLinkInterface $link
* The menu link.
* @param string $langcode
* The langcode.
*
* @return \Drupal\menu_link_content\MenuLinkContentInterface|null
* NULL if entity not found and
* a MenuLinkContentInterface if found.
* NULL if a valid entity is not found and a MenuLinkContentInterface
* if found.
*
* @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
* @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
*/
private function getEntity(MenuLinkInterface $link): ? MenuLinkContentInterface {
// MenuLinkContent::getEntity() has protected visibility and cannot be used
// to directly fetch the entity.
private function getEntity(MenuLinkInterface $link, string $langcode): ? MenuLinkContentInterface {
$metadata = $link->getMetaData();

if (empty($metadata['entity_id'])) {
return NULL;
}
return $this->entityTypeManager
/** @var \Drupal\menu_link_content\Entity\MenuLinkContent $entity */
$entity = $this->entityTypeManager
->getStorage('menu_link_content')
->load($metadata['entity_id']);

// Skip untranslatable/untranslated menu links.
if (!$entity || !$entity->isTranslatable() || !$entity->hasTranslation($langcode)) {
return NULL;
}
$entity = $entity->getTranslation($langcode);

// Skip unpublished translations.
if (!$entity->get('content_translation_status')->value) {
return NULL;
}

return $entity;

}

/**
* Evaluates the access for link's target.
*
* @param \Drupal\Core\Menu\MenuLinkTreeElement $element
* The element to check entity access for.
*/
private function evaluateEntityAccess(MenuLinkTreeElement $element) : void {
// Attempt to fetch the entity type and id from link's route parameters.
// The route parameters should be an array containing entity type => id
// like: ['node' => '1'].
$routeParameters = $element->link->getRouteParameters();
$entityType = key($routeParameters);

if (!$this->entityTypeManager->hasDefinition($entityType)) {
return;
}
$storage = $this->entityTypeManager
->getStorage($entityType);

if (!$entity = $storage->load($routeParameters[$entityType])) {
return;
}
if (!$entity->access('view')) {
// Disallow access if user has no view access to the target entity.
// This updates the existing access result and will be evaluated in
// ::transform().
$element->access = AccessResult::neutral()
->addCacheableDependency($entity);
}
}

}
13 changes: 0 additions & 13 deletions src/Menu/MenuTreeManipulator.php

This file was deleted.

2 changes: 1 addition & 1 deletion src/Plugin/rest/resource/GlobalMobileMenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function get(): ResourceResponse {
$project_name = $environment->getId();

// Create menu tree and add data to the local menu.
$menuTree = $this->menuManager->build();
$menuTree = $this->menuManager->build($langcode);
$menuTree['is_injected'] = TRUE;

// Commented lines are present in the api request,
Expand Down
12 changes: 12 additions & 0 deletions tests/src/Kernel/MenuTreeBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,18 @@ public function testBuildMenuTree() : void {
->save();
$tree = $this->getMenuTree('fi');
$this->assertCount(1, $tree['sub_tree']);

// Unpublish all nodes and make sure they are not visible anymore.
foreach ($this->nodes as $node) {
$node->setUnpublished()
->save();
}
// Rebuild the container to empty static entity cache.
$this->container->get('kernel')->rebuildContainer();

// Make sure no links are visible after the node was unpublished.
$tree = $this->getMenuTree('fi');
$this->assertCount(0, $tree['sub_tree']);
}

}
1 change: 1 addition & 0 deletions tests/src/Kernel/MenuTreeBuilderTestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ protected function setUp(): void {
parent::setUp();

$this->installEntitySchema('node');
$this->installSchema('node', ['node_access']);

// We have a dependency to anonymous user when checking menu permissions
// and might run into 'entity:user' context is required error when trying
Expand Down
8 changes: 8 additions & 0 deletions tests/src/Traits/MenuLinkTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@
*/
trait MenuLinkTrait {

/**
* An array of nodes.
*
* @var \Drupal\node\NodeInterface[]
*/
protected array $nodes = [];

/**
* Creates a new test node.
*
Expand All @@ -31,6 +38,7 @@ protected function createNodeWithAlias() : NodeInterface {
'path' => ['alias' => '/test-node-page', 'langcode' => 'en'],
]);
$node->save();
$this->nodes[] = $node;

return $node;
}
Expand Down