core/lib/Drupal/Core/Access/AccessResult.php | 6 ++++++ .../DefaultExceptionHtmlSubscriber.php | 5 +++-- .../Core/Render/MainContent/HtmlRenderer.php | 3 ++- core/lib/Drupal/Core/Routing/AccessAwareRouter.php | 22 +++++++++++++++++----- .../Core/Routing/AccessAwareRouterInterface.php | 5 +++++ core/modules/comment/src/CommentForm.php | 2 ++ 6 files changed, 35 insertions(+), 8 deletions(-) diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index 0a866e7..a6812eb 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -340,6 +340,12 @@ public function setCacheMaxAge($max_age) { */ public function cachePerRole() { $this->addCacheContexts(['user.roles']) + // @todo this is always needed when the 'user.roles' cache context is set: + // when caching per role, and something has changed in that role (e.g. + // a permission is granted or revoked), then the results for that role + // will change! To invalidate accordingly, we need this cache tag. + // This will be handled automatically once + // https://www.drupal.org/node/2428703 lands. ->addCacheTags(['config:user_role_list']); return $this; } diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php index 079c980..5861e85 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php @@ -7,6 +7,7 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Core\Url; use Drupal\Core\Utility\Error; use Psr\Log\LoggerInterface; @@ -111,10 +112,10 @@ protected function makeSubrequest(GetResponseForExceptionEvent $event, $url, $st } try { - // Persist the 'exception' and '_access_result' attributes to the + // Persist the 'exception' and access result attributes to the // subrequest. $sub_request->attributes->set('exception', $request->attributes->get('exception')); - $sub_request->attributes->set('_access_result', $request->attributes->get('_access_result')); + $sub_request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT)); // Carry over the session to the subrequest. if ($session = $request->getSession()) { diff --git a/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php b/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php index e9c21ba..d9d0858 100644 --- a/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php +++ b/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php @@ -18,6 +18,7 @@ use Drupal\Core\Render\Renderer; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Render\RenderEvents; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Core\Routing\RouteMatchInterface; use Symfony\Component\DependencyInjection\ContainerAwareTrait; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -147,7 +148,7 @@ public function renderResponse(array $main_content, Request $request, RouteMatch list($version) = explode('.', \Drupal::VERSION, 2); // Merge the request's access result cacheability metadata, if it has any. - $access_result = $request->attributes->get('_access_result'); + $access_result = $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT); if ($access_result instanceof CacheableInterface) { $cache_contexts = Cache::mergeTags($cache_tags, $access_result->getCacheContexts()); $cache_tags = Cache::mergeTags($cache_tags, $access_result->getCacheTags()); diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index 569d92c..87e30a3 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -92,18 +92,30 @@ public function matchRequest(Request $request) { // the current user. // @todo This will be removed in https://www.drupal.org/node/2229145. $this->account->id(); + $this->checkAccess($request); + // We can not return $parameters because the access check can change the + // request attributes. + return $request->attributes->all(); + } + + /** + * Apply access check service to the route and parameters in the request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to access check. + */ + protected function checkAccess(Request $request) { // The cacheability (if any) of this request's access check result must be // applied to the response. $access_result = $this->accessManager->checkRequest($request, $this->account, TRUE); - if (!$request->attributes->has('_access_result')) { - $request->attributes->set('_access_result', $access_result); + // Allow a master request to set the access result for a subrequest: if an + // access result attribute is already set, don't overwrite it. + if (!$request->attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { + $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); } if (!$access_result->isAllowed()) { throw new AccessDeniedHttpException(); } - // We can not return $parameters because the access check can change the - // request attributes. - return $request->attributes->all(); } /** diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php index 34fa75a..b6cfe4a 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php @@ -16,6 +16,11 @@ interface AccessAwareRouterInterface extends RouterInterface, RequestMatcherInterface { /** + * Attribute name of the access result for the request.. + */ + const ACCESS_RESULT = '_access_result'; + + /** * {@inheritdoc} * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException diff --git a/core/modules/comment/src/CommentForm.php b/core/modules/comment/src/CommentForm.php index 8f7368a..92db76c 100644 --- a/core/modules/comment/src/CommentForm.php +++ b/core/modules/comment/src/CommentForm.php @@ -96,6 +96,8 @@ public function form(array $form, FormStateInterface $form_state) { // Vary per role, because we check a permission above and attach an asset // library only for authenticated users. + // @todo: change to 'user.roles:authenticated' once + // https://www.drupal.org/node/2432837 lands. $form['#cache']['contexts'][] = 'user.roles'; // If not replying to a comment, use our dedicated page callback for new