diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index 8148d4ca580b5bd95c46ffc843c917278e7c75ff..f67b9f5296fa2cfd72930f137775ba7332531316 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -51,11 +51,16 @@ public static function allowed() { /** * Creates an AccessResultInterface object with isForbidden() === TRUE. * + * @param string|null $reason + * (optional) The reason why access is forbidden. Intended for developers, + * hence not translatable. + * * @return \Drupal\Core\Access\AccessResult * isForbidden() will be TRUE. */ - public static function forbidden() { - return new AccessResultForbidden(); + public static function forbidden($reason = NULL) { + assert('is_string($reason) || is_null($reason)'); + return new AccessResultForbidden($reason); } /** @@ -334,8 +339,16 @@ public function andIf(AccessResultInterface $other) { if ($this->isForbidden() || $other->isForbidden()) { $result = static::forbidden(); if (!$this->isForbidden()) { + if ($other instanceof AccessResultReasonInterface) { + $result->setReason($other->getReason()); + } $merge_other = TRUE; } + else { + if ($this instanceof AccessResultReasonInterface) { + $result->setReason($this->getReason()); + } + } } elseif ($this->isAllowed() && $other->isAllowed()) { $result = static::allowed(); diff --git a/core/lib/Drupal/Core/Access/AccessResultForbidden.php b/core/lib/Drupal/Core/Access/AccessResultForbidden.php index 4dc0120a9d77bd1abc20a23a1b7acdd9cf93353a..2dc913761c2635e641e8f50f9c932168fcccecda 100644 --- a/core/lib/Drupal/Core/Access/AccessResultForbidden.php +++ b/core/lib/Drupal/Core/Access/AccessResultForbidden.php @@ -5,7 +5,25 @@ /** * Value object indicating a forbidden access result, with cacheability metadata. */ -class AccessResultForbidden extends AccessResult { +class AccessResultForbidden extends AccessResult implements AccessResultReasonInterface { + + /** + * The reason why access is forbidden. For use in error messages. + * + * @var string|null + */ + protected $reason; + + /** + * Constructs a new AccessResultForbidden instance. + * + * @param null|string $reason + * (optional) a message to provide details about this access result + */ + public function __construct($reason = NULL) { + $this->reason = $reason; + } + /** * {@inheritdoc} @@ -14,4 +32,19 @@ public function isForbidden() { return TRUE; } + /** + * {@inheritdoc} + */ + public function getReason() { + return $this->reason; + } + + /** + * {@inheritdoc} + */ + public function setReason($reason) { + $this->reason = $reason; + return $this; + } + } diff --git a/core/lib/Drupal/Core/Access/AccessResultReasonInterface.php b/core/lib/Drupal/Core/Access/AccessResultReasonInterface.php new file mode 100644 index 0000000000000000000000000000000000000000..468d9988964cc936d7cb8c150c29846867d136c9 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessResultReasonInterface.php @@ -0,0 +1,36 @@ +csrfToken->validate($csrf_token, self::TOKEN_KEY) && !$this->csrfToken->validate($csrf_token, 'rest')) { - return AccessResult::forbidden()->setCacheMaxAge(0); + return AccessResult::forbidden()->setReason('X-CSRF-Token request header is missing')->setCacheMaxAge(0); } } // Let other access checkers decide if the request is legit. diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index 80e16ebb6fcbe868e804bb44d03e8252a6b40d0c..9346b0ae1f7996d81d93ec1717752e500b5fb251 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -3,6 +3,7 @@ namespace Drupal\Core\Routing; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Cmf\Component\Routing\ChainRouter; use Symfony\Component\HttpFoundation\Request; @@ -105,7 +106,7 @@ protected function checkAccess(Request $request) { $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); } if (!$access_result->isAllowed()) { - throw new AccessDeniedHttpException(); + throw new AccessDeniedHttpException($access_result instanceof AccessResultReasonInterface ? $access_result->getReason() : NULL); } } diff --git a/core/modules/rest/src/Tests/CreateTest.php b/core/modules/rest/src/Tests/CreateTest.php index 28c8af69178c96bffd006143ba32a27874654f32..d71b9e4a0f793e4343c8f13b7436f2f793b7143f 100644 --- a/core/modules/rest/src/Tests/CreateTest.php +++ b/core/modules/rest/src/Tests/CreateTest.php @@ -356,6 +356,10 @@ public function createAccountPerEntity($entity_type) { public function assertCreateEntityOverRestApi($entity_type, $serialized = NULL) { // Note: this will fail with PHP 5.6 when always_populate_raw_post_data is // set to something other than -1. See https://www.drupal.org/node/2456025. + // Try first without the CSRF token, which should fail. + $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType, TRUE); + $this->assertResponse(403, 'X-CSRF-Token request header is missing'); + // Then try with the CSRF token. $response = $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType); $this->assertResponse(201); diff --git a/core/modules/rest/src/Tests/DeleteTest.php b/core/modules/rest/src/Tests/DeleteTest.php index 4cc80162ec2857d7ac121a11c4a139b1fdda21c9..7de3fb48e4a75eda1495bb857a5d0c276aac31bd 100644 --- a/core/modules/rest/src/Tests/DeleteTest.php +++ b/core/modules/rest/src/Tests/DeleteTest.php @@ -38,6 +38,9 @@ public function testDelete() { // Create an entity programmatically. $entity = $this->entityCreate($entity_type); $entity->save(); + // Try first to delete over REST API without the CSRF token. + $this->httpRequest($entity->urlInfo(), 'DELETE', NULL, NULL, TRUE); + $this->assertResponse(403, 'X-CSRF-Token request header is missing'); // Delete it over the REST API. $response = $this->httpRequest($entity->urlInfo(), 'DELETE'); // Clear the static cache with entity_load(), otherwise we won't see the diff --git a/core/modules/rest/src/Tests/RESTTestBase.php b/core/modules/rest/src/Tests/RESTTestBase.php index 4e660c35616e49a5c8eb595192081ce8b42d1df4..a3f6a5fde168d77792327e81d29878c3b49f5b4a 100644 --- a/core/modules/rest/src/Tests/RESTTestBase.php +++ b/core/modules/rest/src/Tests/RESTTestBase.php @@ -85,11 +85,13 @@ protected function setUp() { * The body for POST and PUT. * @param string $mime_type * The MIME type of the transmitted content. + * @param bool $forget_xcsrf_token + * If TRUE, the CSRF token won't be included in request. * * @return string * The content returned from the request. */ - protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { + protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL, $forget_xcsrf_token = FALSE) { if (!isset($mime_type)) { $mime_type = $this->defaultMimeType; } @@ -130,9 +132,11 @@ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { CURLOPT_POSTFIELDS => $body, CURLOPT_URL => $url, CURLOPT_NOBODY => FALSE, - CURLOPT_HTTPHEADER => array( + CURLOPT_HTTPHEADER => !$forget_xcsrf_token ? array( 'Content-Type: ' . $mime_type, 'X-CSRF-Token: ' . $token, + ) : array( + 'Content-Type: ' . $mime_type, ), ); break; @@ -144,9 +148,11 @@ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { CURLOPT_POSTFIELDS => $body, CURLOPT_URL => $url, CURLOPT_NOBODY => FALSE, - CURLOPT_HTTPHEADER => array( + CURLOPT_HTTPHEADER => !$forget_xcsrf_token ? array( 'Content-Type: ' . $mime_type, 'X-CSRF-Token: ' . $token, + ) : array( + 'Content-Type: ' . $mime_type, ), ); break; @@ -158,9 +164,11 @@ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { CURLOPT_POSTFIELDS => $body, CURLOPT_URL => $url, CURLOPT_NOBODY => FALSE, - CURLOPT_HTTPHEADER => array( + CURLOPT_HTTPHEADER => !$forget_xcsrf_token ? array( 'Content-Type: ' . $mime_type, 'X-CSRF-Token: ' . $token, + ) : array( + 'Content-Type: ' . $mime_type, ), ); break; @@ -171,7 +179,7 @@ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) { CURLOPT_CUSTOMREQUEST => 'DELETE', CURLOPT_URL => $url, CURLOPT_NOBODY => FALSE, - CURLOPT_HTTPHEADER => array('X-CSRF-Token: ' . $token), + CURLOPT_HTTPHEADER => !$forget_xcsrf_token ? array('X-CSRF-Token: ' . $token) : array(), ); break; } diff --git a/core/modules/rest/src/Tests/UpdateTest.php b/core/modules/rest/src/Tests/UpdateTest.php index b8f1b54494c2fb96df63bbf1cc11223828fd88d1..dcab1ff76bd8b1a072d917afc7712c2c04894c84 100644 --- a/core/modules/rest/src/Tests/UpdateTest.php +++ b/core/modules/rest/src/Tests/UpdateTest.php @@ -374,6 +374,11 @@ protected function patchEntity(EntityInterface $entity, array $read_only_fields, } $serialized = $serializer->serialize($normalized, $format, $context); + // Try first without CSRF token which should fail. + $this->httpRequest($url, 'PATCH', $serialized, $mime_type, TRUE); + $this->assertResponse(403, 'X-CSRF-Token request header is missing'); + + // Then try with CSRF token. $this->httpRequest($url, 'PATCH', $serialized, $mime_type); $this->assertResponse(200); } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php new file mode 100644 index 0000000000000000000000000000000000000000..3409f379712c78e53f91f6098b4445dc9802836d --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php @@ -0,0 +1,45 @@ +assertEquals(NULL, $a->getReason()); + + $reason = $this->getRandomGenerator()->string(); + $b = new AccessResultForbidden($reason); + $this->assertEquals($reason, $b->getReason()); + } + + /** + * Test setReason() + * + * @covers ::setReason + */ + public function testSetReason() { + $a = new AccessResultForbidden(); + + $reason = $this->getRandomGenerator()->string(); + $return = $a->setReason($reason); + + $this->assertSame($reason, $a->getReason()); + $this->assertSame($a, $return); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php index 287c344d5cef320085b86d435d6ee99d30040a28..ca262c7e18f05071ad65b1fd17230ffb7232811f 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultTest.php @@ -10,6 +10,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Access\AccessResultNeutral; +use Drupal\Core\Access\AccessResultReasonInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; @@ -112,6 +113,23 @@ public function testAccessForbidden() { $verify($b); } + /** + * @covers ::forbidden + */ + public function testAccessForbiddenReason() { + $verify = function (AccessResult $access, $reason) { + $this->assertInstanceOf(AccessResultReasonInterface::class, $access); + $this->assertSame($reason, $access->getReason()); + }; + + $b = AccessResult::forbidden(); + $verify($b, NULL); + + $reason = $this->getRandomGenerator()->string(); + $b = AccessResult::forbidden($reason); + $verify($b, $reason); + } + /** * @covers ::allowedIf * @covers ::isAllowed diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php index df3b580c82c7abb5d9f81df630722c038802a5fa..0b7bcc6730e86add02ca9227b981d199b537f607 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php @@ -8,6 +8,7 @@ use Drupal\Tests\UnitTestCase; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Route; /** @@ -105,6 +106,22 @@ public function testMatchRequestDenied() { $this->assertSame($expected, $parameters); } + /** + * Tests the matchRequest() function for access denied with reason message. + */ + public function testCheckAccessResultWithReason() { + $this->setupRouter(); + $request = new Request(); + $reason = $this->getRandomGenerator()->string(); + $access_result = AccessResult::forbidden($reason); + $this->accessManager->expects($this->once()) + ->method('checkRequest') + ->with($request) + ->willReturn($access_result); + $this->setExpectedException(AccessDeniedHttpException::class, $reason); + $this->router->matchRequest($request); + } + /** * Ensure that methods are passed to the wrapped router. *