Issue #2464659 by Wim Leers, rteijeiro: Routes that are varied by the...

Issue #2464659 by Wim Leers, rteijeiro: Routes that are varied by the 'user.permissions' cache context for anonymous users must also get the anonymous Role's cache tag

core/core.services.yml

@@ -952,6 +952,11 @@ services:
     tags:
       - { name: event_subscriber }
     arguments: ['@path.alias_manager', '@path_processor_manager', '@path.current']
+  anonymous_user_response_subscriber:
+    class: Drupal\Core\EventSubscriber\AnonymousUserResponseSubscriber
+    tags:
+      - { name: event_subscriber }
+    arguments: ['@current_user']
   finish_response_subscriber:
     class: Drupal\Core\EventSubscriber\FinishResponseSubscriber
     tags:

core/lib/Drupal/Core/EventSubscriber/AnonymousUserResponseSubscriber.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\EventSubscriber\AnonymousUserResponseSubscriber.
+ */
+
+namespace Drupal\Core\EventSubscriber;
+
+use Drupal\Core\Cache\Cache;
+use Drupal\Core\Session\AccountInterface;
+use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Response subscriber to handle finished responses for the anonymous user.
+ */
+class AnonymousUserResponseSubscriber implements EventSubscriberInterface {
+
+  /**
+   * The current user.
+   *
+   * @var \Drupal\Core\Session\AccountInterface
+   */
+  protected $currentUser;
+
+  /**
+   * Constructs an AnonymousUserResponseSubscriber object.
+   *
+   * @param \Drupal\Core\Session\AccountInterface $current_user
+   *   The current user.
+   */
+  public function __construct(AccountInterface $current_user) {
+    $this->currentUser = $current_user;
+  }
+
+  /**
+   * Adds a cache tag if the 'user.permissions' cache context is present.
+   *
+   * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event
+   *   The event to process.
+   */
+  public function onRespond(FilterResponseEvent $event) {
+    if (!$event->isMasterRequest()) {
+      return;
+    }
+
+    if (!$this->currentUser->isAnonymous()) {
+      return;
+    }
+
+    $response = $event->getResponse();
+
+    // The 'user.permissions' cache context ensures that if the permissions for
+    // a role are modified, users are not served stale render cache content.
+    // But, when entire responses are cached in reverse proxies, the value for
+    // the cache context is never calculated, causing the stale response to not
+    // be invalidated. Therefore, when varying by permissions and the current
+    // user is the anonymous user, also add the cache tag for the 'anonymous'
+    // role.
+    $cache_contexts = $response->headers->get('X-Drupal-Cache-Contexts');
+    if ($cache_contexts && in_array('user.permissions', explode(' ', $cache_contexts))) {
+      $cache_tags = ['config:user.role.anonymous'];
+      if ($response->headers->get('X-Drupal-Cache-Tags')) {
+        $existing_cache_tags = explode(' ', $response->headers->get('X-Drupal-Cache-Tags'));
+        $cache_tags = Cache::mergeTags($existing_cache_tags, $cache_tags);
+      }
+      $response->headers->set('X-Drupal-Cache-Tags', implode(' ', $cache_tags));
+    }
+  }
+
+  /**
+   * Registers the methods in this class that should be listeners.
+   *
+   * @return array
+   *   An array of event listener definitions.
+   */
+  public static function getSubscribedEvents() {
+    $events[KernelEvents::RESPONSE][] = ['onRespond', -5];
+    return $events;
+  }
+
+}

core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php

@@ -96,7 +96,7 @@ public function __construct(LanguageManagerInterface $language_manager, ConfigFa
    *   The event to process.
    */
   public function onRespond(FilterResponseEvent $event) {
-    if ($event->getRequestType() !== HttpKernelInterface::MASTER_REQUEST) {
+    if (!$event->isMasterRequest()) {
       return;
     }
 

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

@@ -266,7 +266,7 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) {
     $this->assertPageCacheContextsAndTags(
       Url::fromRoute('view.frontpage.page_1'),
       $cache_contexts,
-      Cache::mergeTags($empty_node_listing_cache_tags, ['rendered'])
+      Cache::mergeTags($empty_node_listing_cache_tags, ['rendered', 'config:user.role.anonymous'])
     );
 
     // Create some nodes on the frontpage view. Add more than 10 nodes in order
@@ -325,7 +325,7 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) {
     $this->assertPageCacheContextsAndTags(
       Url::fromRoute('view.frontpage.page_1'),
       $cache_contexts,
-      Cache::mergeTags($first_page_output_cache_tags, ['rendered'])
+      Cache::mergeTags($first_page_output_cache_tags, ['rendered', 'config:user.role.anonymous'])
     );
 
     // Second page.
@@ -344,6 +344,9 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) {
       'user_view',
       'user:0',
       'rendered',
+      // FinishResponseSubscriber adds this cache tag to responses that have the
+      // 'user.permissions' cache context for anonymous users.
+      'config:user.role.anonymous',
     ]);
 
     // Let's update a node title on the first page and ensure that the page

core/modules/system/src/Tests/Bootstrap/PageCacheTest.php

@@ -9,9 +9,11 @@
 
 use Drupal\Component\Datetime\DateTimePlus;
 use Drupal\Core\Routing\RequestContext;
+use Drupal\Core\Url;
 use Drupal\simpletest\WebTestBase;
 use Drupal\Core\Cache\Cache;
 use Drupal\user\Entity\Role;
+use Drupal\user\RoleInterface;
 
 /**
  * Enables the page cache and tests it with various HTTP requests.
@@ -236,6 +238,69 @@ function testPageCache() {
     $this->assertEqual($this->drupalGetHeader('Foo'), 'bar', 'Custom header was sent.');
   }
 
+  /**
+   * Tests the automatic presence of the anonymous role's cache tag.
+   *
+   * The 'user.permissions' cache context ensures that if the permissions for a
+   * role are modified, users are not served stale render cache content. But,
+   * when entire responses are cached in reverse proxies, the value for the
+   * cache context is never calculated, causing the stale response to not be
+   * invalidated. Therefore, when varying by permissions and the current user is
+   * the anonymous user, the cache tag for the 'anonymous' role must be added.
+   *
+   * This test verifies that, and it verifies that it does not happen for other
+   * roles.
+   */
+  function testPageCacheAnonymousRolePermissions() {
+    $config = $this->config('system.performance');
+    $config->set('cache.page.use_internal', 1);
+    $config->set('cache.page.max_age', 300);
+    $config->save();
+
+    $content_url = Url::fromRoute('system_test.permission_dependent_content');
+    $route_access_url = Url::fromRoute('system_test.permission_dependent_route_access');
+
+    // 1. anonymous user, without permission.
+    $this->drupalGet($content_url);
+    $this->assertText('Permission to pet llamas: no!');
+    $this->assertCacheContext('user.permissions');
+    $this->assertCacheTag('config:user.role.anonymous');
+    $this->drupalGet($route_access_url);
+    $this->assertCacheContext('user.permissions');
+    $this->assertCacheTag('config:user.role.anonymous');
+
+    // 2. anonymous user, with permission.
+    user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, ['pet llamas']);
+    $this->drupalGet($content_url);
+    $this->assertText('Permission to pet llamas: yes!');
+    $this->assertCacheContext('user.permissions');
+    $this->assertCacheTag('config:user.role.anonymous');
+    $this->drupalGet($route_access_url);
+    $this->assertCacheContext('user.permissions');
+    $this->assertCacheTag('config:user.role.anonymous');
+
+    // 3. authenticated user, without permission.
+    $auth_user = $this->drupalCreateUser();
+    $this->drupalLogin($auth_user);
+    $this->drupalGet($content_url);
+    $this->assertText('Permission to pet llamas: no!');
+    $this->assertCacheContext('user.permissions');
+    $this->assertNoCacheTag('config:user.role.authenticated');
+    $this->drupalGet($route_access_url);
+    $this->assertCacheContext('user.permissions');
+    $this->assertNoCacheTag('config:user.role.authenticated');
+
+    // 4. authenticated user, with permission.
+    user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, ['pet llamas']);
+    $this->drupalGet($content_url);
+    $this->assertText('Permission to pet llamas: yes!');
+    $this->assertCacheContext('user.permissions');
+    $this->assertNoCacheTag('config:user.role.authenticated');
+    $this->drupalGet($route_access_url);
+    $this->assertCacheContext('user.permissions');
+    $this->assertNoCacheTag('config:user.role.authenticated');
+  }
+
   /**
    * Tests the omit_vary_cookie setting.
    */

core/modules/system/src/Tests/Cache/PageCacheTagsIntegrationTest.php

@@ -103,6 +103,9 @@ function testPageCacheTags() {
       'config:system.menu.footer',
       'config:system.menu.main',
       'config:system.site',
+      // FinishResponseSubscriber adds this cache tag to responses that have the
+      // 'user.permissions' cache context for anonymous users.
+      'config:user.role.anonymous',
     ));
 
     // Full node page 2.
@@ -132,6 +135,9 @@ function testPageCacheTags() {
       'comment_list',
       'node_list',
       'config:views.view.comments_recent',
+      // FinishResponseSubscriber adds this cache tag to responses that have the
+      // 'user.permissions' cache context for anonymous users.
+      'config:user.role.anonymous',
     ));
   }
 

core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php

@@ -7,7 +7,10 @@
 
 namespace Drupal\system_test\Controller;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Render\RendererInterface;
+use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -34,6 +37,20 @@ class SystemTestController extends ControllerBase {
    */
   protected $persistentLock;
 
+  /**
+   * The current user.
+   *
+   * @var \Drupal\Core\Session\AccountInterface
+   */
+  protected $currentUser;
+
+  /**
+   * The renderer.
+   *
+   * @var \Drupal\Core\Render\RendererInterface
+   */
+  protected $renderer;
+
   /**
    * Constructs the SystemTestController.
    *
@@ -41,17 +58,28 @@ class SystemTestController extends ControllerBase {
    *   The lock service.
    * @param \Drupal\Core\Lock\LockBackendInterface $persistent_lock
    *   The persistent lock service.
+   * @param \Drupal\Core\Session\AccountInterface $current_user
+   *   The current user.
+   * @param \Drupal\Core\Render\RendererInterface $renderer
+   *   The renderer.
    */
-  public function __construct(LockBackendInterface $lock, LockBackendInterface $persistent_lock) {
+  public function __construct(LockBackendInterface $lock, LockBackendInterface $persistent_lock, AccountInterface $current_user, RendererInterface $renderer) {
     $this->lock = $lock;
     $this->persistentLock = $persistent_lock;
+    $this->currentUser = $current_user;
+    $this->renderer = $renderer;
   }
 
   /**
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container) {
-    return new static($container->get('lock'), $container->get('lock.persistent'));
+    return new static(
+      $container->get('lock'),
+      $container->get('lock.persistent'),
+      $container->get('current_user'),
+      $container->get('renderer')
+    );
   }
 
   /**
@@ -235,4 +263,28 @@ public function configureTitle($foo) {
     return 'Bar.' . $foo;
   }
 
+  /**
+   * Shows permission-dependent content.
+   *
+   * @return array
+   *   A render array.
+   */
+  public function permissionDependentContent() {
+    $build = [];
+
+    // The content depends on the access result.
+    $access = AccessResult::allowedIfHasPermission($this->currentUser, 'pet llamas');
+    $this->renderer->addDependency($build, $access);
+
+    // Build the content.
+    if ($access->isAllowed()) {
+      $build['allowed'] = ['#markup' => 'Permission to pet llamas: yes!'];
+    }
+    else {
+      $build['forbidden'] = ['#markup' => 'Permission to pet llamas: no!'];
+    }
+
+    return $build;
+  }
+
 }

core/modules/system/tests/modules/system_test/src/system_test.permissions.yml

+pet llamas:
+  title: 'Pet llamas'
+  description: 'Petting of llamas.'

core/modules/system/tests/modules/system_test/system_test.routing.yml

@@ -109,3 +109,16 @@ system_test.get_destination:
   requirements:
     _access: 'TRUE'
 
+system_test.permission_dependent_content:
+  path: '/system-test/permission-dependent-content'
+  defaults:
+    _controller: '\Drupal\system_test\Controller\SystemTestController::permissionDependentContent'
+  requirements:
+    _access: 'TRUE'
+
+system_test.permission_dependent_route_access:
+  path: '/system-test/permission-dependent-route-access'
+  defaults:
+    _controller: '\Drupal\system_test\Controller\SystemTestController::mainContentFallback'
+  requirements:
+    _permission: 'pet llamas'

core/modules/views/src/Tests/GlossaryTest.php

@@ -94,6 +94,9 @@ public function testGlossaryView() {
       'node_list',
       'user_list',
       'rendered',
+      // FinishResponseSubscriber adds this cache tag to responses that have the
+      // 'user.permissions' cache context for anonymous users.
+      'config:user.role.anonymous',
     ]);
   }