diff --git a/core/lib/Drupal/Core/Access/AccessResultNeutral.php b/core/lib/Drupal/Core/Access/AccessResultNeutral.php index 091565d..2fb75eb 100644 --- a/core/lib/Drupal/Core/Access/AccessResultNeutral.php +++ b/core/lib/Drupal/Core/Access/AccessResultNeutral.php @@ -15,7 +15,7 @@ class AccessResultNeutral extends AccessResult implements AccessResultReasonInte protected $reason; /** - * Constructs a new AccessResultForbidden instance. + * Constructs a new AccessResultNeutral instance. * * @param null|string $reason * (optional) a message to provide details about this access result diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 6c2b5f3..103ac29 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -148,7 +148,6 @@ public function post(EntityInterface $entity = NULL) { 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 // prevent security issues. @@ -201,7 +200,6 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity if ($entity->getEntityTypeId() != $definition['entity_type']) { throw new BadRequestHttpException('Invalid entity type'); } - $entity_access = $original_entity->access('update', NULL, TRUE); if (!$entity_access->isAllowed()) { throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'update')); @@ -267,12 +265,10 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ public function delete(EntityInterface $entity) { - $entity_access = $entity->access('delete', NULL, TRUE); if (!$entity_access->isAllowed()) { throw new AccessDeniedHttpException($entity_access->getReason() ?: $this->accessDeniedExceptionMessage($entity, 'delete')); } - try { $entity->delete(); $this->logger->notice('Deleted entity %type with ID %id.', array('%type' => $entity->getEntityTypeId(), '%id' => $entity->id())); 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 4bbecd0..ca23eb2 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 @@ -2,4 +2,4 @@ services: rest_test.authentication.test_auth: class: Drupal\rest_test\Authentication\Provider\TestAuth tags: - - { name: authentication_provider, provider_id: 'rest_test_auth' } \ No newline at end of file + - { name: authentication_provider, provider_id: 'rest_test_auth' } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index a44b2ec..975408d 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -10,7 +10,6 @@ use Drupal\Core\Url; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; -use Drupal\Tests\Core\Render\Element\PasswordConfirmTest; use Drupal\Tests\rest\Functional\ResourceTestBase; use GuzzleHttp\RequestOptions; use Psr\Http\Message\ResponseInterface; @@ -242,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 @@ -301,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); @@ -335,15 +378,12 @@ 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')); @@ -352,10 +392,10 @@ public function testGet() { $response = $this->request('GET', $url, $request_options); $permission = $this->entity->getEntityType()->getAdminPermission(); if ($permission !== FALSE) { - $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); + $this->assertResourceErrorResponse(403, "The '{$permission}' permission is required.", $response); } else { - $this->assertResourceErrorResponse(403, "You are not authorized to view this {$this->entity->getEntityTypeId()} entity" . (($this->entity->bundle() !== $this->entity->getEntityTypeId()) ? " of bundle {$this->entity->bundle()}." : "."), $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); } $this->setUpAuthorization('GET'); @@ -416,9 +456,7 @@ public function testGet() { // DX: 403 when unauthorized. $response = $this->request('GET', $url, $request_options); - // In case we have a BC layer, permissions are used, which don't have a good - // error message. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response); $this->grantPermissionsToTestedRole(['restful get entity:' . static::$entityTypeId]); @@ -572,7 +610,7 @@ public function testPost() { $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); } else { - $this->assertResourceErrorResponse(403, "You are not authorized to create this {$this->entity->getEntityTypeId()} entity" . (($this->entity->bundle() !== $this->entity->getEntityTypeId()) ? " of bundle {$this->entity->bundle()}." : "."), $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('POST'), $response); } $this->setUpAuthorization('POST'); @@ -639,9 +677,7 @@ public function testPost() { // DX: 403 when unauthorized. $response = $this->request('POST', $url, $request_options); - // In case we have a BC layer, permissions are used, which don't have a good - // error message. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('POST'), $response); $this->grantPermissionsToTestedRole(['restful post entity:' . static::$entityTypeId]); @@ -767,7 +803,7 @@ public function testPatch() { $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); } else { - $this->assertResourceErrorResponse(403, "You are not authorized to update this {$this->entity->getEntityTypeId()} entity" . (($this->entity->bundle() !== $this->entity->getEntityTypeId()) ? " of bundle {$this->entity->bundle()}." : "."), $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('PATCH'), $response); } $this->setUpAuthorization('PATCH'); @@ -849,9 +885,7 @@ public function testPatch() { // DX: 403 when unauthorized. $response = $this->request('PATCH', $url, $request_options); - // In case we have a BC layer, permissions are used, which don't have a good - // error message. - $this->assertResourceErrorResponse(403, '', $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('PATCH'), $response); $this->grantPermissionsToTestedRole(['restful patch entity:' . static::$entityTypeId]); @@ -927,7 +961,7 @@ public function testDelete() { $this->assertResourceErrorResponse(403, "The {$permission} permission is required.", $response); } else { - $this->assertResourceErrorResponse(403, "You are not authorized to delete this {$this->entity->getEntityTypeId()} entity" . (($this->entity->bundle() !== $this->entity->getEntityTypeId()) ? " of bundle {$this->entity->bundle()}." : "."), $response); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('DELETE'), $response); } $this->setUpAuthorization('DELETE'); @@ -956,9 +990,7 @@ public function testDelete() { // DX: 403 when unauthorized. $response = $this->request('DELETE', $url, $request_options); - // In case we have a BC layer, permissions are used, which don't have a good - // error message. - $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 6a3874b..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. 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); + } + +}