From c7f47000ee205440ebb07217809aa3cc71e0378b Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Thu, 21 Feb 2019 08:26:15 -0600
Subject: [PATCH] Issue #3028490 by tim.plunkett, Kristen Pol, xjm, tedbow,
r.aubin, phenaproxima: Users with "configure any layout" can see entities
they don't have "view" access to
---
.../OverridesSectionStorage.php | 12 +-
.../src/Functional/LayoutBuilderTest.php | 32 ++++++
.../src/Unit/OverridesSectionStorageTest.php | 104 ++++++++++++++++++
3 files changed, 144 insertions(+), 4 deletions(-)
diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
index a488f2377c12..0454814bfe50 100644
--- a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
+++ b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
@@ -227,13 +227,17 @@ private function extractEntityFromRoute($value, array $defaults) {
*/
public function buildRoutes(RouteCollection $collection) {
foreach ($this->getEntityTypes() as $entity_type_id => $entity_type) {
+ // If the canonical route does not exist, do not provide any Layout
+ // Builder UI routes for this entity type.
+ if (!$collection->get("entity.$entity_type_id.canonical")) {
+ continue;
+ }
+
$defaults = [];
$defaults['entity_type_id'] = $entity_type_id;
- $requirements = [];
- if ($this->hasIntegerId($entity_type)) {
- $requirements[$entity_type_id] = '\d+';
- }
+ // Retrieve the requirements from the canonical route.
+ $requirements = $collection->get("entity.$entity_type_id.canonical")->getRequirements();
$options = [];
// Ensure that upcasting is run in the correct order.
diff --git a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
index 77dcb64ac597..b3b1a4d90df0 100644
--- a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
+++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php
@@ -283,6 +283,38 @@ public function testLayoutBuilderUi() {
$assert_session->elementNotExists('css', '.field--name-field-my-text');
}
+ /**
+ * Test that layout builder checks entity view access.
+ */
+ public function testAccess() {
+ $assert_session = $this->assertSession();
+
+ $this->drupalLogin($this->drupalCreateUser([
+ 'configure any layout',
+ 'administer node display',
+ ]));
+
+ $field_ui_prefix = 'admin/structure/types/manage/bundle_with_section_field';
+ // Allow overrides for the layout.
+ $this->drupalPostForm("$field_ui_prefix/display/default", ['layout[enabled]' => TRUE], 'Save');
+ $this->drupalPostForm("$field_ui_prefix/display/default", ['layout[allow_custom]' => TRUE], 'Save');
+
+ $this->drupalLogin($this->drupalCreateUser(['configure any layout']));
+ $this->drupalGet('node/1');
+ $assert_session->pageTextContains('The first node body');
+ $assert_session->pageTextNotContains('Powered by Drupal');
+ $node = Node::load(1);
+ $node->setUnpublished();
+ $node->save();
+ $this->drupalGet('node/1');
+ $assert_session->pageTextNotContains('The first node body');
+ $assert_session->pageTextContains('Access denied');
+
+ $this->drupalGet('node/1/layout');
+ $assert_session->pageTextNotContains('The first node body');
+ $assert_session->pageTextContains('Access denied');
+ }
+
/**
* Tests that a non-default view mode works as expected.
*/
diff --git a/core/modules/layout_builder/tests/src/Unit/OverridesSectionStorageTest.php b/core/modules/layout_builder/tests/src/Unit/OverridesSectionStorageTest.php
index 93fd799323e5..af2ae0d12ea3 100644
--- a/core/modules/layout_builder/tests/src/Unit/OverridesSectionStorageTest.php
+++ b/core/modules/layout_builder/tests/src/Unit/OverridesSectionStorageTest.php
@@ -294,6 +294,23 @@ public function testBuildRoutes() {
$entity_types['no_canonical_link'] = $no_canonical_link->reveal();
$this->entityFieldManager->getFieldStorageDefinitions('no_canonical_link')->shouldNotBeCalled();
+ $canonical_link_no_route = $this->prophesize(EntityTypeInterface::class);
+ $canonical_link_no_route->entityClassImplements(FieldableEntityInterface::class)->willReturn(TRUE);
+ $canonical_link_no_route->hasViewBuilderClass()->willReturn(TRUE);
+ $canonical_link_no_route->hasLinkTemplate('canonical')->willReturn(TRUE);
+ $canonical_link_no_route->getLinkTemplate('canonical')->willReturn('/entity/{entity}');
+ $canonical_link_no_route->hasHandlerClass('form', 'layout_builder')->willReturn(TRUE);
+ $entity_types['canonical_link_no_route'] = $canonical_link_no_route->reveal();
+ $this->entityFieldManager->getFieldStorageDefinitions('canonical_link_no_route')->shouldNotBeCalled();
+
+ $from_canonical = $this->prophesize(EntityTypeInterface::class);
+ $from_canonical->entityClassImplements(FieldableEntityInterface::class)->willReturn(TRUE);
+ $from_canonical->hasViewBuilderClass()->willReturn(TRUE);
+ $from_canonical->hasLinkTemplate('canonical')->willReturn(TRUE);
+ $from_canonical->getLinkTemplate('canonical')->willReturn('/entity/{entity}');
+ $from_canonical->hasHandlerClass('form', 'layout_builder')->willReturn(TRUE);
+ $entity_types['from_canonical'] = $from_canonical->reveal();
+
$with_string_id = $this->prophesize(EntityTypeInterface::class);
$with_string_id->entityClassImplements(FieldableEntityInterface::class)->willReturn(TRUE);
$with_string_id->hasViewBuilderClass()->willReturn(TRUE);
@@ -323,6 +340,87 @@ public function testBuildRoutes() {
$this->entityTypeManager->getDefinitions()->willReturn($entity_types);
$expected = [
+ 'entity.from_canonical.canonical' => new Route(
+ '/entity/{entity}',
+ [],
+ [
+ 'custom requirement' => 'from_canonical_route',
+ ]
+ ),
+ 'entity.with_string_id.canonical' => new Route(
+ '/entity/{entity}'
+ ),
+ 'entity.with_integer_id.canonical' => new Route(
+ '/entity/{entity}',
+ [],
+ [
+ 'with_integer_id' => '\d+',
+ ]
+ ),
+ 'layout_builder.overrides.from_canonical.view' => new Route(
+ '/entity/{entity}/layout',
+ [
+ 'entity_type_id' => 'from_canonical',
+ 'section_storage_type' => 'overrides',
+ 'section_storage' => '',
+ '_entity_form' => 'from_canonical.layout_builder',
+ '_title_callback' => '\Drupal\layout_builder\Controller\LayoutBuilderController::title',
+ ],
+ [
+ '_has_layout_section' => 'true',
+ '_layout_builder_access' => 'view',
+ 'custom requirement' => 'from_canonical_route',
+ ],
+ [
+ 'parameters' => [
+ 'section_storage' => ['layout_builder_tempstore' => TRUE],
+ 'from_canonical' => ['type' => 'entity:from_canonical'],
+ ],
+ '_layout_builder' => TRUE,
+ ]
+ ),
+ 'layout_builder.overrides.from_canonical.discard_changes' => new Route(
+ '/entity/{entity}/layout/discard-changes',
+ [
+ 'entity_type_id' => 'from_canonical',
+ 'section_storage_type' => 'overrides',
+ 'section_storage' => '',
+ '_form' => '\Drupal\layout_builder\Form\DiscardLayoutChangesForm',
+ ],
+ [
+ '_has_layout_section' => 'true',
+ '_layout_builder_access' => 'view',
+ 'custom requirement' => 'from_canonical_route',
+ ],
+ [
+ 'parameters' => [
+ 'section_storage' => ['layout_builder_tempstore' => TRUE],
+ 'from_canonical' => ['type' => 'entity:from_canonical'],
+ ],
+ '_layout_builder' => TRUE,
+ ]
+ ),
+ 'layout_builder.overrides.from_canonical.revert' => new Route(
+ '/entity/{entity}/layout/revert',
+ [
+ 'entity_type_id' => 'from_canonical',
+ 'section_storage_type' => 'overrides',
+ 'section_storage' => '',
+ '_form' => '\Drupal\layout_builder\Form\RevertOverridesForm',
+ ],
+ [
+ '_has_layout_section' => 'true',
+ '_layout_builder_access' => 'view',
+ 'custom requirement' => 'from_canonical_route',
+ ],
+ [
+ 'parameters' => [
+ 'section_storage' => ['layout_builder_tempstore' => TRUE],
+ 'from_canonical' => ['type' => 'entity:from_canonical'],
+ ],
+ '_layout_builder' => TRUE,
+ ]
+ ),
'layout_builder.overrides.with_string_id.view' => new Route(
'/entity/{entity}/layout',
[
@@ -451,6 +549,12 @@ public function testBuildRoutes() {
];
$collection = new RouteCollection();
+ // Entity types that declare a link template for canonical must have a
+ // canonical route present in the route colletion.
+ $collection->add('entity.from_canonical.canonical', $expected['entity.from_canonical.canonical']);
+ $collection->add('entity.with_string_id.canonical', $expected['entity.with_string_id.canonical']);
+ $collection->add('entity.with_integer_id.canonical', $expected['entity.with_integer_id.canonical']);
+
$this->plugin->buildRoutes($collection);
$this->assertEquals($expected, $collection->all());
$this->assertSame(array_keys($expected), array_keys($collection->all()));
--
GitLab