Issue #2946073 by claudiu.cristea, mpp, neclimdul, Berdir, idimopoulo…

…s, remkoklein: Node grant access check missing cacheable dependency on node

(cherry picked from commit 6685ea27ee977078bf83323e3ab3cd4f6b13685b)

modules/node/src/NodeAccessControlHandler.php

@@ -3,6 +3,7 @@
 namespace Drupal\node;
 
 use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
 use Drupal\Core\Entity\EntityHandlerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
@@ -104,7 +105,16 @@ protected function checkAccess(EntityInterface $node, $operation, AccountInterfa
     }
 
     // Evaluate node grants.
-    return $this->grantStorage->access($node, $operation, $account);
+    $access_result = $this->grantStorage->access($node, $operation, $account);
+    if ($operation === 'view' && $access_result instanceof RefinableCacheableDependencyInterface) {
+      // Node variations can affect the access to the node. For instance, the
+      // access result cache varies on the node's published status. Only the
+      // 'view' node grant can currently be cached. The 'update' and 'delete'
+      // grants are already marked as uncacheable in the node grant storage.
+      // @see \Drupal\node\NodeGrantDatabaseStorage::access()
+      $access_result->addCacheableDependency($node);
+    }
+    return $access_result;
   }
 
   /**

modules/node/src/NodeGrantDatabaseStorage.php

@@ -71,7 +71,7 @@ public function access(NodeInterface $node, $operation, AccountInterface $accoun
       // Return the equivalent of the default grant, defined by
       // self::writeDefault().
       if ($operation === 'view') {
-        return AccessResult::allowedIf($node->isPublished())->addCacheableDependency($node);
+        return AccessResult::allowedIf($node->isPublished());
       }
       else {
         return AccessResult::neutral();

modules/node/tests/src/Functional/NodeAccessCacheabilityWithNodeGrants.php

@@ -0,0 +1,66 @@
+<?php
+
+namespace Drupal\Tests\node\Functional;
+
+use Drupal\Core\Entity\Entity\EntityViewDisplay;
+use Drupal\node\Entity\NodeType;
+use Drupal\Tests\BrowserTestBase;
+use Drupal\Tests\field\Traits\EntityReferenceTestTrait;
+
+/**
+ * Tests node view access cacheability with node grants.
+ *
+ * @group node
+ */
+class NodeAccessCacheabilityWithNodeGrants extends BrowserTestBase {
+
+  use EntityReferenceTestTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['node', 'node_test'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * Tests node view access cacheability with node grants.
+   */
+  public function testAccessCacheabilityWithNodeGrants() {
+    NodeType::create(['type' => 'page'])->save();
+    $this->createEntityReferenceField('node', 'page', 'ref', 'Ref', 'node');
+    EntityViewDisplay::create([
+      'targetEntityType' => 'node',
+      'bundle' => 'page',
+      'mode' => 'default',
+      'status' => TRUE,
+    ])->setComponent('ref', ['type' => 'entity_reference_label'])
+      ->save();
+
+    // Check that at least one module implements hook_node_grants() as this test
+    // only tests this case.
+    // @see \node_test_node_grants()
+    $node_grants_implementations = \Drupal::moduleHandler()->getImplementations('node_grants');
+    $this->assertNotEmpty($node_grants_implementations);
+
+    // Create an unpublished node.
+    $referenced = $this->createNode(['status' => FALSE]);
+    // Create a node referencing $referenced.
+    $node = $this->createNode(['ref' => $referenced]);
+
+    // Check that the referenced entity link doesn't show on the host entity.
+    $this->drupalGet($node->toUrl());
+    $this->assertSession()->linkNotExists($referenced->label());
+
+    // Publish the referenced node.
+    $referenced->setPublished()->save();
+
+    // Check that the referenced entity link shows on the host entity.
+    $this->getSession()->reload();
+    $this->assertSession()->linkExists($referenced->label());
+  }
+
+}