From 9a34ed55ded91d89edb0ebad9b2aba45a0774067 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 13 May 2025 13:53:30 +0100 Subject: [PATCH] Issue #3516477 by catch, ericgsmith, acbramley, kristiaanvandeneynde, mxr576: Avoid cache redirect error when using 'view own unpublished content' permission alongside node grants (cherry picked from commit 7d1a9ca93b37369c8b763ecc2fafb1479239aa8b) --- .../node/src/NodeAccessControlHandler.php | 9 ++ .../NodeAccessCacheRedirectWarningTest.php | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 core/modules/node/tests/src/Functional/NodeAccessCacheRedirectWarningTest.php diff --git a/core/modules/node/src/NodeAccessControlHandler.php b/core/modules/node/src/NodeAccessControlHandler.php index 13020cea47bd..8a487847ec3b 100644 --- a/core/modules/node/src/NodeAccessControlHandler.php +++ b/core/modules/node/src/NodeAccessControlHandler.php @@ -223,7 +223,16 @@ protected function checkViewAccess(NodeInterface $node, AccountInterface $accoun return NULL; } + // When access is granted due to the 'view own unpublished content' + // permission and for no other reason, node grants are bypassed. However, + // to ensure the full set of cacheable metadata is available to variation + // cache, additionally add the node_grants cache context so that if the + // status or the owner of the node changes, cache redirects will continue to + // reflect the latest state without needing to be invalidated. $cacheability->addCacheContexts(['user']); + if ($this->moduleHandler->hasImplementations('node_grants')) { + $cacheability->addCacheContexts(['user.node_grants:view']); + } if ($account->id() != $node->getOwnerId()) { return NULL; } diff --git a/core/modules/node/tests/src/Functional/NodeAccessCacheRedirectWarningTest.php b/core/modules/node/tests/src/Functional/NodeAccessCacheRedirectWarningTest.php new file mode 100644 index 000000000000..0d49a7c416ce --- /dev/null +++ b/core/modules/node/tests/src/Functional/NodeAccessCacheRedirectWarningTest.php @@ -0,0 +1,89 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\node\Functional; + +/** + * Tests the node access grants cache context service. + * + * @group node + * @group Cache + */ +class NodeAccessCacheRedirectWarningTest extends NodeTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['block', 'node_access_test_empty']; + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + + node_access_rebuild(); + } + + /** + * Ensures that node access checks don't cause cache redirect warnings. + * + * @covers \Drupal\node\NodeAccessControlHandler + */ + public function testNodeAccessCacheRedirectWarning(): void { + $this->drupalPlaceBlock('local_tasks_block'); + + // Ensure that both a node_grants implementation exists, and that the + // current user has 'view own unpublished nodes' permission. Node's access + // control handler bypasses node grants when 'view own published nodes' is + // granted and the node is unpublished, which means that the code path is + // significantly different when a node is published vs. unpublished, and + // that cache contexts vary depend on the state of the node. + $this->assertTrue(\Drupal::moduleHandler()->hasImplementations('node_grants')); + + $author = $this->drupalCreateUser([ + 'create page content', + 'edit any page content', + 'view own unpublished content', + ]); + $this->drupalLogin($author); + + $node = $this->drupalCreateNode(['uid' => $author->id(), 'status' => 0]); + + $this->drupalGet($node->toUrl()); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains($node->label()); + + $node->setPublished(); + $node->save(); + + $this->drupalGet($node->toUrl()); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains($node->label()); + + // When the node has been viewed in both the unpublished and published state + // a cache redirect should exist for the local tasks block. Repeating the + // process of changing the node status and viewing the node will test that + // no stale redirect is found. + $node->setUnpublished(); + $node->save(); + + $this->drupalGet($node->toUrl()); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains($node->label()); + + $node->setPublished(); + $node->save(); + + $this->drupalGet($node->toUrl()); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains($node->label()); + } + +} -- GitLab