Issue #2869426 by Wim Leers, dawehner, tstoeckler, alexpott, Berdir, larowlan:...

Issue #2869426 by Wim Leers, dawehner, tstoeckler, alexpott, Berdir, larowlan: EntityResource should add _entity_access requirement to REST routes

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

@@ -89,12 +89,15 @@ public function applies(Route $route) {
   public function access(Request $request, AccountInterface $account) {
     $method = $request->getMethod();
 
+    // Read-only operations are always allowed.
+    if (in_array($method, ['GET', 'HEAD', 'OPTIONS', 'TRACE'], TRUE)) {
+      return AccessResult::allowed();
+    }
+
     // This check only applies if
-    // 1. this is a write operation
-    // 2. the user was successfully authenticated and
-    // 3. the request comes with a session cookie.
-    if (!in_array($method, ['GET', 'HEAD', 'OPTIONS', 'TRACE'])
-      && $account->isAuthenticated()
+    // 1. the user was successfully authenticated and
+    // 2. the request comes with a session cookie.
+    if ($account->isAuthenticated()
       && $this->sessionConfiguration->hasSession($request)
     ) {
       if (!$request->headers->has('X-CSRF-Token')) {

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

@@ -123,6 +123,21 @@ public function onExceptionSendChallenge(GetResponseForExceptionEvent $event) {
     }
   }
 
+  /**
+   * Detect disallowed authentication methods on access denied exceptions.
+   *
+   * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
+   */
+  public function onExceptionAccessDenied(GetResponseForExceptionEvent $event) {
+    if (isset($this->filter) && $event->isMasterRequest()) {
+      $request = $event->getRequest();
+      $exception = $event->getException();
+      if ($exception instanceof AccessDeniedHttpException && $this->authenticationProvider->applies($request) && !$this->filter->appliesToRoutedRequest($request, TRUE)) {
+        $event->setException(new AccessDeniedHttpException('The used authentication method is not allowed on this route.', $exception));
+      }
+    }
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -136,6 +151,7 @@ public static function getSubscribedEvents() {
     // Access check must be performed after routing.
     $events[KernelEvents::REQUEST][] = ['onKernelRequestFilterProvider', 31];
     $events[KernelEvents::EXCEPTION][] = ['onExceptionSendChallenge', 75];
+    $events[KernelEvents::EXCEPTION][] = ['onExceptionAccessDenied', 80];
     return $events;
   }
 

core/lib/Drupal/Core/Routing/AccessAwareRouter.php

@@ -4,6 +4,8 @@
 
 use Drupal\Core\Access\AccessManagerInterface;
 use Drupal\Core\Access\AccessResultReasonInterface;
+use Drupal\Core\Cache\CacheableDependencyInterface;
+use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException;
 use Drupal\Core\Session\AccountInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
@@ -111,7 +113,12 @@ protected function checkAccess(Request $request) {
       $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result);
     }
     if (!$access_result->isAllowed()) {
-      throw new AccessDeniedHttpException($access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL);
+      if ($access_result instanceof CacheableDependencyInterface && $request->isMethodCacheable()) {
+        throw new CacheableAccessDeniedHttpException($access_result, $access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL);
+      }
+      else {
+        throw new AccessDeniedHttpException($access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL);
+      }
     }
   }
 

core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php

@@ -12,6 +12,7 @@
 use Drupal\Core\Http\Exception\CacheableUnauthorizedHttpException;
 use Drupal\user\UserAuthInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
 
 /**
  * HTTP Basic authentication provider.
@@ -155,7 +156,9 @@ public function challengeException(Request $request, \Exception $previous) {
     $cacheability = CacheableMetadata::createFromObject($site_config)
       ->addCacheTags(['config:user.role.anonymous'])
       ->addCacheContexts(['user.roles:anonymous']);
-    return new CacheableUnauthorizedHttpException($cacheability, (string) $challenge, 'No authentication credentials provided.', $previous);
+    return $request->isMethodCacheable()
+      ? new CacheableUnauthorizedHttpException($cacheability, (string) $challenge, 'No authentication credentials provided.', $previous)
+      : new UnauthorizedHttpException((string) $challenge, 'No authentication credentials provided.', $previous);
   }
 
 }

core/modules/block/src/BlockAccessControlHandler.php

@@ -136,7 +136,10 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
         }
       }
       else {
-        $access = AccessResult::forbidden();
+        $reason = count($conditions) > 1
+          ? "One of the block visibility conditions ('%s') denied access."
+          : "The block visibility condition '%s' denied access.";
+        $access = AccessResult::forbidden(sprintf($reason, implode("', '", array_keys($conditions))));
       }
 
       $this->mergeCacheabilityFromConditions($access, $conditions);

core/modules/block/tests/src/Functional/Rest/BlockResourceTestBase.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\block\Functional\Rest;
 
 use Drupal\block\Entity\Block;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
 
 abstract class BlockResourceTestBase extends EntityResourceTestBase {
@@ -135,7 +136,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) {
 
     switch ($method) {
       case 'GET':
-        return "You are not authorized to view this block entity.";
+        return "The block visibility condition 'user_role' denied access.";
       default:
         return parent::getExpectedUnauthorizedAccessMessage($method);
     }
@@ -143,17 +144,25 @@ protected function getExpectedUnauthorizedAccessMessage($method) {
 
   /**
    * {@inheritdoc}
+   *
+   * @todo Fix this in https://www.drupal.org/node/2820315.
    */
   protected function getExpectedUnauthorizedAccessCacheability() {
+    return (new CacheableMetadata())
+      ->setCacheTags(['4xx-response', 'http_response'])
+      ->setCacheContexts(['user.roles']);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedUnauthorizedEntityAccessCacheability($is_authenticated) {
     // @see \Drupal\block\BlockAccessControlHandler::checkAccess()
-    return parent::getExpectedUnauthorizedAccessCacheability()
-      ->setCacheTags([
-        '4xx-response',
+    return parent::getExpectedUnauthorizedEntityAccessCacheability($is_authenticated)
+      ->addCacheTags([
         'config:block.block.llama',
-        'http_response',
-        static::$auth ? 'user:2' : 'user:0',
-      ])
-      ->setCacheContexts(['user.roles']);
+        $is_authenticated ? 'user:2' : 'user:0',
+      ]);
   }
 
 }

core/modules/block_content/tests/src/Functional/Rest/BlockContentResourceTestBase.php

@@ -180,9 +180,9 @@ protected function getExpectedUnauthorizedAccessMessage($method) {
   /**
    * {@inheritdoc}
    */
-  protected function getExpectedUnauthorizedAccessCacheability() {
+  protected function getExpectedUnauthorizedEntityAccessCacheability($is_authenticated) {
     // @see \Drupal\block_content\BlockContentAccessControlHandler()
-    return parent::getExpectedUnauthorizedAccessCacheability()
+    return parent::getExpectedUnauthorizedEntityAccessCacheability($is_authenticated)
       ->addCacheTags(['block_content:1']);
   }
 

core/modules/comment/tests/src/Functional/Rest/CommentResourceTestBase.php

@@ -319,8 +319,10 @@ protected function getExpectedUnauthorizedAccessMessage($method) {
         return "The 'post comments' permission is required.";
       case 'PATCH';
         return "The 'edit own comments' permission is required, the user must be the comment author, and the comment must be published.";
-      default:
-        return parent::getExpectedUnauthorizedAccessMessage($method);
+      case 'DELETE':
+        // \Drupal\comment\CommentAccessControlHandler::checkAccess() does not
+        // specify a reason for not allowing a comment to be deleted.
+        return '';
     }
   }
 
@@ -360,9 +362,9 @@ public function testPostSkipCommentApproval() {
   /**
    * {@inheritdoc}
    */
-  protected function getExpectedUnauthorizedAccessCacheability() {
+  protected function getExpectedUnauthorizedEntityAccessCacheability($is_authenticated) {
     // @see \Drupal\comment\CommentAccessControlHandler::checkAccess()
-    return parent::getExpectedUnauthorizedAccessCacheability()
+    return parent::getExpectedUnauthorizedEntityAccessCacheability($is_authenticated)
       ->addCacheTags(['comment:1']);
   }
 

core/modules/config/tests/config_test/tests/src/Functional/Rest/ConfigTestResourceTestBase.php

@@ -70,4 +70,20 @@ protected function getNormalizedPostEntity() {
     // @todo Update in https://www.drupal.org/node/2300677.
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedUnauthorizedAccessMessage($method) {
+    if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) {
+      return parent::getExpectedUnauthorizedAccessMessage($method);
+    }
+
+    switch ($method) {
+      case 'GET':
+        return "The 'view config_test' permission is required.";
+      default:
+        return parent::getExpectedUnauthorizedAccessMessage($method);
+    }
+  }
+
 }

core/modules/dblog/tests/src/Functional/DbLogResourceTest.php

@@ -67,7 +67,7 @@ public function testWatchdog() {
     $request_options = $this->getAuthenticationRequestOptions('GET');
 
     $response = $this->request('GET', $url, $request_options);
-    $this->assertResourceErrorResponse(403, "The 'restful get dblog' permission is required.", $response);
+    $this->assertResourceErrorResponse(403, "The 'restful get dblog' permission is required.", $response, ['4xx-response', 'http_response'], ['user.permissions'], FALSE, FALSE);
 
     // Create a user account that has the required permissions to read
     // the watchdog resource via the REST API.

core/modules/file/tests/src/Functional/Rest/FileResourceTestBase.php

@@ -218,6 +218,9 @@ public function testPost() {
    */
   protected function getExpectedUnauthorizedAccessMessage($method) {
     if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) {
+      if ($method === 'DELETE') {
+        return 'Only the file owner can update or delete the file entity.';
+      }
       return parent::getExpectedUnauthorizedAccessMessage($method);
     }
 

core/modules/language/tests/src/Functional/Hal/ConfigurableLanguageHalJsonBasicAuthTest.php

@@ -3,14 +3,14 @@
 namespace Drupal\Tests\language\Functional\Hal;
 
 use Drupal\Tests\language\Functional\Rest\ConfigurableLanguageResourceTestBase;
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 
 /**
  * @group hal
  */
 class ConfigurableLanguageHalJsonBasicAuthTest extends ConfigurableLanguageResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
 
   /**
    * {@inheritdoc}

core/modules/language/tests/src/Functional/Hal/ContentLanguageSettingsHalJsonBasicAuthTest.php

@@ -3,14 +3,14 @@
 namespace Drupal\Tests\language\Functional\Hal;
 
 use Drupal\Tests\language\Functional\Rest\ContentLanguageSettingsResourceTestBase;
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 
 /**
  * @group hal
  */
 class ContentLanguageSettingsHalJsonBasicAuthTest extends ContentLanguageSettingsResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
 
   /**
    * {@inheritdoc}

core/modules/language/tests/src/Functional/Rest/ConfigurableLanguageJsonBasicAuthTest.php

@@ -2,14 +2,14 @@
 
 namespace Drupal\Tests\language\Functional\Rest;
 
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 
 /**
  * @group rest
  */
 class ConfigurableLanguageJsonBasicAuthTest extends ConfigurableLanguageResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
 
   /**
    * {@inheritdoc}

core/modules/language/tests/src/Functional/Rest/ConfigurableLanguageXmlBasicAuthTest.php

@@ -2,7 +2,7 @@
 
 namespace Drupal\Tests\language\Functional\Rest;
 
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 use Drupal\Tests\rest\Functional\EntityResource\XmlEntityNormalizationQuirksTrait;
 
 /**
@@ -10,7 +10,7 @@
  */
 class ConfigurableLanguageXmlBasicAuthTest extends ConfigurableLanguageResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
   use XmlEntityNormalizationQuirksTrait;
 
   /**

core/modules/language/tests/src/Functional/Rest/ContentLanguageSettingsJsonBasicAuthTest.php

@@ -2,14 +2,14 @@
 
 namespace Drupal\Tests\language\Functional\Rest;
 
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 
 /**
  * @group rest
  */
 class ContentLanguageSettingsJsonBasicAuthTest extends ContentLanguageSettingsResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
 
   /**
    * {@inheritdoc}

core/modules/language/tests/src/Functional/Rest/ContentLanguageSettingsXmlBasicAuthTest.php

@@ -2,7 +2,7 @@
 
 namespace Drupal\Tests\language\Functional\Rest;
 
-use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait;
+use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait;
 use Drupal\Tests\rest\Functional\EntityResource\XmlEntityNormalizationQuirksTrait;
 
 /**
@@ -10,7 +10,7 @@
  */
 class ContentLanguageSettingsXmlBasicAuthTest extends ContentLanguageSettingsResourceTestBase {
 
-  use BasicAuthResourceWithInterfaceTranslationTestTrait;
+  use BasicAuthResourceTestTrait;
   use XmlEntityNormalizationQuirksTrait;
 
   /**

core/modules/media/tests/src/Functional/Rest/MediaResourceTestBase.php

@@ -436,9 +436,9 @@ protected function getExpectedNormalizedFileEntity() {
   /**
    * {@inheritdoc}
    */
-  protected function getExpectedUnauthorizedAccessCacheability() {
+  protected function getExpectedUnauthorizedEntityAccessCacheability($is_authenticated) {
     // @see \Drupal\media\MediaAccessControlHandler::checkAccess()
-    return parent::getExpectedUnauthorizedAccessCacheability()
+    return parent::getExpectedUnauthorizedEntityAccessCacheability($is_authenticated)
       ->addCacheTags(['media:1']);
   }
 

core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php

@@ -72,7 +72,8 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
         }
 
       case 'delete':
-        return AccessResult::allowedIf(!$entity->isNew() && $account->hasPermission('administer menu'))->cachePerPermissions()->addCacheableDependency($entity);
+        return AccessResult::allowedIfHasPermission($account, 'administer menu')
+          ->andIf(AccessResult::allowedIf(!$entity->isNew())->addCacheableDependency($entity));
     }
   }
 

core/modules/menu_link_content/tests/src/Functional/Rest/MenuLinkContentResourceTestBase.php

@@ -204,7 +204,7 @@ protected function getExpectedUnauthorizedAccessMessage($method) {
 
     switch ($method) {
       case 'DELETE':
-        return "You are not authorized to delete this menu_link_content entity.";
+        return "The 'administer menu' permission is required.";
       default:
         return parent::getExpectedUnauthorizedAccessMessage($method);
     }