Issue #2099137 by Wim Leers, amateescu, rteijeiro, larowlan, effulgentsia: Entity/field access and node grants not taken into account with core cache contexts

core/lib/Drupal/Core/Access/AccessResult.php

@@ -339,13 +339,24 @@ abstract class AccessResult implements AccessResultInterface, CacheableDependenc
    * {@inheritdoc}
    */
   public function orIf(AccessResultInterface $other) {
+    $merge_other = FALSE;
     // $other's cacheability metadata is merged if $merge_other gets set to TRUE
-    // and this happens in two cases:
+    // and this happens in three cases:
     // 1. $other's access result is the one that determines the combined access
     //    result.
     // 2. This access result is not cacheable and $other's access result is the
     //    same. i.e. attempt to return a cacheable access result.
-    $merge_other = FALSE;
+    // 3. Neither access result is 'forbidden' and both are cacheable: inherit
+    //    the other's cacheability metadata because it may turn into a
+    //    'forbidden' for another value of the cache contexts in the
+    //    cacheability metadata. In other words: this is necessary to respect
+    //    the contagious nature of the 'forbidden' access result.
+    //    e.g. we have two access results A and B. Neither is forbidden. A is
+    //    globally cacheable (no cache contexts). B is cacheable per role. If we
+    //    don't have merging case 3, then A->orIf(B) will be globally cacheable,
+    //    which means that even if a user of a different role logs in, the
+    //    cached access result will be used, even though for that other role, B
+    //    is forbidden!
     if ($this->isForbidden() || $other->isForbidden()) {
       $result = static::forbidden();
       if (!$this->isForbidden() || ($this->getCacheMaxAge() === 0 && $other->isForbidden())) {
@@ -354,13 +365,13 @@ abstract class AccessResult implements AccessResultInterface, CacheableDependenc
     }
     elseif ($this->isAllowed() || $other->isAllowed()) {
       $result = static::allowed();
-      if (!$this->isAllowed() || ($this->getCacheMaxAge() === 0 && $other->isAllowed())) {
+      if (!$this->isAllowed() || ($this->getCacheMaxAge() === 0 && $other->isAllowed()) || ($this->getCacheMaxAge() !== 0 && $other instanceof CacheableDependencyInterface && $other->getCacheMaxAge() !== 0)) {
         $merge_other = TRUE;
       }
     }
     else {
       $result = static::neutral();
-      if (!$this->isNeutral() || ($this->getCacheMaxAge() === 0 && $other->isNeutral())) {
+      if (!$this->isNeutral() || ($this->getCacheMaxAge() === 0 && $other->isNeutral()) || ($this->getCacheMaxAge() !== 0 && $other instanceof CacheableDependencyInterface && $other->getCacheMaxAge() !== 0)) {
         $merge_other = TRUE;
       }
     }

core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php

@@ -33,13 +33,6 @@ class EntityFormDisplay extends EntityDisplayBase implements EntityFormDisplayIn
    */
   protected $displayContext = 'form';
 
-  /**
-   * The renderer.
-   *
-   * @var \Drupal\Core\Render\RendererInterface
-   */
-  protected $renderer;
-
   /**
    * Returns the entity_form_display object used to build an entity form.
    *
@@ -122,7 +115,6 @@ class EntityFormDisplay extends EntityDisplayBase implements EntityFormDisplayIn
    */
   public function __construct(array $values, $entity_type) {
     $this->pluginManager = \Drupal::service('plugin.manager.field.widget');
-    $this->renderer = \Drupal::service('renderer');
 
     parent::__construct($values, $entity_type);
   }

core/lib/Drupal/Core/Entity/Entity/EntityViewDisplay.php

@@ -238,8 +238,11 @@ class EntityViewDisplay extends EntityDisplayBase implements EntityViewDisplayIn
         // Then let the formatter build the output for each entity.
         foreach ($entities as $id => $entity) {
           $items = $grouped_items[$id];
-          $build_list[$id][$name] = $formatter->view($items);
-          $build_list[$id][$name]['#access'] = $items->access('view');
+          /** @var \Drupal\Core\Access\AccessResultInterface $field_access */
+          $field_access = $items->access('view', NULL, TRUE);
+          $build_list[$id][$name] = $field_access->isAllowed() ? $formatter->view($items) : [];
+          // Apply the field access cacheability metadata to the render array.
+          $this->renderer->addDependency($build_list[$id][$name], $field_access);
         }
       }
     }

core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php

@@ -218,9 +218,9 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce
 
     // Invoke hook_entity_create_access() and hook_ENTITY_TYPE_create_access().
     // Hook results take precedence over overridden implementations of
-    // EntityAccessControlHandler::checkAccess(). Entities that have checks that
-    // need to be done before the hook is invoked should do so by overriding
-    // this method.
+    // EntityAccessControlHandler::checkCreateAccess(). Entities that have
+    // checks that need to be done before the hook is invoked should do so by
+    // overriding this method.
 
     // We grant access to the entity if both of these conditions are met:
     // - No modules say to deny access.

core/lib/Drupal/Core/Entity/EntityDisplayBase.php

@@ -113,6 +113,13 @@ abstract class EntityDisplayBase extends ConfigEntityBase implements EntityDispl
    */
   protected $pluginManager;
 
+  /**
+   * The renderer.
+   *
+   * @var \Drupal\Core\Render\RendererInterface
+   */
+  protected $renderer;
+
   /**
    * {@inheritdoc}
    */
@@ -125,6 +132,8 @@ abstract class EntityDisplayBase extends ConfigEntityBase implements EntityDispl
       throw new \InvalidArgumentException('EntityDisplay entities can only handle fieldable entity types.');
     }
 
+    $this->renderer = \Drupal::service('renderer');
+
     // A plugin manager and a context type needs to be set by extending classes.
     if (!isset($this->pluginManager)) {
       throw new \RuntimeException('Missing plugin manager.');

core/lib/Drupal/Core/Entity/EntityTypeInterface.php

@@ -660,7 +660,7 @@ interface EntityTypeInterface {
    *
    * Enables code listing entities of this type to ensure that rendered listings
    * are varied as necessary, typically to ensure users of role A see other
-   * entities listed as users of role B.
+   * entities listed than users of role B.
    *
    * @return string[]
    */

core/lib/Drupal/Core/Entity/EntityViewBuilder.php

@@ -174,11 +174,6 @@ class EntityViewBuilder extends EntityHandlerBase implements EntityHandlerInterf
       ),
     );
 
-     // @todo Remove when https://www.drupal.org/node/2099137 lands.
-     $build['#cache']['contexts'] = Cache::mergeContexts($build['#cache']['contexts'], [
-       'user.roles',
-     ]);
-
     // Cache the rendered output if permitted by the view mode and global entity
     // type configuration.
     if ($this->isViewModeCacheable($view_mode) && !$entity->isNew() && $entity->isDefaultRevision() && $this->entityType->isRenderCacheable()) {

core/lib/Drupal/Core/Field/FormatterBase.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\Field;
 
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Render\Element;
 
 /**
  * Base class for 'Field formatter' plugin implementations.
@@ -76,10 +77,11 @@ abstract class FormatterBase extends PluginSettingsBase implements FormatterInte
    * {@inheritdoc}
    */
   public function view(FieldItemListInterface $items) {
-    $addition = array();
-
     $elements = $this->viewElements($items);
-    if ($elements) {
+
+    // If there are actual renderable children, use #theme => field, otherwise,
+    // let access cacheability metadata pass through for correct bubbling.
+    if (Element::children($elements)) {
       $entity = $items->getEntity();
       $entity_type = $entity->getEntityTypeId();
       $field_name = $this->fieldDefinition->getName();
@@ -99,10 +101,10 @@ abstract class FormatterBase extends PluginSettingsBase implements FormatterInte
         '#formatter' => $this->getPluginId(),
       );
 
-      $addition = array_merge($info, $elements);
+      $elements = array_merge($info, $elements);
     }
 
-    return $addition;
+    return $elements;
   }
 
   /**

core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/EntityReferenceFormatterBase.php

@@ -7,9 +7,12 @@
 
 namespace Drupal\Core\Field\Plugin\Field\FieldFormatter;
 
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\EntityReferenceFieldItemListInterface;
+use Drupal\Core\Field\FieldItemListInterface;
 use Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem;
 use Drupal\Core\Field\FormatterBase;
+use Drupal\Core\Render\BubbleableMetadata;
 use Drupal\Core\TypedData\TranslatableInterface;
 
 /**
@@ -36,6 +39,8 @@ abstract class EntityReferenceFormatterBase extends FormatterBase {
    *
    * @return \Drupal\Core\Entity\EntityInterface[]
    *   The array of referenced entities to display, keyed by delta.
+   *
+   * @see ::prepareView()
    */
   protected function getEntitiesToView(EntityReferenceFieldItemListInterface $items) {
     $entities = array();
@@ -51,8 +56,10 @@ abstract class EntityReferenceFormatterBase extends FormatterBase {
           $entity = $entity->getTranslation($parent_entity_langcode);
         }
 
-        // Check entity access if needed.
-        if (!$this->needsAccessCheck($item) || $entity->access('view')) {
+        $access = $this->checkAccess($entity);
+        // Add the access result's cacheability, ::view() needs it.
+        $item->_accessCacheability = BubbleableMetadata::createFromObject($access);
+        if ($access->isAllowed()) {
           // Add the referring item, in case the formatter needs it.
           $entity->_referringItem = $items[$delta];
           $entities[$delta] = $entity;
@@ -63,6 +70,49 @@ abstract class EntityReferenceFormatterBase extends FormatterBase {
     return $entities;
   }
 
+  /**
+   * {@inheritdoc}
+   *
+   * @see ::prepareView()
+   * @see ::getEntitiestoView()
+   */
+  public function view(FieldItemListInterface $items) {
+    $elements = parent::view($items);
+
+    $field_level_access_cacheability = new BubbleableMetadata();
+
+    // Try to map the cacheability of the access result that was set at
+    // _accessCacheability in getEntitiesToView() to the corresponding render
+    // subtree. If no such subtree is found, then merge it with the field-level
+    // access cacheability.
+    foreach ($items as $delta => $item) {
+      // Ignore items for which access cacheability could not be determined in
+      // prepareView().
+      if (!empty($item->_accessCacheability)) {
+        if (isset($elements[$delta])) {
+          BubbleableMetadata::createFromRenderArray($elements[$delta])
+            ->merge($item->_accessCacheability)
+            ->applyTo($elements[$delta]);
+        }
+        else {
+          $field_level_access_cacheability = $field_level_access_cacheability->merge($item->_accessCacheability);
+        }
+      }
+    }
+
+    // Apply the cacheability metadata for the inaccessible entities and the
+    // entities for which the corresponding render subtree could not be found.
+    // This causes the field to be rendered (and cached) according to the cache
+    // contexts by which the access results vary, to ensure only users with
+    // access to this field can view it. It also tags this field with the cache
+    // tags on which the access results depend, to ensure users that cannot view
+    // this field at the moment will gain access once any of those cache tags
+    // are invalidated.
+    $field_level_access_cacheability->applyTo($elements);
+
+    return $elements;
+  }
+
   /**
    * {@inheritdoc}
    *
@@ -121,16 +171,20 @@ abstract class EntityReferenceFormatterBase extends FormatterBase {
   }
 
   /**
-   * Returns whether entity access should be checked.
+   * Checks access to the given entity.
    *
-   * @param \Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem $item
-   *    The item to check.
+   * By default, entity access is checked. However, a subclass can choose to
+   * exclude certain items from entity access checking by immediately granting
+   * access.
    *
-   * @return bool
-   *   TRUE if entity access should be checked.
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *    The entity to check.
+   *
+   * @return \Drupal\Core\Access\AccessResult
+   *   A cacheable access result.
    */
-  protected function needsAccessCheck(EntityReferenceItem $item) {
-    return TRUE;
+  protected function checkAccess(EntityInterface $entity) {
+    return $entity->access('view', NULL, TRUE);
   }
 
 }

core/modules/block_content/src/Tests/BlockContentCacheTagsTest.php

@@ -52,6 +52,15 @@ class BlockContentCacheTagsTest extends EntityCacheTagsTestBase {
     return $block_content;
   }
 
+  /**
+   * {@inheritdoc}
+   *
+   * @see \Drupal\block_content\BlockContentAccessControlHandler::checkAccess()
+   */
+  protected function getAccessCacheContextsForEntity(EntityInterface $entity) {
+    return [];
+  }
+
   /**
    * {@inheritdoc}
    *
@@ -86,12 +95,12 @@ class BlockContentCacheTagsTest extends EntityCacheTagsTestBase {
 
     // Expected contexts and tags for the BlockContent entity.
     // @see \Drupal\Core\Entity\EntityViewBuilder::getBuildDefaults().
-    $expected_entity_cache_contexts = ['theme', 'user.roles'];
+    $expected_entity_cache_contexts = ['theme'];
     $expected_entity_cache_tags = Cache::mergeTags(['block_content_view'], $this->entity->getCacheTags(), $this->getAdditionalCacheTagsForEntity($this->entity));
 
     // Verify that what was render cached matches the above expectations.
     $cid = $this->createCacheId($expected_block_cache_keys, $expected_block_cache_contexts);
     $redirected_cid = $this->createCacheId($expected_block_cache_keys, Cache::mergeContexts($expected_block_cache_contexts, $expected_entity_cache_contexts));
-    $this->verifyRenderCache($cid, Cache::mergeTags($expected_block_cache_tags, $expected_entity_cache_tags), $redirected_cid);
+    $this->verifyRenderCache($cid, Cache::mergeTags($expected_block_cache_tags, $expected_entity_cache_tags), ($cid !== $redirected_cid) ? $redirected_cid : NULL);
   }
 }

core/modules/comment/src/Tests/CommentCacheTagsTest.php

@@ -81,6 +81,18 @@ class CommentCacheTagsTest extends EntityWithUriCacheTagsTestBase {
     return $comment;
   }
 
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getAdditionalCacheContextsForEntity(EntityInterface $entity) {
+    return [
+      // Field access for the user picture rendered as part of the node that
+      // this comment is created on.
+      'user.permissions',
+    ];
+  }
+
   /**
    * {@inheritdoc}
    *

core/modules/field/src/Tests/EntityReference/EntityReferenceFormatterTest.php

@@ -10,6 +10,7 @@ namespace Drupal\field\Tests\EntityReference;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
 use Drupal\entity_reference\Tests\EntityReferenceTestTrait;
+use Drupal\Core\Render\BubbleableMetadata;
 use Drupal\field\Entity\FieldStorageConfig;
 use Drupal\filter\Entity\FilterFormat;
 use Drupal\system\Tests\Entity\EntityUnitTestBase;
@@ -218,26 +219,44 @@ class EntityReferenceFormatterTest extends EntityUnitTestBase {
     // The 'link' settings is TRUE by default.
     $build = $this->buildRenderArray([$this->referencedEntity, $this->unsavedReferencedEntity], $formatter);
 
+    $expected_field_cacheability = [
+      'contexts' => [],
+      'tags' => [],
+      'max-age' => Cache::PERMANENT,
+    ];
+    $this->assertEqual($build['#cache'], $expected_field_cacheability, 'The field render array contains the entity access cacheability metadata');
     $expected_item_1 = array(
       '#type' => 'link',
       '#title' => $this->referencedEntity->label(),
       '#url' => $this->referencedEntity->urlInfo(),
       '#options' => $this->referencedEntity->urlInfo()->getOptions(),
       '#cache' => array(
+        'contexts' => [
+          'user.permissions',
+        ],
         'tags' => $this->referencedEntity->getCacheTags(),
       ),
+      '#attached' => [],
+      '#post_render_cache' => [],
     );
     $this->assertEqual(drupal_render($build[0]), drupal_render($expected_item_1), sprintf('The markup returned by the %s formatter is correct for an item with a saved entity.', $formatter));
+    $this->assertEqual(BubbleableMetadata::createFromRenderArray($build[0]), BubbleableMetadata::createFromRenderArray($expected_item_1));
 
     // The second referenced entity is "autocreated", therefore not saved and
     // lacking any URL info.
     $expected_item_2 = array(
       '#markup' => $this->unsavedReferencedEntity->label(),
       '#cache' => array(
+        'contexts' => [
+          'user.permissions',
+        ],
         'tags' => $this->unsavedReferencedEntity->getCacheTags(),
+        'max-age' => Cache::PERMANENT,
       ),
+      '#attached' => [],
+      '#post_render_cache' => [],
     );
-    $this->assertEqual($build[1], $expected_item_2, sprintf('The markup returned by the %s formatter is correct for an item with a unsaved entity.', $formatter));
+    $this->assertEqual($build[1], $expected_item_2, sprintf('The render array returned by the %s formatter is correct for an item with a unsaved entity.', $formatter));
 
     // Test with the 'link' setting set to FALSE.
     $build = $this->buildRenderArray([$this->referencedEntity, $this->unsavedReferencedEntity], $formatter, array('link' => FALSE));

core/modules/file/src/Plugin/Field/FieldFormatter/FileFormatterBase.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\file\Plugin\Field\FieldFormatter;
 
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\Plugin\Field\FieldFormatter\EntityReferenceFormatterBase;
 use Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem;
 
@@ -25,11 +27,16 @@ abstract class FileFormatterBase extends EntityReferenceFormatterBase {
   /**
    * {@inheritdoc}
    */
-  protected function needsAccessCheck(EntityReferenceItem $item) {
+  protected function checkAccess(EntityInterface $entity) {
     // Only check access if the current file access control handler explicitly
     // opts in by implementing FileAccessFormatterControlHandlerInterface.
-    $access_handler_class = $item->entity->getEntityType()->getHandlerClass('access');
-    return is_subclass_of($access_handler_class, '\Drupal\file\FileAccessFormatterControlHandlerInterface');
+    $access_handler_class = $entity->getEntityType()->getHandlerClass('access');
+    if (is_subclass_of($access_handler_class, '\Drupal\file\FileAccessFormatterControlHandlerInterface')) {
+      return $entity->access('view', NULL, TRUE);
+    }
+    else {
+      return AccessResult::allowed();
+    }
   }
 
 }

core/modules/file/src/Plugin/Field/FieldFormatter/RSSEnclosureFormatter.php

@@ -39,6 +39,7 @@ class RSSEnclosureFormatter extends FileFormatterBase {
         ),
       );
     }
+    return [];
   }
 
 }

core/modules/filter/src/Tests/FilterFormatAccessTest.php

@@ -126,7 +126,7 @@ class FilterFormatAccessTest extends WebTestBase {
     $this->assertTrue($this->allowedFormat->access('use', $this->webUser), 'A regular user has access to use a text format they were granted access to.');
     $this->assertEqual(AccessResult::allowed()->addCacheContexts(['user.permissions']), $this->allowedFormat->access('use', $this->webUser, TRUE), 'A regular user has access to use a text format they were granted access to.');
     $this->assertFalse($this->disallowedFormat->access('use', $this->webUser), 'A regular user does not have access to use a text format they were not granted access to.');
-    $this->assertEqual(AccessResult::neutral(), $this->disallowedFormat->access('use', $this->webUser, TRUE)); //, 'A regular user does not have access to use a text format they were not granted access to.');
+    $this->assertEqual(AccessResult::neutral()->cachePerPermissions(), $this->disallowedFormat->access('use', $this->webUser, TRUE), 'A regular user does not have access to use a text format they were not granted access to.');
     $this->assertTrue($fallback_format->access('use', $this->webUser), 'A regular user has access to use the fallback format.');
     $this->assertEqual(AccessResult::allowed(), $fallback_format->access('use', $this->webUser, TRUE), 'A regular user has access to use the fallback format.');
 

core/modules/node/src/Tests/Views/FrontPageTest.php

@@ -290,7 +290,6 @@ class FrontPageTest extends ViewTestBase {
     }
     $cache_contexts = Cache::mergeContexts($cache_contexts, [
       'timezone',
-      'user.roles'
     ]);
 
     // First page.

core/modules/system/entity.api.php

@@ -1844,6 +1844,7 @@ function hook_entity_field_access($operation, \Drupal\Core\Field\FieldDefinition
   if ($field_definition->getName() == 'field_of_interest' && $operation == 'edit') {
     return AccessResult::allowedIfHasPermission($account, 'update field of interest');
   }
+  return AccessResult::neutral();
 }
 
 /**

core/modules/system/src/Tests/Entity/EntityCacheTagsTestBase.php

@@ -129,6 +129,21 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
    */
   abstract protected function createEntity();
 
+  /**
+   * Returns the access cache contexts for the tested entity.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity to be tested, as created by createEntity().
+   *
+   * @return string[]
+   *   An array of the additional cache contexts.
+   *
+   * @see \Drupal\Core\Entity\EntityAccessControlHandlerInterface
+   */
+  protected function getAccessCacheContextsForEntity(EntityInterface $entity) {
+    return ['user.permissions'];
+  }
+
   /**
    * Returns the additional (non-standard) cache contexts for the tested entity.
    *
@@ -317,7 +332,7 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
 
     // The default cache contexts for rendered entities.
     $default_cache_contexts = ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme'];
-    $entity_cache_contexts = Cache::mergeContexts($default_cache_contexts, ['user.roles']);
+    $entity_cache_contexts = $default_cache_contexts;
 
     // Cache tags present on every rendered page.
     $page_cache_tags = Cache::mergeTags(
@@ -326,6 +341,7 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
       // which adds the block config entity type's list cache tags.
       \Drupal::moduleHandler()->moduleExists('block') ? ['config:block_list']: []
     );
+    $page_cache_tags_referencing_entity = in_array('user.permissions', $this->getAccessCacheContextsForEntity($this->referencingEntity)) ? ['config:user.role.anonymous'] : [];
 
     $view_cache_tag = array();
     if ($this->entity->getEntityType()->hasHandlerClass('view_builder')) {
@@ -366,11 +382,15 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
     $this->pass("Test referencing entity.", 'Debug');
     $this->verifyPageCache($referencing_entity_url, 'MISS');
     // Verify a cache hit, but also the presence of the correct cache tags.
-    $this->verifyPageCache($referencing_entity_url, 'HIT', Cache::mergeTags($referencing_entity_cache_tags, $page_cache_tags));
+    $this->verifyPageCache($referencing_entity_url, 'HIT', Cache::mergeTags($referencing_entity_cache_tags, $page_cache_tags, $page_cache_tags_referencing_entity));
     // Also verify the existence of an entity render cache entry.
     $cache_keys = ['entity_view', 'entity_test', $this->referencingEntity->id(), 'full'];
     $cid = $this->createCacheId($cache_keys, $entity_cache_contexts);
-    $redirected_cid = $this->createRedirectedCacheId($cache_keys, $entity_cache_contexts);
+    $access_cache_contexts = $this->getAccessCacheContextsForEntity($this->entity);
+    $redirected_cid = NULL;
+    if (count($access_cache_contexts)) {
+      $redirected_cid = $this->createCacheId($cache_keys, Cache::mergeContexts($entity_cache_contexts, $this->getAdditionalCacheContextsForEntity($this->referencingEntity), $access_cache_contexts));
+    }
     $this->verifyRenderCache($cid, $referencing_entity_cache_tags, $redirected_cid);
 
     $this->pass("Test non-referencing entity.", 'Debug');
@@ -387,7 +407,7 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
     // Prime the page cache for the listing of referencing entities.
     $this->verifyPageCache($listing_url, 'MISS');
     // Verify a cache hit, but also the presence of the correct cache tags.
-    $this->verifyPageCache($listing_url, 'HIT', Cache::mergeTags($referencing_entity_cache_tags, $page_cache_tags));
+    $this->verifyPageCache($listing_url, 'HIT', Cache::mergeTags($referencing_entity_cache_tags, $page_cache_tags, $page_cache_tags_referencing_entity));
 
 
     $this->pass("Test empty listing.", 'Debug');
@@ -638,32 +658,6 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
     return implode(':', $cid_parts);
   }
 
-  /**
-   * Creates the redirected cache ID, if any.
-   *
-   * If a subclass overrides ::getAdditionalCacheContextsForEntity(), it can
-   * specify the additional cache contexts by which the given entity must be
-   * varied, because those are the cache contexts that are bubbled from the
-   * field formatters.
-   *
-   * @param string[] $keys
-   *   A list of cache keys used for the regular (pre-bubbling) CID.
-   * @param string[] $contexts
-   *   A set of cache contexts used for the regular (pre-bubbling) CID.
-   *
-   * @return string|null
-   *   The redirected (post-bubbling) CID, if any.
-   */
-  protected function createRedirectedCacheId(array $keys, array $contexts) {
-    $additional_cache_contexts = $this->getAdditionalCacheContextsForEntity($this->referencingEntity);
-    if (count($additional_cache_contexts)) {
-      return $this->createCacheId($keys, Cache::mergeContexts($contexts, $additional_cache_contexts));
-    }
-    else {
-      return NULL;
-    }
-  }
-
   /**
    * Verify that a given render cache entry exists, with the correct cache tags.
    *
@@ -681,12 +675,21 @@ abstract class EntityCacheTagsTestBase extends PageCacheTagsTestBase {
     sort($cache_entry->tags);
     sort($tags);
     $this->assertIdentical($cache_entry->tags, $tags);
+    $is_redirecting_cache_item = isset($cache_entry->data['#cache_redirect']);
     if ($redirected_cid === NULL) {
-      $this->assertTrue(!isset($cache_entry->data['#cache_redirect']), 'Render cache entry is not a redirect.');
+      $this->assertFalse($is_redirecting_cache_item, 'Render cache entry is not a redirect.');
+      // If this is a redirecting cache item unlike we expected, log it.
+      if ($is_redirecting_cache_item) {
+        debug($cache_entry->data);
+      }
     }
     else {
       // Verify that $cid contains a cache redirect.
-      $this->assertTrue(isset($cache_entry->data['#cache_redirect']), 'Render cache entry is a redirect.');
+      $this->assertTrue($is_redirecting_cache_item, 'Render cache entry is a redirect.');
+      // If this is not a redirecting cache item unlike we expected, log it.
+      if (!$is_redirecting_cache_item) {
+        debug($cache_entry->data);
+      }
       // Verify that the cache redirect points to the expected CID.
       $redirect_cache_metadata = $cache_entry->data['#cache'];
       $actual_redirection_cid = $this->createCacheId(

core/modules/system/src/Tests/Entity/EntityViewBuilderTest.php

@@ -63,7 +63,7 @@ class EntityViewBuilderTest extends EntityUnitTestBase {
     // Get a fully built entity view render array.
     $entity_test->save();
     $build = $this->container->get('entity.manager')->getViewBuilder('entity_test')->view($entity_test, 'full');
-    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(Cache::mergeContexts($build['#cache']['contexts'], ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme'])));
+    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme']));
     $cid = implode(':', $cid_parts);
     $bin = $build['#cache']['bin'];
 
@@ -113,7 +113,7 @@ class EntityViewBuilderTest extends EntityUnitTestBase {
 
     // Get a fully built entity view render array for the referenced entity.
     $build = $this->container->get('entity.manager')->getViewBuilder('entity_test')->view($entity_test_reference, 'full');
-    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(Cache::mergeContexts($build['#cache']['contexts'], ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme'])));
+    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme']));
     $cid_reference = implode(':', $cid_parts);
     $bin_reference = $build['#cache']['bin'];
 
@@ -132,7 +132,7 @@ class EntityViewBuilderTest extends EntityUnitTestBase {
 
     // Get a fully built entity view render array.
     $build = $this->container->get('entity.manager')->getViewBuilder('entity_test')->view($entity_test, 'full');
-    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(Cache::mergeContexts($build['#cache']['contexts'], ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme'])));
+    $cid_parts = array_merge($build['#cache']['keys'], $cache_contexts_manager->convertTokensToKeys(['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme']));
     $cid = implode(':', $cid_parts);
     $bin = $build['#cache']['bin'];
 

core/modules/system/src/Tests/Entity/EntityWithUriCacheTagsTestBase.php

@@ -32,7 +32,7 @@ abstract class EntityWithUriCacheTagsTestBase extends EntityCacheTagsTestBase {
     $view_mode = $this->selectViewMode($entity_type);
 
     // The default cache contexts for rendered entities.
-    $entity_cache_contexts = ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme', 'user.roles'];
+    $entity_cache_contexts = ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme'];
 
     // Generate the standardized entity cache tags.
     $cache_tag = $this->entity->getCacheTags();
@@ -51,7 +51,11 @@ abstract class EntityWithUriCacheTagsTestBase extends EntityCacheTagsTestBase {
     if (\Drupal::entityManager()->getDefinition($entity_type)->isRenderCacheable()) {
       $cache_keys = ['entity_view', $entity_type, $this->entity->id(), $view_mode];
       $cid = $this->createCacheId($cache_keys, $entity_cache_contexts);
-      $redirected_cid = $this->createRedirectedCacheId($cache_keys, $entity_cache_contexts);
+      $redirected_cid = NULL;
+      $additional_cache_contexts = $this->getAdditionalCacheContextsForEntity($this->entity);
+      if (count($additional_cache_contexts)) {
+        $redirected_cid = $this->createCacheId($cache_keys, Cache::mergeContexts($entity_cache_contexts, $additional_cache_contexts));
+      }
       $expected_cache_tags = Cache::mergeTags($cache_tag, $view_cache_tag, $this->getAdditionalCacheTagsForEntity($this->entity), array($render_cache_tag));
       $this->verifyRenderCache($cid, $expected_cache_tags, $redirected_cid);
     }

core/modules/user/src/Plugin/Field/FieldFormatter/AuthorFormatter.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\user\Plugin\Field\FieldFormatter;
 
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldItemListInterface;
 use Drupal\Core\Field\Plugin\Field\FieldFormatter\EntityReferenceFormatterBase;
@@ -31,18 +33,16 @@ class AuthorFormatter extends EntityReferenceFormatterBase {
   public function viewElements(FieldItemListInterface $items) {
     $elements = array();
 
-    foreach ($items as $delta => $item) {
+    foreach ($this->getEntitiesToView($items) as $delta => $entity) {
       /** @var $referenced_user \Drupal\user\UserInterface */
-      if ($referenced_user = $item->entity) {
-        $elements[$delta] = array(
-          '#theme' => 'username',
-          '#account' => $referenced_user,
-          '#link_options' => array('attributes' => array('rel' => 'author')),
-          '#cache' => array(
-            'tags' => $referenced_user->getCacheTags(),
-          ),
-        );
-      }
+      $elements[$delta] = array(
+        '#theme' => 'username',
+        '#account' => $entity,
+        '#link_options' => array('attributes' => array('rel' => 'author')),
+        '#cache' => array(
+          'tags' => $entity->getCacheTags(),
+        ),
+      );
     }
 
     return $elements;
@@ -55,4 +55,13 @@ class AuthorFormatter extends EntityReferenceFormatterBase {
     return $field_definition->getFieldStorageDefinition()->getSetting('target_type') == 'user';
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkAccess(EntityInterface $entity) {
+    // Always allow an entity author's username to be read, even if the current
+    // user does not have permission to view the entity author's profile.
+    return AccessResult::allowed();
+  }
+
 }

core/modules/views/src/Plugin/views/field/Field.php

@@ -18,6 +18,7 @@ use Drupal\Core\Form\FormHelper;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Language\LanguageInterface;
 use Drupal\Core\Language\LanguageManagerInterface;
+use Drupal\Core\Render\BubbleableMetadata;
 use Drupal\Core\Render\Element;
 use Drupal\Core\Render\RendererInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -731,26 +732,22 @@ class Field extends FieldPluginBase implements CacheablePluginInterface, MultiIt
     );
     $render_array = $entity->get($this->definition['field_name'])->view($display);
 
-    $items = array();
     if ($this->options['field_api_classes']) {
       return array(array('rendered' => $this->renderer->render($render_array)));
     }
 
-    foreach (Element::children($render_array) as $count) {
-      $items[$count]['rendered'] = $render_array[$count];
-      // FieldItemListInterface::view() adds an #access property to the render
-      // array that determines whether or not the current user is allowed to
-      // view the field in the context of the current entity. We need to respect
-      // this parameter when we pull out the children of the field array for
-      // rendering.
-      if (isset($render_array['#access'])) {
-        $items[$count]['rendered']['#access'] = $render_array['#access'];
-      }
-      // Only add the raw field items (for use in tokens) if the current user
-      // has access to view the field content.
-      if ((!isset($items[$count]['rendered']['#access']) || $items[$count]['rendered']['#access']) && !empty($render_array['#items'][$count])) {
-        $items[$count]['raw'] = $render_array['#items'][$count];
+    $items = array();
+    foreach (Element::children($render_array) as $delta) {
+      $items[$delta]['rendered'] = $render_array[$delta];
+      // Merge the cacheability metadata of the top-level render array into
+      // each child because they will most likely be rendered individually.
+      if (isset($render_array['#cache'])) {
+        BubbleableMetadata::createFromRenderArray($render_array)
+          ->merge(BubbleableMetadata::createFromRenderArray($items[$delta]['rendered']))
+          ->applyTo($items[$delta]['rendered']);
       }
+      // Add the raw field items (for use in tokens).
+      $items[$delta]['raw'] = $render_array['#items'][$delta];
     }
     return $items;
   }

core/tests/Drupal/Tests/Core/Access/AccessResultTest.php

@@ -796,6 +796,37 @@ class AccessResultTest extends UnitTestCase {
     }
   }
 
+  /**
+   * @covers ::orIf
+   *
+   * Tests the special case of ORing non-forbidden access results that are both
+   * cacheable but have different cacheability metadata.
+   * This is only the case for non-forbidden access results; we still abort the
+   * ORing process as soon as a forbidden access result is encountered. This is
+   * tested in ::testOrIf().
+   */
+  public function testOrIfCacheabilityMerging() {
+    $merge_both_directions = function(AccessResult $a, AccessResult $b) {
+      // A globally cacheable access result.
+      $a->setCacheMaxAge(3600);
+      // Another access result that is cacheable per permissions.
+      $b->setCacheMaxAge(86400)->cachePerPermissions();
+
+      $r1 = $a->orIf($b);
+      $this->assertTrue($r1->getCacheMaxAge() === 3600);
+      $this->assertSame(['user.permissions'], $r1->getCacheContexts());
+      $r2 = $b->orIf($a);
+      $this->assertTrue($r2->getCacheMaxAge() === 3600);
+      $this->assertSame(['user.permissions'], $r2->getCacheContexts());
+    };
+
+    // Merge either direction, get the same result.
+    $merge_both_directions(AccessResult::allowed(), AccessResult::allowed());
+    $merge_both_directions(AccessResult::allowed(), AccessResult::neutral());
+    $merge_both_directions(AccessResult::neutral(), AccessResult::neutral());
+    $merge_both_directions(AccessResult::neutral(), AccessResult::allowed());
+  }
+
   /**
    * Tests allowedIfHasPermissions().
    *