.../Drupal/Core/Cache/CacheableDependencyTrait.php | 67 ++++++++++ .../Cache/RefinableCacheableDependencyTrait.php | 42 +------ .../DefaultExceptionHtmlSubscriber.php | 7 ++ .../EventSubscriber/ExceptionJsonSubscriber.php | 14 ++- .../CacheableAccessDeniedHttpException.php | 24 ++++ .../Exception/CacheableBadRequestHttpException.php | 24 ++++ .../Exception/CacheableConflictHttpException.php | 24 ++++ .../Http/Exception/CacheableGoneHttpException.php | 24 ++++ .../CacheableLengthRequiredHttpException.php | 24 ++++ .../CacheableMethodNotAllowedHttpException.php | 24 ++++ .../CacheableNotAcceptableHttpException.php | 24 ++++ .../Exception/CacheableNotFoundHttpException.php | 24 ++++ .../CacheablePreconditionFailedHttpException.php | 24 ++++ .../CacheablePreconditionRequiredHttpException.php | 24 ++++ .../CacheableServiceUnavailableHttpException.php | 24 ++++ .../CacheableTooManyRequestsHttpException.php | 24 ++++ .../CacheableUnauthorizedHttpException.php | 24 ++++ .../CacheableUnprocessableEntityHttpException.php | 24 ++++ .../CacheableUnsupportedMediaTypeHttpException.php | 24 ++++ .../src/Authentication/Provider/BasicAuth.php | 31 ++++- .../ConfigurableLanguageHalJsonBasicAuthTest.php | 4 +- ...ContentLanguageSettingsHalJsonBasicAuthTest.php | 4 +- .../src/EventSubscriber/RestConfigSubscriber.php | 1 + .../src/Plugin/rest/resource/EntityResource.php | 3 +- core/modules/rest/src/RequestHandler.php | 5 +- .../tests/modules/rest_test/rest_test.services.yml | 5 + .../RequestPolicy/DenyTestAuthRequests.php | 26 ++++ .../tests/src/Functional/AnonResourceTestTrait.php | 2 +- .../src/Functional/BasicAuthResourceTestTrait.php | 9 +- ...thResourceWithInterfaceTranslationTestTrait.php | 28 +++++ .../src/Functional/CookieResourceTestTrait.php | 24 +++- .../EntityResource/Block/BlockResourceTestBase.php | 15 +++ .../BlockContent/BlockContentResourceTestBase.php | 9 ++ .../Comment/CommentResourceTestBase.php | 9 ++ .../ConfigurableLanguageJsonBasicAuthTest.php | 4 +- .../ContentLanguageSettingsJsonBasicAuthTest.php | 4 +- .../EntityResource/EntityResourceTestBase.php | 140 +++++++++++---------- .../EntityResource/Media/MediaResourceTestBase.php | 9 ++ .../SearchPage/SearchPageResourceTestBase.php | 9 ++ .../EntityResource/User/UserResourceTestBase.php | 15 ++- .../rest/tests/src/Functional/ResourceTestBase.php | 97 ++++++++++++-- .../EventSubscriber/DefaultExceptionSubscriber.php | 23 +++- core/modules/user/src/UserAccessControlHandler.php | 2 +- 43 files changed, 826 insertions(+), 142 deletions(-) diff --git a/core/lib/Drupal/Core/Cache/CacheableDependencyTrait.php b/core/lib/Drupal/Core/Cache/CacheableDependencyTrait.php new file mode 100644 index 0000000..19cb0f2 --- /dev/null +++ b/core/lib/Drupal/Core/Cache/CacheableDependencyTrait.php @@ -0,0 +1,67 @@ +cacheContexts = $cacheability->getCacheContexts(); + $this->cacheTags = $cacheability->getCacheTags(); + $this->cacheMaxAge = $cacheability->getCacheMaxAge(); + return $this; + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + return $this->cacheTags; + } + + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return $this->cacheContexts; + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + return $this->cacheMaxAge; + } + +} diff --git a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php index 21b61b2..fcbc11f 100644 --- a/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php +++ b/core/lib/Drupal/Core/Cache/RefinableCacheableDependencyTrait.php @@ -7,47 +7,7 @@ */ trait RefinableCacheableDependencyTrait { - /** - * Cache contexts. - * - * @var string[] - */ - protected $cacheContexts = []; - - /** - * Cache tags. - * - * @var string[] - */ - protected $cacheTags = []; - - /** - * Cache max-age. - * - * @var int - */ - protected $cacheMaxAge = Cache::PERMANENT; - - /** - * {@inheritdoc} - */ - public function getCacheTags() { - return $this->cacheTags; - } - - /** - * {@inheritdoc} - */ - public function getCacheContexts() { - return $this->cacheContexts; - } - - /** - * {@inheritdoc} - */ - public function getCacheMaxAge() { - return $this->cacheMaxAge; - } + use CacheableDependencyTrait; /** * {@inheritdoc} diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php index f168a02..1e5df6d 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php @@ -2,6 +2,8 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Cache\CacheableResponseInterface; +use Drupal\Core\Cache\Exception\CacheableHttpExceptionInterface; use Drupal\Core\Routing\RedirectDestinationInterface; use Drupal\Core\Utility\Error; use Psr\Log\LoggerInterface; @@ -170,6 +172,11 @@ protected function makeSubrequest(GetResponseForExceptionEvent $event, $url, $st $response->setStatusCode($status_code); } + // Persist the exception's cacheability metadata, if any. + if ($response instanceof CacheableResponseInterface && $exception instanceof CacheableHttpExceptionInterface) { + $response->addCacheableDependency($exception->getCacheableMetadata()); + } + // Persist any special HTTP headers that were set on the exception. if ($exception instanceof HttpExceptionInterface) { $response->headers->add($exception->getHeaders()); diff --git a/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php index 6ccb743..620ad1b 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php @@ -2,6 +2,9 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\CacheableJsonResponse; +use Drupal\Core\Http\Exception\CacheableHttpExceptionInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; @@ -35,7 +38,16 @@ protected static function getPriority() { public function on4xx(GetResponseForExceptionEvent $event) { /** @var \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface $exception */ $exception = $event->getException(); - $response = new JsonResponse(['message' => $event->getException()->getMessage()], $exception->getStatusCode(), $exception->getHeaders()); + + // If the exception is cacheable, generate a cacheable response. + if ($exception instanceof CacheableDependencyInterface) { + $response = new CacheableJsonResponse(['message' => $event->getException()->getMessage()], $exception->getStatusCode(), $exception->getHeaders()); + $response->addCacheableDependency($exception); + } + else { + $response = new JsonResponse(['message' => $event->getException()->getMessage()], $exception->getStatusCode(), $exception->getHeaders()); + } + $event->setResponse($response); } diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableAccessDeniedHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableAccessDeniedHttpException.php new file mode 100644 index 0000000..e0fd5cb --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableAccessDeniedHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableBadRequestHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableBadRequestHttpException.php new file mode 100644 index 0000000..97d432a --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableBadRequestHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableConflictHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableConflictHttpException.php new file mode 100644 index 0000000..ca804fb --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableConflictHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableGoneHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableGoneHttpException.php new file mode 100644 index 0000000..4568c91 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableGoneHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableLengthRequiredHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableLengthRequiredHttpException.php new file mode 100644 index 0000000..a75f80a --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableLengthRequiredHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableMethodNotAllowedHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableMethodNotAllowedHttpException.php new file mode 100644 index 0000000..1ccc388 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableMethodNotAllowedHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableNotAcceptableHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableNotAcceptableHttpException.php new file mode 100644 index 0000000..94bf1c2 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableNotAcceptableHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableNotFoundHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableNotFoundHttpException.php new file mode 100644 index 0000000..9e5e136 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableNotFoundHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionFailedHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionFailedHttpException.php new file mode 100644 index 0000000..7921d3e --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionFailedHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionRequiredHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionRequiredHttpException.php new file mode 100644 index 0000000..d66b255 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheablePreconditionRequiredHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableServiceUnavailableHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableServiceUnavailableHttpException.php new file mode 100644 index 0000000..313b9ae --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableServiceUnavailableHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($retryAfter, $message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableTooManyRequestsHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableTooManyRequestsHttpException.php new file mode 100644 index 0000000..e709c0b --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableTooManyRequestsHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($retryAfter, $message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableUnauthorizedHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableUnauthorizedHttpException.php new file mode 100644 index 0000000..35dbd72 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableUnauthorizedHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($challenge, $message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableUnprocessableEntityHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableUnprocessableEntityHttpException.php new file mode 100644 index 0000000..655c67a --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableUnprocessableEntityHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableUnsupportedMediaTypeHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableUnsupportedMediaTypeHttpException.php new file mode 100644 index 0000000..c6f6023 --- /dev/null +++ b/core/lib/Drupal/Core/Http/Exception/CacheableUnsupportedMediaTypeHttpException.php @@ -0,0 +1,24 @@ +setCacheability($cacheability); + parent::__construct($message, $previous, $code); + } + +} diff --git a/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php b/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php old mode 100644 new mode 100755 index c72e3f0..01034f4 --- a/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php +++ b/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php @@ -5,12 +5,13 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Authentication\AuthenticationProviderInterface; use Drupal\Core\Authentication\AuthenticationProviderChallengeInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Flood\FloodInterface; +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. @@ -126,11 +127,35 @@ public function authenticate(Request $request) { * {@inheritdoc} */ public function challengeException(Request $request, \Exception $previous) { - $site_name = $this->configFactory->get('system.site')->get('name'); + $site_config = $this->configFactory->get('system.site'); + $site_name = $site_config->get('name'); $challenge = SafeMarkup::format('Basic realm="@realm"', [ '@realm' => !empty($site_name) ? $site_name : 'Access restricted', ]); - return new UnauthorizedHttpException((string) $challenge, 'No authentication credentials provided.', $previous); + + // A 403 is converted to a 401 here, but it doesn't matter what the + // cacheability was of the 403 exception: what matters here is that + // authentication credentials are missing, i.e. that this request was made + // as the anonymous user. + // Therefore, all we must do, is make this response: + // 1. vary by whether the current user has the 'anonymous' role or not. This + // works fine because: + // - Thanks to \Drupal\basic_auth\PageCache\DisallowBasicAuthRequests, + // Page Cache never caches a response whose request has Basic Auth + // credentials. + // - Dynamic Page Cache will cache a different result for when the + // request is unauthenticated (this 401) versus authenticated (some + // other response) + // 2. have the 'config:user.role.anonymous' cache tag, because the only + // reason this 401 would no longer be a 401 is if permissions for the + // 'anonymous' role change, causing that cache tag to be invalidated. + // @see \Drupal\Core\EventSubscriber\AuthenticationSubscriber::onExceptionSendChallenge() + // @see \Drupal\Core\EventSubscriber\ClientErrorResponseSubscriber() + // @see \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onAllResponds() + $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); } } diff --git a/core/modules/hal/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageHalJsonBasicAuthTest.php b/core/modules/hal/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageHalJsonBasicAuthTest.php index 71b41bc..dea13fc 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageHalJsonBasicAuthTest.php +++ b/core/modules/hal/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageHalJsonBasicAuthTest.php @@ -2,7 +2,7 @@ namespace Drupal\Tests\hal\Functional\EntityResource\ConfigurableLanguage; -use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait; +use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait; use Drupal\Tests\rest\Functional\EntityResource\ConfigurableLanguage\ConfigurableLanguageResourceTestBase; /** @@ -10,7 +10,7 @@ */ class ConfigurableLanguageHalJsonBasicAuthTest extends ConfigurableLanguageResourceTestBase { - use BasicAuthResourceTestTrait; + use BasicAuthResourceWithInterfaceTranslationTestTrait; /** * {@inheritdoc} diff --git a/core/modules/hal/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsHalJsonBasicAuthTest.php b/core/modules/hal/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsHalJsonBasicAuthTest.php index 942901b..71f4fab 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsHalJsonBasicAuthTest.php +++ b/core/modules/hal/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsHalJsonBasicAuthTest.php @@ -2,7 +2,7 @@ namespace Drupal\Tests\hal\Functional\EntityResource\ContentLanguageSettings; -use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait; +use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait; use Drupal\Tests\rest\Functional\EntityResource\ContentLanguageSettings\ContentLanguageSettingsResourceTestBase; /** @@ -10,7 +10,7 @@ */ class ContentLanguageSettingsHalJsonBasicAuthTest extends ContentLanguageSettingsResourceTestBase { - use BasicAuthResourceTestTrait; + use BasicAuthResourceWithInterfaceTranslationTestTrait; /** * {@inheritdoc} diff --git a/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php b/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php index d199545..fa7cc10 100644 --- a/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/RestConfigSubscriber.php @@ -37,6 +37,7 @@ public function __construct(RouteBuilderInterface $router_builder) { */ public function onSave(ConfigCrudEvent $event) { $saved_config = $event->getConfig(); + // @see \Drupal\rest\Plugin\rest\resource\EntityResource::permissions() if ($saved_config->getName() === 'rest.settings' && $event->isChanged('bc_entity_resource_permissions')) { $this->routerBuilder->setRebuildNeeded(); } diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 5d9849d..050c7f5 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -12,6 +12,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageException; use Drupal\Core\Field\FieldItemListInterface; +use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException; use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; @@ -122,7 +123,7 @@ public static function create(ContainerInterface $container, array $configuratio public function get(EntityInterface $entity) { $entity_access = $entity->access('view', NULL, TRUE); if (!$entity_access->isAllowed()) { - throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->generateFallbackAccessDeniedMessage($entity, 'view')); + throw new CacheableAccessDeniedHttpException($entity_access, $entity_access->getReason() ?: $this->generateFallbackAccessDeniedMessage($entity, 'view')); } $response = new ResourceResponse($entity, 200); diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index 4b7020c..bebf700 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -134,7 +134,10 @@ public function handle(RouteMatchInterface $route_match, Request $request) { $response = call_user_func_array([$resource, $method], $arguments); if ($response instanceof CacheableResponseInterface) { - // Add rest config's cache tags. + // Add global rest settings config's cache tag, for BC flags. + // @see \Drupal\rest\Plugin\rest\resource\EntityResource::permissions() + // @see \Drupal\rest\EventSubscriber\RestConfigSubscriber + $response->getCacheableMetadata()->addCacheTags(['config:rest.settings']); $response->addCacheableDependency($resource_config); } diff --git a/core/modules/rest/tests/modules/rest_test/rest_test.services.yml b/core/modules/rest/tests/modules/rest_test/rest_test.services.yml index ccdbeae..d316cf6 100644 --- a/core/modules/rest/tests/modules/rest_test/rest_test.services.yml +++ b/core/modules/rest/tests/modules/rest_test/rest_test.services.yml @@ -7,3 +7,8 @@ services: class: Drupal\rest_test\Authentication\Provider\TestAuthGlobal tags: - { name: authentication_provider, provider_id: 'rest_test_auth_global', global: TRUE } + rest_test.page_cache_request_policy.deny_test_auth_requests: + class: Drupal\rest_test\PageCache\RequestPolicy\DenyTestAuthRequests + public: false + tags: + - { name: page_cache_request_policy } diff --git a/core/modules/rest/tests/modules/rest_test/src/PageCache/RequestPolicy/DenyTestAuthRequests.php b/core/modules/rest/tests/modules/rest_test/src/PageCache/RequestPolicy/DenyTestAuthRequests.php new file mode 100644 index 0000000..8b547a9 --- /dev/null +++ b/core/modules/rest/tests/modules/rest_test/src/PageCache/RequestPolicy/DenyTestAuthRequests.php @@ -0,0 +1,26 @@ +headers->has('REST-test-auth') || $request->headers->has('REST-test-auth-global')) { + return self::DENY; + } + } + +} diff --git a/core/modules/rest/tests/src/Functional/AnonResourceTestTrait.php b/core/modules/rest/tests/src/Functional/AnonResourceTestTrait.php index b05ddf2..6527433 100644 --- a/core/modules/rest/tests/src/Functional/AnonResourceTestTrait.php +++ b/core/modules/rest/tests/src/Functional/AnonResourceTestTrait.php @@ -24,7 +24,7 @@ /** * {@inheritdoc} */ - protected function assertResponseWhenMissingAuthentication(ResponseInterface $response) { + protected function assertResponseWhenMissingAuthentication($method, ResponseInterface $response) { throw new \LogicException('When testing for anonymous users, authentication cannot be missing.'); } diff --git a/core/modules/rest/tests/src/Functional/BasicAuthResourceTestTrait.php b/core/modules/rest/tests/src/Functional/BasicAuthResourceTestTrait.php index 6f8c621..7a2e925 100644 --- a/core/modules/rest/tests/src/Functional/BasicAuthResourceTestTrait.php +++ b/core/modules/rest/tests/src/Functional/BasicAuthResourceTestTrait.php @@ -14,6 +14,8 @@ * authenticated, a 401 response must be sent. * - Because every request must send an authorization, there is no danger of * CSRF attacks. + * + * @see \Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait */ trait BasicAuthResourceTestTrait { @@ -31,8 +33,11 @@ protected function getAuthenticationRequestOptions($method) { /** * {@inheritdoc} */ - protected function assertResponseWhenMissingAuthentication(ResponseInterface $response) { - $this->assertResourceErrorResponse(401, 'No authentication credentials provided.', $response); + protected function assertResponseWhenMissingAuthentication($method, ResponseInterface $response) { + $expected_page_cache_header_value = $method === 'GET' ? 'MISS' : FALSE; + // @see \Drupal\basic_auth\Authentication\Provider\BasicAuth::challengeException() + $expected_dynamic_page_cache_header_value = $expected_page_cache_header_value; + $this->assertResourceErrorResponse(401, 'No authentication credentials provided.', $response, ['4xx-response', 'config:system.site', 'config:user.role.anonymous', 'http_response'], ['user.roles:anonymous'], $expected_page_cache_header_value, $expected_dynamic_page_cache_header_value); } /** diff --git a/core/modules/rest/tests/src/Functional/BasicAuthResourceWithInterfaceTranslationTestTrait.php b/core/modules/rest/tests/src/Functional/BasicAuthResourceWithInterfaceTranslationTestTrait.php new file mode 100644 index 0000000..37b8381 --- /dev/null +++ b/core/modules/rest/tests/src/Functional/BasicAuthResourceWithInterfaceTranslationTestTrait.php @@ -0,0 +1,28 @@ +assertResourceErrorResponse(401, 'No authentication credentials provided.', $response, ['4xx-response', 'config:system.site', 'config:user.role.anonymous', 'http_response'], ['languages:language_interface', 'user.roles:anonymous'], $expected_page_cache_header_value, $expected_page_cache_header_value); + } + +} diff --git a/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php b/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php old mode 100644 new mode 100755 index 3b7c3b4..f7f4eab --- a/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php +++ b/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php @@ -91,11 +91,31 @@ protected function getAuthenticationRequestOptions($method) { /** * {@inheritdoc} */ - protected function assertResponseWhenMissingAuthentication(ResponseInterface $response) { + protected function assertResponseWhenMissingAuthentication($method, ResponseInterface $response) { // Requests needing cookie authentication but missing it results in a 403 // response. The cookie authentication mechanism sets no response message. + // Hence, effectively, this is just the 403 response that one gets as the + // anonymous user trying to access a certain REST resource. + // @see \Drupal\user\Authentication\Provider\Cookie // @todo https://www.drupal.org/node/2847623 - $this->assertResourceErrorResponse(403, FALSE, $response); + if ($method === 'GET') { + $expected_cookie_403_cacheability = $this->getExpectedUnauthorizedAccessCacheability(); + // - \Drupal\Core\EventSubscriber\AnonymousUserResponseSubscriber applies + // to cacheable anonymous responses: it updates their cacheability. + // - A 403 response to a GET request is cacheable. + // Therefore we must update our cacheability expectations accordingly. + if (in_array('user.permissions', $expected_cookie_403_cacheability->getCacheContexts(), TRUE)) { + $expected_cookie_403_cacheability->addCacheTags(['config:user.role.anonymous']); + } + // @todo Fix \Drupal\block\BlockAccessControlHandler::mergeCacheabilityFromConditions() in https://www.drupal.org/node/2867881 + if (static::$entityTypeId === 'block') { + $expected_cookie_403_cacheability->setCacheTags(str_replace('user:2', 'user:0', $expected_cookie_403_cacheability->getCacheTags())); + } + $this->assertResourceErrorResponse(403, FALSE, $response, $expected_cookie_403_cacheability->getCacheTags(), $expected_cookie_403_cacheability->getCacheContexts(), 'MISS', 'MISS'); + } + else { + $this->assertResourceErrorResponse(403, FALSE, $response); + } } /** diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php old mode 100644 new mode 100755 index d86f9b1..13a4568 --- a/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Block/BlockResourceTestBase.php @@ -141,4 +141,19 @@ protected function getExpectedUnauthorizedAccessMessage($method) { } } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\block\BlockAccessControlHandler::checkAccess() + return parent::getExpectedUnauthorizedAccessCacheability() + ->setCacheTags([ + '4xx-response', + 'config:block.block.llama', + 'http_response', + static::$auth ? 'user:2' : 'user:0', + ]) + ->setCacheContexts(['user.roles']); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/BlockContent/BlockContentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/BlockContent/BlockContentResourceTestBase.php index 8b67cc6..5d7329f 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/BlockContent/BlockContentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/BlockContent/BlockContentResourceTestBase.php @@ -171,4 +171,13 @@ protected function getExpectedUnauthorizedAccessMessage($method) { return parent::getExpectedUnauthorizedAccessMessage($method); } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\block_content\BlockContentAccessControlHandler() + return parent::getExpectedUnauthorizedAccessCacheability() + ->addCacheTags(['block_content:1']); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php index bade2a7..60b5930 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php @@ -357,4 +357,13 @@ public function testPostSkipCommentApproval() { $this->assertTrue($unserialized->getStatus()); } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\comment\CommentAccessControlHandler::checkAccess() + return parent::getExpectedUnauthorizedAccessCacheability() + ->addCacheTags(['comment:1']); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageJsonBasicAuthTest.php b/core/modules/rest/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageJsonBasicAuthTest.php index e453f95..8b768f3 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageJsonBasicAuthTest.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/ConfigurableLanguage/ConfigurableLanguageJsonBasicAuthTest.php @@ -2,14 +2,14 @@ namespace Drupal\Tests\rest\Functional\EntityResource\ConfigurableLanguage; -use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait; +use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait; /** * @group rest */ class ConfigurableLanguageJsonBasicAuthTest extends ConfigurableLanguageResourceTestBase { - use BasicAuthResourceTestTrait; + use BasicAuthResourceWithInterfaceTranslationTestTrait; /** * {@inheritdoc} diff --git a/core/modules/rest/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsJsonBasicAuthTest.php b/core/modules/rest/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsJsonBasicAuthTest.php index 392213f..4644815 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsJsonBasicAuthTest.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsJsonBasicAuthTest.php @@ -2,14 +2,14 @@ namespace Drupal\Tests\rest\Functional\EntityResource\ContentLanguageSettings; -use Drupal\Tests\rest\Functional\BasicAuthResourceTestTrait; +use Drupal\Tests\rest\Functional\BasicAuthResourceWithInterfaceTranslationTestTrait; /** * @group rest */ class ContentLanguageSettingsJsonBasicAuthTest extends ContentLanguageSettingsResourceTestBase { - use BasicAuthResourceTestTrait; + use BasicAuthResourceWithInterfaceTranslationTestTrait; /** * {@inheritdoc} diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 6faf028..0a78115 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -5,6 +5,7 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableResponseInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\Entity\ConfigEntityInterface; use Drupal\Core\Entity\ContentEntityNullStorage; use Drupal\Core\Entity\FieldableEntityInterface; @@ -260,6 +261,17 @@ protected function getExpectedUnauthorizedAccessMessage($method) { } /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + return (new CacheableMetadata()) + ->setCacheTags(static::$auth + ? ['4xx-response', 'http_response'] + : ['4xx-response', 'config:user.role.anonymous', 'http_response']) + ->setCacheContexts(['user.permissions']); + } + + /** * The expected cache tags for the GET/HEAD response of the test entity. * * @see ::testGet @@ -269,6 +281,9 @@ protected function getExpectedUnauthorizedAccessMessage($method) { protected function getExpectedCacheTags() { $expected_cache_tags = [ 'config:rest.resource.entity.' . static::$entityTypeId, + // Necessary for 'bc_entity_resource_permissions'. + // @see \Drupal\rest\Plugin\rest\resource\EntityResource::permissions() + 'config:rest.settings', ]; if (!static::$auth) { $expected_cache_tags[] = 'config:user.role.anonymous'; @@ -351,7 +366,7 @@ public function testGet() { // response. if (static::$auth) { $response = $this->request('GET', $url, $request_options); - $this->assertResponseWhenMissingAuthentication($response); + $this->assertResponseWhenMissingAuthentication('GET', $response); } $request_options[RequestOptions::HEADERS]['REST-test-auth'] = '1'; @@ -373,7 +388,8 @@ public function testGet() { // DX: 403 when unauthorized. $response = $this->request('GET', $url, $request_options); - $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); + $expected_403_cacheability = $this->getExpectedUnauthorizedAccessCacheability(); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response, $expected_403_cacheability->getCacheTags(), $expected_403_cacheability->getCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); $this->assertArrayNotHasKey('Link', $response->getHeaders()); @@ -382,60 +398,47 @@ public function testGet() { // 200 for well-formed HEAD request. $response = $this->request('HEAD', $url, $request_options); - $this->assertResourceResponse(200, '', $response); - $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); - $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); - if (!$this->account) { - $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Cache')); - } - else { - $this->assertFalse($response->hasHeader('X-Drupal-Cache')); - } + $this->assertResourceResponse(200, '', $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); $head_headers = $response->getHeaders(); // 200 for well-formed GET request. Page Cache hit because of HEAD request. // Same for Dynamic Page Cache hit. $response = $this->request('GET', $url, $request_options); - $this->assertResourceResponse(200, FALSE, $response); - $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); - if (!static::$auth) { - $this->assertSame(['HIT'], $response->getHeader('X-Drupal-Cache')); - $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); - } - else { - $this->assertFalse($response->hasHeader('X-Drupal-Cache')); - $this->assertSame(['HIT'], $response->getHeader('X-Drupal-Dynamic-Cache')); - // Assert that Dynamic Page Cache did not store a ResourceResponse object, - // which needs serialization after every cache hit. Instead, it should - // contain a flattened response. Otherwise performance suffers. - // @see \Drupal\rest\EventSubscriber\ResourceResponseSubscriber::flattenResponse() - $cache_items = $this->container->get('database') - ->query("SELECT cid, data FROM {cache_dynamic_page_cache} WHERE cid LIKE :pattern", [ - ':pattern' => '%[route]=rest.%', - ]) - ->fetchAllAssoc('cid'); - $this->assertCount(2, $cache_items); - $found_cache_redirect = FALSE; - $found_cached_response = FALSE; - foreach ($cache_items as $cid => $cache_item) { - $cached_data = unserialize($cache_item->data); - if (!isset($cached_data['#cache_redirect'])) { - $found_cached_response = TRUE; - $cached_response = $cached_data['#response']; - $this->assertNotInstanceOf(ResourceResponseInterface::class, $cached_response); - $this->assertInstanceOf(CacheableResponseInterface::class, $cached_response); + $this->assertResourceResponse(200, FALSE, $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), static::$auth ? FALSE : 'HIT', static::$auth ? 'HIT' : 'MISS'); + // Assert that Dynamic Page Cache did not store a ResourceResponse object, + // which needs serialization after every cache hit. Instead, it should + // contain a flattened response. Otherwise performance suffers. + // @see \Drupal\rest\EventSubscriber\ResourceResponseSubscriber::flattenResponse() + $cache_items = $this->container->get('database') + ->query("SELECT cid, data FROM {cache_dynamic_page_cache} WHERE cid LIKE :pattern", [ + ':pattern' => '%[route]=rest.%', + ]) + ->fetchAllAssoc('cid'); + $this->assertTrue(count($cache_items) >= 2); + $found_cache_redirect = FALSE; + $found_cached_200_response = FALSE; + $other_cached_responses_are_4xx = TRUE; + foreach ($cache_items as $cid => $cache_item) { + $cached_data = unserialize($cache_item->data); + if (!isset($cached_data['#cache_redirect'])) { + $cached_response = $cached_data['#response']; + if ($cached_response->getStatusCode() === 200) { + $found_cached_200_response = TRUE; } - else { - $found_cache_redirect = TRUE; + elseif (!$cached_response->isClientError()) { + $other_cached_responses_are_4xx = FALSE; } + $this->assertNotInstanceOf(ResourceResponseInterface::class, $cached_response); + $this->assertInstanceOf(CacheableResponseInterface::class, $cached_response); + } + else { + $found_cache_redirect = TRUE; } - $this->assertTrue($found_cache_redirect); - $this->assertTrue($found_cached_response); } - $cache_tags_header_value = $response->getHeader('X-Drupal-Cache-Tags')[0]; - $this->assertEquals($this->getExpectedCacheTags(), empty($cache_tags_header_value) ? [] : explode(' ', $cache_tags_header_value)); - $cache_contexts_header_value = $response->getHeader('X-Drupal-Cache-Contexts')[0]; - $this->assertEquals($this->getExpectedCacheContexts(), empty($cache_contexts_header_value) ? [] : explode(' ', $cache_contexts_header_value)); + $this->assertTrue($found_cache_redirect); + $this->assertTrue($found_cached_200_response); + $this->assertTrue($other_cached_responses_are_4xx); + // Sort the serialization data first so we can do an identical comparison // for the keys with the array order the same (it needs to match with // identical comparison). @@ -495,7 +498,7 @@ public function testGet() { $response = $this->request('GET', $url, $request_options); - $this->assertResourceResponse(200, FALSE, $response); + $this->assertResourceResponse(200, FALSE, $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); // Again do an identical comparison, but this time transform the expected @@ -529,7 +532,7 @@ public function testGet() { $response = $this->request('GET', $url, $request_options); - $this->assertResourceResponse(200, FALSE, $response); + $this->assertResourceResponse(200, FALSE, $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); // This ensures the BC layer for bc_timestamp_normalizer_unix works as @@ -565,7 +568,21 @@ public function testGet() { // 200 for well-formed request. $response = $this->request('GET', $url, $request_options); - $this->assertResourceResponse(200, FALSE, $response); + $expected_cache_tags = $this->getExpectedCacheTags(); + $expected_cache_contexts = $this->getExpectedCacheContexts(); + // @todo Fix BlockAccessControlHandler::mergeCacheabilityFromConditions() in + // https://www.drupal.org/node/2867881 + if (static::$entityTypeId === 'block') { + $expected_cache_contexts = Cache::mergeContexts($expected_cache_contexts, ['user.permissions']); + } + // \Drupal\Core\EventSubscriber\AnonymousUserResponseSubscriber applies to + // cacheable anonymous responses: it updates their cacheability. Therefore + // we must update our cacheability expectations for anonymous responses + // accordingly. + if (!static::$auth && in_array('user.permissions', $expected_cache_contexts, TRUE)) { + $expected_cache_tags = Cache::mergeTags($expected_cache_tags, ['config:user.role.anonymous']); + } + $this->assertResourceResponse(200, FALSE, $response, $expected_cache_tags, $expected_cache_contexts, static::$auth ? FALSE : 'MISS', 'MISS'); $this->resourceConfigStorage->load(static::$resourceConfigId)->disable()->save(); @@ -582,7 +599,7 @@ public function testGet() { // DX: upon re-enabling a resource, immediate 200. $response = $this->request('GET', $url, $request_options); - $this->assertResourceResponse(200, FALSE, $response); + $this->assertResourceResponse(200, FALSE, $response, $expected_cache_tags, $expected_cache_contexts, static::$auth ? FALSE : 'MISS', 'MISS'); $this->resourceConfigStorage->load(static::$resourceConfigId)->delete(); @@ -759,7 +776,7 @@ public function testPost() { // DX: forgetting authentication: authentication provider-specific error // response. $response = $this->request('POST', $url, $request_options); - $this->assertResponseWhenMissingAuthentication($response); + $this->assertResponseWhenMissingAuthentication('POST', $response); } @@ -998,7 +1015,7 @@ public function testPatch() { // DX: forgetting authentication: authentication provider-specific error // response. $response = $this->request('PATCH', $url, $request_options); - $this->assertResponseWhenMissingAuthentication($response); + $this->assertResponseWhenMissingAuthentication('PATCH', $response); } @@ -1169,7 +1186,7 @@ public function testDelete() { // DX: forgetting authentication: authentication provider-specific error // response. $response = $this->request('DELETE', $url, $request_options); - $this->assertResponseWhenMissingAuthentication($response); + $this->assertResponseWhenMissingAuthentication('DELETE', $response); } @@ -1191,14 +1208,7 @@ public function testDelete() { // 204 for well-formed request. $response = $this->request('DELETE', $url, $request_options); - $this->assertSame(204, $response->getStatusCode()); - // DELETE responses should not include a Content-Type header. But Apache - // sets it to 'text/html' by default. We also cannot detect the presence of - // Apache either here in the CLI. For now having this documented here is all - // we can do. - // $this->assertSame(FALSE, $response->hasHeader('Content-Type')); - $this->assertSame('', (string) $response->getBody()); - $this->assertFalse($response->hasHeader('X-Drupal-Cache')); + $this->assertResourceResponse(204, '', $response); $this->config('rest.settings')->set('bc_entity_resource_permissions', TRUE)->save(TRUE); @@ -1217,11 +1227,7 @@ public function testDelete() { // 204 for well-formed request. $response = $this->request('DELETE', $url, $request_options); - $this->assertSame(204, $response->getStatusCode()); - // @todo Uncomment the following line when https://www.drupal.org/node/2821711 is fixed. - // $this->assertSame(FALSE, $response->hasHeader('Content-Type')); - $this->assertSame('', (string) $response->getBody()); - $this->assertFalse($response->hasHeader('X-Drupal-Cache')); + $this->assertResourceResponse(204, '', $response); } /** diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php index 475a1ca..bf94261 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Media/MediaResourceTestBase.php @@ -261,4 +261,13 @@ public function testPost() { $this->markTestSkipped('POSTing File Media items is not supported until https://www.drupal.org/node/1927648 is solved.'); } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\media\MediaAccessControlHandler::checkAccess() + return parent::getExpectedUnauthorizedAccessCacheability() + ->addCacheTags(['media:1']); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/SearchPage/SearchPageResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/SearchPage/SearchPageResourceTestBase.php index e81993f..eca9ca8 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/SearchPage/SearchPageResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/SearchPage/SearchPageResourceTestBase.php @@ -99,4 +99,13 @@ protected function getExpectedUnauthorizedAccessMessage($method) { } } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\search\SearchPageAccessControlHandler::checkAccess() + return parent::getExpectedUnauthorizedAccessCacheability() + ->addCacheTags(['config:search.page.hinode_search']); + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php index caf2909..b7367df 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php @@ -162,7 +162,7 @@ public function testPatchDxForSecuritySensitiveBaseFields() { // DX: 422 when changing email without providing the password. $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response, FALSE, FALSE, FALSE, FALSE); $normalization['pass'] = [['existing' => 'wrong']]; @@ -170,7 +170,7 @@ public function testPatchDxForSecuritySensitiveBaseFields() { // DX: 422 when changing email while providing a wrong password. $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the Email.\n", $response, FALSE, FALSE, FALSE, FALSE); $normalization['pass'] = [['existing' => $this->account->passRaw]]; @@ -191,7 +191,7 @@ public function testPatchDxForSecuritySensitiveBaseFields() { // DX: 422 when changing password without providing the current password. $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\npass: Your current password is missing or incorrect; it's required to change the Password.\n", $response); + $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\npass: Your current password is missing or incorrect; it's required to change the Password.\n", $response, FALSE, FALSE, FALSE, FALSE); $normalization['pass'][0]['existing'] = $this->account->pass_raw; @@ -279,4 +279,13 @@ protected function getExpectedUnauthorizedAccessMessage($method) { } } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessCacheability() { + // @see \Drupal\user\UserAccessControlHandler::checkAccess() + return parent::getExpectedUnauthorizedAccessCacheability() + ->addCacheTags(['user:3']); + } + } diff --git a/core/modules/rest/tests/src/Functional/ResourceTestBase.php b/core/modules/rest/tests/src/Functional/ResourceTestBase.php index 0a0e34c..c7dfc69 100644 --- a/core/modules/rest/tests/src/Functional/ResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/ResourceTestBase.php @@ -214,8 +214,13 @@ protected function getExpectedUnauthorizedAccessMessage($method) { /** * Verifies the error response in case of missing authentication. + * + * @param string $method + * HTTP method. + * @param \Psr\Http\Message\ResponseInterface $response + * The response to assert. */ - abstract protected function assertResponseWhenMissingAuthentication(ResponseInterface $response); + abstract protected function assertResponseWhenMissingAuthentication($method, ResponseInterface $response); /** * Asserts normalization-specific edge cases. @@ -250,6 +255,14 @@ protected function getExpectedUnauthorizedAccessMessage($method) { abstract protected function assertAuthenticationEdgeCases($method, Url $url, array $request_options); /** + * Returns the expected cacheability of an unauthorized access response. + * + * @return \Drupal\Core\Cache\RefinableCacheableDependencyInterface + * The expected cacheability. + */ + abstract protected function getExpectedUnauthorizedAccessCacheability(); + + /** * Initializes authentication. * * E.g. for cookie authentication, we first need to get a cookie. @@ -348,12 +361,67 @@ protected function request($method, Url $url, array $request_options) { * The expected response body. FALSE in case this should not be asserted. * @param \Psr\Http\Message\ResponseInterface $response * The response to assert. + * @param string[]|false $expected_cache_tags + * (optional) The expected cache tags in the X-Drupal-Cache-Tags response + * header, or FALSE if that header should be absent. Defaults to FALSE. + * @param string[]|false $expected_cache_contexts + * (optional) The expected cache contexts in the X-Drupal-Cache-Contexts + * response header, or FALSE if that header should be absent. Defaults to + * FALSE. + * @param string|false $expected_page_cache_header_value + * (optional) The expected X-Drupal-Cache response header value, or FALSE if + * that header should be absent. Possible strings: 'MISS', 'HIT'. Defaults + * to FALSE. + * @param string|false $expected_dynamic_page_cache_header_value + * (optional) The expected X-Drupal-Dynamic-Cache response header value, or + * FALSE if that header should be absent. Possible strings: 'MISS', 'HIT'. + * Defaults to FALSE. */ - protected function assertResourceResponse($expected_status_code, $expected_body, ResponseInterface $response) { + protected function assertResourceResponse($expected_status_code, $expected_body, ResponseInterface $response, $expected_cache_tags = FALSE, $expected_cache_contexts = FALSE, $expected_page_cache_header_value = FALSE, $expected_dynamic_page_cache_header_value = FALSE) { $this->assertSame($expected_status_code, $response->getStatusCode()); - $this->assertSame([static::$mimeType], $response->getHeader('Content-Type')); - if ($expected_body !== FALSE) { - $this->assertSame($expected_body, (string) $response->getBody()); + if ($expected_status_code === 204) { + // DELETE responses should not include a Content-Type header. But Apache + // sets it to 'text/html' by default. We also cannot detect the presence + // of Apache either here in the CLI. For now having this documented here + // is all we can do. + // $this->assertSame(FALSE, $response->hasHeader('Content-Type')); + $this->assertSame('', (string) $response->getBody()); + } + else { + $this->assertSame([static::$mimeType], $response->getHeader('Content-Type')); + if ($expected_body !== FALSE) { + $this->assertSame($expected_body, (string) $response->getBody()); + } + } + + // Expected cache tags: X-Drupal-Cache-Tags header. + $this->assertSame($expected_cache_tags !== FALSE, $response->hasHeader('X-Drupal-Cache-Tags')); + if (is_array($expected_cache_tags)) { + $this->assertSame($expected_cache_tags, explode(' ', $response->getHeader('X-Drupal-Cache-Tags')[0])); + } + + // Expected cache contexts: X-Drupal-Cache-Contexts header. + $this->assertSame($expected_cache_contexts !== FALSE, $response->hasHeader('X-Drupal-Cache-Contexts')); + if (is_array($expected_cache_contexts)) { + $this->assertSame($expected_cache_contexts, explode(' ', $response->getHeader('X-Drupal-Cache-Contexts')[0])); + } + + // Expected Page Cache header value: X-Drupal-Cache header. + if ($expected_page_cache_header_value !== FALSE) { + $this->assertTrue($response->hasHeader('X-Drupal-Cache')); + $this->assertSame($expected_page_cache_header_value, $response->getHeader('X-Drupal-Cache')[0]); + } + else { + $this->assertFalse($response->hasHeader('X-Drupal-Cache')); + } + + // Expected Dynamic Page Cache header value: X-Drupal-Dynamic-Cache header. + if ($expected_dynamic_page_cache_header_value !== FALSE) { + $this->assertTrue($response->hasHeader('X-Drupal-Dynamic-Cache')); + $this->assertSame($expected_dynamic_page_cache_header_value, $response->getHeader('X-Drupal-Dynamic-Cache')[0]); + } + else { + $this->assertFalse($response->hasHeader('X-Drupal-Dynamic-Cache')); } } @@ -366,10 +434,25 @@ protected function assertResourceResponse($expected_status_code, $expected_body, * The expected error message. * @param \Psr\Http\Message\ResponseInterface $response * The error response to assert. + * @param string[]|false $expected_cache_tags + * (optional) The expected cache tags in the X-Drupal-Cache-Tags response + * header, or FALSE if that header should be absent. Defaults to FALSE. + * @param string[]|false $expected_cache_contexts + * (optional) The expected cache contexts in the X-Drupal-Cache-Contexts + * response header, or FALSE if that header should be absent. Defaults to + * FALSE. + * @param string|false $expected_page_cache_header_value + * (optional) The expected X-Drupal-Cache response header value, or FALSE if + * that header should be absent. Possible strings: 'MISS', 'HIT'. Defaults + * to FALSE. + * @param string|false $expected_dynamic_page_cache_header_value + * (optional) The expected X-Drupal-Dynamic-Cache response header value, or + * FALSE if that header should be absent. Possible strings: 'MISS', 'HIT'. + * Defaults to FALSE. */ - protected function assertResourceErrorResponse($expected_status_code, $expected_message, ResponseInterface $response) { + protected function assertResourceErrorResponse($expected_status_code, $expected_message, ResponseInterface $response, $expected_cache_tags = FALSE, $expected_cache_contexts = FALSE, $expected_page_cache_header_value = FALSE, $expected_dynamic_page_cache_header_value = FALSE) { $expected_body = ($expected_message !== FALSE) ? $this->serializer->encode(['message' => $expected_message], static::$format) : FALSE; - $this->assertResourceResponse($expected_status_code, $expected_body, $response); + $this->assertResourceResponse($expected_status_code, $expected_body, $response, $expected_cache_tags, $expected_cache_contexts, $expected_page_cache_header_value, $expected_dynamic_page_cache_header_value); } /** diff --git a/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php index a1e7bad..9ab7b6e 100644 --- a/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php +++ b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php @@ -2,7 +2,10 @@ namespace Drupal\serialization\EventSubscriber; +use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\CacheableResponse; use Drupal\Core\EventSubscriber\HttpExceptionSubscriberBase; +use Drupal\Core\Http\Exception\CacheableHttpExceptionInterface; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\Serializer\SerializerInterface; @@ -51,8 +54,12 @@ protected function getHandledFormats() { */ protected static function getPriority() { // This will fire after the most common HTML handler, since HTML requests - // are still more common than HTTP requests. - return -75; + // are still more common than HTTP requests. But it has a lower priority + // than \Drupal\Core\EventSubscriber\ExceptionJsonSubscriber::on4xx(), so + // that this also handles the 'json' format. Then all serialization formats + // (::getHandledFormats()) are handled by this exception subscriber, which + // results in better consistency. + return -70; } /** @@ -67,14 +74,22 @@ public function on4xx(GetResponseForExceptionEvent $event) { $request = $event->getRequest(); $format = $request->getRequestFormat(); - $content = ['message' => $event->getException()->getMessage()]; + $content = ['message' => $exception->getMessage()]; $encoded_content = $this->serializer->serialize($content, $format); $headers = $exception->getHeaders(); // Add the MIME type from the request to send back in the header. $headers['Content-Type'] = $request->getMimeType($format); - $response = new Response($encoded_content, $exception->getStatusCode(), $headers); + // If the exception is cacheable, generate a cacheable response. + if ($exception instanceof CacheableDependencyInterface) { + $response = new CacheableResponse($encoded_content, $exception->getStatusCode(), $headers); + $response->addCacheableDependency($exception); + } + else { + $response = new Response($encoded_content, $exception->getStatusCode(), $headers); + } + $event->setResponse($response); } diff --git a/core/modules/user/src/UserAccessControlHandler.php b/core/modules/user/src/UserAccessControlHandler.php index 712b32a..8ff01d1 100644 --- a/core/modules/user/src/UserAccessControlHandler.php +++ b/core/modules/user/src/UserAccessControlHandler.php @@ -58,7 +58,7 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter return AccessResult::allowed()->cachePerUser(); } else { - return AccessResultNeutral::neutral("The 'access user profiles' permission is required and the user must be active."); + return AccessResultNeutral::neutral("The 'access user profiles' permission is required and the user must be active.")->cachePerPermissions()->addCacheableDependency($entity); } break;