diff --git a/core/lib/Drupal/Core/Access/AccessResult.php b/core/lib/Drupal/Core/Access/AccessResult.php index f67b9f5..eaa2450 100644 --- a/core/lib/Drupal/Core/Access/AccessResult.php +++ b/core/lib/Drupal/Core/Access/AccessResult.php @@ -31,11 +31,16 @@ /** * Creates an AccessResultInterface object with isNeutral() === TRUE. * + * @param string|null $reason + * (optional) The reason why access is forbidden. Intended for developers, + * hence not translatable. + * * @return \Drupal\Core\Access\AccessResult * isNeutral() will be TRUE. */ - public static function neutral() { - return new AccessResultNeutral(); + public static function neutral($reason = NULL) { + assert('is_string($reason) || is_null($reason)'); + return new AccessResultNeutral($reason); } /** @@ -106,7 +111,12 @@ public static function forbiddenIf($condition) { * isNeutral() will be TRUE. */ public static function allowedIfHasPermission(AccountInterface $account, $permission) { - return static::allowedIf($account->hasPermission($permission))->addCacheContexts(['user.permissions']); + $access_result = static::allowedIf($account->hasPermission($permission))->addCacheContexts(['user.permissions']); + + if ($access_result instanceof AccessResultReasonInterface) { + $access_result->setReason("The '$permission' permission is required."); + } + return $access_result; } /** @@ -147,7 +157,21 @@ public static function allowedIfHasPermissions(AccountInterface $account, array } } - return static::allowedIf($access)->addCacheContexts(empty($permissions) ? [] : ['user.permissions']); + $access_result = static::allowedIf($access)->addCacheContexts(empty($permissions) ? [] : ['user.permissions']); + + if ($access_result instanceof AccessResultReasonInterface) { + if (count($permissions) === 1) { + $access_result->setReason("The '$permission' permission is required."); + } + else { + $quote = function ($s) { + return "'$s'"; + }; + $access_result->setReason(sprintf("The following permissions are required: %s.", implode(" $conjunction ", array_map($quote, $permissions)))); + } + } + + return $access_result; } /** @@ -319,6 +343,14 @@ public function orIf(AccessResultInterface $other) { $result = static::neutral(); if (!$this->isNeutral() || ($this->getCacheMaxAge() === 0 && $other->isNeutral()) || ($this->getCacheMaxAge() !== 0 && $other instanceof CacheableDependencyInterface && $other->getCacheMaxAge() !== 0)) { $merge_other = TRUE; + if ($other instanceof AccessResultReasonInterface) { + $result->setReason($other->getReason()); + } + } + else { + if ($this instanceof AccessResultReasonInterface) { + $result->setReason($this->getReason()); + } } } $result->inheritCacheability($this); @@ -358,6 +390,14 @@ public function andIf(AccessResultInterface $other) { $result = static::neutral(); if (!$this->isNeutral()) { $merge_other = TRUE; + if ($other instanceof AccessResultReasonInterface) { + $result->setReason($other->getReason()); + } + } + else { + if ($this instanceof AccessResultReasonInterface) { + $result->setReason($this->getReason()); + } } } $result->inheritCacheability($this); diff --git a/core/lib/Drupal/Core/Access/AccessResultNeutral.php b/core/lib/Drupal/Core/Access/AccessResultNeutral.php index 7a180f8..2fb75eb 100644 --- a/core/lib/Drupal/Core/Access/AccessResultNeutral.php +++ b/core/lib/Drupal/Core/Access/AccessResultNeutral.php @@ -5,7 +5,24 @@ /** * Value object indicating a neutral access result, with cacheability metadata. */ -class AccessResultNeutral extends AccessResult { +class AccessResultNeutral extends AccessResult implements AccessResultReasonInterface { + + /** + * The reason why access is neutral. For use in messages. + * + * @var string|null + */ + protected $reason; + + /** + * Constructs a new AccessResultNeutral 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 +31,19 @@ public function isNeutral() { 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/EventSubscriber/AuthenticationSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php index 62f5486..be3f55e 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php @@ -96,7 +96,7 @@ public function onKernelRequestFilterProvider(GetResponseEvent $event) { if (isset($this->filter) && $event->getRequestType() === HttpKernelInterface::MASTER_REQUEST) { $request = $event->getRequest(); if ($this->authenticationProvider->applies($request) && !$this->filter->appliesToRoutedRequest($request, TRUE)) { - throw new AccessDeniedHttpException(); + throw new AccessDeniedHttpException('The used authentication method is not allowed on this route.'); } } } diff --git a/core/modules/basic_auth/src/Tests/Authentication/BasicAuthTest.php b/core/modules/basic_auth/src/Tests/Authentication/BasicAuthTest.php index 2aca988..04bb0fd 100644 --- a/core/modules/basic_auth/src/Tests/Authentication/BasicAuthTest.php +++ b/core/modules/basic_auth/src/Tests/Authentication/BasicAuthTest.php @@ -173,6 +173,12 @@ function testUnauthorizedErrorMessage() { $this->basicAuthGet($url, $account->getUsername(), $this->randomMachineName()); $this->assertResponse('403', 'The user is blocked when wrong credentials are passed.'); $this->assertText('Access denied', "A user friendly access denied message is displayed"); + + // Case when correct credentials but hasn't access to the route. + $url = Url::fromRoute('router_test.15'); + $this->basicAuthGet($url, $account->getUsername(), $account->pass_raw); + $this->assertResponse('403', 'The used authentication method is not allowed on this route.'); + $this->assertText('Access denied', "A user friendly access denied message is displayed"); } /** diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index a5cb361..103ac29 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -106,7 +106,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(); + throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'view')); } $response = new ResourceResponse($entity, 200); @@ -144,8 +144,9 @@ public function post(EntityInterface $entity = NULL) { throw new BadRequestHttpException('No entity content received.'); } - if (!$entity->access('create')) { - throw new AccessDeniedHttpException(); + $entity_access = $entity->access('create', NULL, TRUE); + if (!$entity_access->isAllowed()) { + throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'create')); } $definition = $this->getPluginDefinition(); // Verify that the deserialized entity is of the type that we expect to @@ -199,8 +200,9 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity if ($entity->getEntityTypeId() != $definition['entity_type']) { throw new BadRequestHttpException('Invalid entity type'); } - if (!$original_entity->access('update')) { - throw new AccessDeniedHttpException(); + $entity_access = $original_entity->access('update', NULL, TRUE); + if (!$entity_access->isAllowed()) { + throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'update')); } // Overwrite the received properties. @@ -263,8 +265,9 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ public function delete(EntityInterface $entity) { - if (!$entity->access('delete')) { - throw new AccessDeniedHttpException(); + $entity_access = $entity->access('delete', NULL, TRUE); + if (!$entity_access->isAllowed()) { + throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'delete')); } try { $entity->delete(); @@ -279,6 +282,26 @@ public function delete(EntityInterface $entity) { } /** + * Return the proper message checking if the entity has bundles. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity object. + * @param string $operation + * The operation executed before to call the exception. + * + * @return string $operation + * The proper message to display in the AccessDeniedHttpException. + */ + public function accessDeniedExceptionMessage(EntityInterface $entity, $operation) { + $message = "You are not authorized to {$operation} this {$entity->getEntityTypeId()} entity"; + + if ($entity->bundle() !== $entity->getEntityTypeId()) { + $message .= " of bundle {$entity->bundle()}"; + } + return "{$message}."; + } + + /** * {@inheritdoc} */ public function permissions() { 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 new file mode 100644 index 0000000..ca23eb2 --- /dev/null +++ b/core/modules/rest/tests/modules/rest_test/rest_test.services.yml @@ -0,0 +1,5 @@ +services: + rest_test.authentication.test_auth: + class: Drupal\rest_test\Authentication\Provider\TestAuth + tags: + - { name: authentication_provider, provider_id: 'rest_test_auth' } diff --git a/core/modules/rest/tests/modules/rest_test/src/Authentication/Provider/TestAuth.php b/core/modules/rest/tests/modules/rest_test/src/Authentication/Provider/TestAuth.php new file mode 100644 index 0000000..36302ac --- /dev/null +++ b/core/modules/rest/tests/modules/rest_test/src/Authentication/Provider/TestAuth.php @@ -0,0 +1,27 @@ +headers->has('REST-test-auth'); + } + + /** + * {@inheritdoc} + */ + public function authenticate(Request $request) { + return NULL; + } + +} diff --git a/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php b/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php index 18dc296..c187b65 100644 --- a/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php +++ b/core/modules/rest/tests/src/Functional/CookieResourceTestTrait.php @@ -92,7 +92,9 @@ protected function getAuthenticationRequestOptions($method) { * {@inheritdoc} */ protected function assertResponseWhenMissingAuthentication(ResponseInterface $response) { - $this->assertResourceErrorResponse(403, '', $response); + // Requests needing cookie authentication but missing it results in a 403 + // response. The cookie authentication mechanism sets no response message. + $this->assertResourceErrorResponse(403, FALSE, $response); } /** diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index e9d9c48..975408d 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -241,6 +241,44 @@ protected function getNormalizedPatchEntity() { } /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessMessage($method) { + + if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) { + return $this->getExpectedBCUnauthorizedAccessMessage($method); + } + + $operation = ''; + if ($method === 'GET') { + $operation = 'view'; + } + if ($method === 'POST') { + $operation = 'create'; + } + if ($method === 'PATCH') { + $operation = 'update'; + } + if ($method === 'DELETE') { + $operation = 'delete'; + } + $message = sprintf('You are not authorized to %s this %s entity', $operation, $this->entity->getEntityTypeId()); + + if ($this->entity->bundle() !== $this->entity->getEntityTypeId()) { + $message .= ' of bundle ' . $this->entity->bundle(); + } + + return "$message."; + } + + /** + * {@inheritdoc} + */ + protected function getExpectedBcUnauthorizedAccessMessage($method) { + return "The 'restful " . strtolower($method) . " entity:" . $this->entity->getEntityTypeId() . "' permission is required."; + } + + /** * The expected cache tags for the GET/HEAD response of the test entity. * * @see ::testGet @@ -300,7 +338,13 @@ public function testGet() { // response because ?_format query string is present. $response = $this->request('GET', $url, $request_options); if ($has_canonical_url) { - $this->assertResourceErrorResponse(403, '', $response); + $permission = $this->entity->getEntityType()->getAdminPermission(); + if ($permission !== FALSE) { + $this->assertResourceErrorResponse(403, "The '{$permission}' permission is required.", $response); + } + else { + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); + } } else { $this->assertResourceErrorResponse(404, 'No route found for "GET ' . str_replace($this->baseUrl, '', $this->getUrl()->setAbsolute()->toString()) . '"', $response); @@ -334,14 +378,25 @@ public function testGet() { $this->assertResponseWhenMissingAuthentication($response); } + $request_options[RequestOptions::HEADERS]['REST-test-auth'] = '1'; + + // DX: 403 when attempting to use unallowed authentication provider. + $response = $this->request('GET', $url, $request_options); + $this->assertResourceErrorResponse(403, 'The used authentication method is not allowed on this route.', $response); + + unset($request_options[RequestOptions::HEADERS]['REST-test-auth']); $request_options = NestedArray::mergeDeep($request_options, $this->getAuthenticationRequestOptions('GET')); // DX: 403 when unauthorized. $response = $this->request('GET', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); - + $permission = $this->entity->getEntityType()->getAdminPermission(); + if ($permission !== FALSE) { + $this->assertResourceErrorResponse(403, "The '{$permission}' permission is required.", $response); + } + else { + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); + } $this->setUpAuthorization('GET'); @@ -401,8 +456,7 @@ public function testGet() { // DX: 403 when unauthorized. $response = $this->request('GET', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); $this->grantPermissionsToTestedRole(['restful get entity:' . static::$entityTypeId]); @@ -551,9 +605,13 @@ public function testPost() { // DX: 403 when unauthorized. $response = $this->request('POST', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); - + $permission = $this->entity->getEntityType()->getAdminPermission(); + if ($permission !== FALSE) { + $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); + } + else { + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('POST'), $response); + } $this->setUpAuthorization('POST'); @@ -619,8 +677,7 @@ public function testPost() { // DX: 403 when unauthorized. $response = $this->request('POST', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('POST'), $response); $this->grantPermissionsToTestedRole(['restful post entity:' . static::$entityTypeId]); @@ -741,9 +798,13 @@ public function testPatch() { // DX: 403 when unauthorized. $response = $this->request('PATCH', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); - + $permission = $this->entity->getEntityType()->getAdminPermission(); + if ($permission !== FALSE) { + $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); + } + else { + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('PATCH'), $response); + } $this->setUpAuthorization('PATCH'); @@ -824,8 +885,7 @@ public function testPatch() { // DX: 403 when unauthorized. $response = $this->request('PATCH', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('PATCH'), $response); $this->grantPermissionsToTestedRole(['restful patch entity:' . static::$entityTypeId]); @@ -896,9 +956,13 @@ public function testDelete() { // DX: 403 when unauthorized. $response = $this->request('DELETE', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); - + $permission = $this->entity->getEntityType()->getAdminPermission(); + if ($permission !== FALSE) { + $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); + } + else { + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('DELETE'), $response); + } $this->setUpAuthorization('DELETE'); @@ -926,8 +990,7 @@ public function testDelete() { // DX: 403 when unauthorized. $response = $this->request('DELETE', $url, $request_options); - // @todo Update the message in https://www.drupal.org/node/2808233. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('DELETE'), $response); $this->grantPermissionsToTestedRole(['restful delete entity:' . static::$entityTypeId]); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Term/TermResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Term/TermResourceTestBase.php index b6dce4f..652ae7e 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Term/TermResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Term/TermResourceTestBase.php @@ -137,4 +137,27 @@ protected function getNormalizedPostEntity() { ]; } + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessMessage($method) { + if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) { + return parent::getExpectedUnauthorizedAccessMessage($method); + } + + if ($method === 'GET') { + return "The 'access content' permission is required."; + } + if ($method === 'POST') { + return "The 'administer taxonomy' permission is required."; + } + if ($method === 'PATCH') { + return "The following permissions are required: 'edit terms in camelids' OR 'administer taxonomy'."; + } + if ($method === 'DELETE') { + return "The following permissions are required: 'delete terms in camelids' OR 'administer taxonomy'."; + } + return parent::getExpectedUnauthorizedAccessMessage($method); + } + } diff --git a/core/modules/rest/tests/src/Functional/ResourceTestBase.php b/core/modules/rest/tests/src/Functional/ResourceTestBase.php index 218e96f..a7dacac 100644 --- a/core/modules/rest/tests/src/Functional/ResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/ResourceTestBase.php @@ -204,6 +204,29 @@ protected function provisionResource($resource_type, $formats = [], $authenticat abstract protected function assertAuthenticationEdgeCases($method, Url $url, array $request_options); /** + * Return the expected error message. + * + * @param string $method + * The HTTP method (GET, POST, PATCH, DELETE). + * + * @return string + * The Error string. + */ + abstract protected function getExpectedUnauthorizedAccessMessage($method); + + /** + * Return the default expected error message if the + * bc_entity_resource_permissions is true. + * + * @param string $method + * The HTTP method (GET, POST, PATCH, DELETE). + * + * @return string + * The Error string. + */ + abstract protected function getExpectedBcUnauthorizedAccessMessage($method); + + /** * Initializes authentication. * * E.g. for cookie authentication, we first need to get a cookie. @@ -321,7 +344,7 @@ protected function assertResourceResponse($expected_status_code, $expected_body, * The error response to assert. */ protected function assertResourceErrorResponse($expected_status_code, $expected_message, ResponseInterface $response) { - $expected_body = $this->serializer->encode(['message' => $expected_message], static::$format); + $expected_body = ($expected_message !== FALSE) ? $this->serializer->encode(['message' => $expected_message], static::$format) : FALSE; $this->assertResourceResponse($expected_status_code, $expected_body, $response); } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessResultNeutralTest.php b/core/tests/Drupal/Tests/Core/Access/AccessResultNeutralTest.php new file mode 100644 index 0000000..e96ba29 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Access/AccessResultNeutralTest.php @@ -0,0 +1,46 @@ +assertEquals(NULL, $a->getReason()); + + $reason = $this->getRandomGenerator()->string(); + $b = new AccessResultNeutral($reason); + $this->assertEquals($reason, $b->getReason()); + } + + /** + * Test setReason() + * + * @covers ::setReason + */ + public function testSetReason() { + $a = new AccessResultNeutral(); + + $reason = $this->getRandomGenerator()->string(); + $return = $a->setReason($reason); + + $this->assertSame($reason, $a->getReason()); + $this->assertSame($a, $return); + } + +}