diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index 8148d4c..f67b9f5 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 4dc0120..2dc9137 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 0000000..30be5f8 --- /dev/null +++ b/core/lib/Drupal/Core/Access/AccessResultReasonInterface.php @@ -0,0 +1,36 @@ + + * new AccessResultForbidden('You are not authorized to hack core'); + * + * + * @see \Drupal\Core\Access\AccessResultInterface + */ +interface AccessResultReasonInterface extends AccessResultInterface { + + /** + * Gets the reason for this access result. + * + * @return string|NULL + * The reason of this access result or NULL if no reason is provided. + */ + public function getReason(); + + /** + * Sets the reason for this access result. + * + * @param $reason string|NULL + * The reason of this access result or NULL if no reason is provided. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result instance. + */ + public function setReason($reason); + +} diff --git a/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php index 470ea9f..53f326d 100644 --- a/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php @@ -102,7 +102,7 @@ public function access(Request $request, AccountInterface $account) { // Kept here for sessions active during update. if (!$this->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 80e16eb..9346b0a 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/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php new file mode 100644 index 0000000..7b5c982 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultForbiddenTest.php @@ -0,0 +1,44 @@ +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 287c344..ca262c7 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; @@ -113,6 +114,23 @@ public function testAccessForbidden() { } /** + * @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 * @covers ::isForbidden diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php index df3b580..dccda5c 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; /** @@ -106,6 +107,22 @@ public function testMatchRequestDenied() { } /** + * Tests the matchRequest() function for access denied with specific 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. * * @covers ::__call