diff --git a/jsonapi.services.yml b/jsonapi.services.yml index b5c3f22..f059898 100644 --- a/jsonapi.services.yml +++ b/jsonapi.services.yml @@ -113,6 +113,10 @@ services: jsonapi.field_resolver: class: Drupal\jsonapi\Context\FieldResolver arguments: ['@entity_type.manager', '@entity_field.manager', '@entity_type.bundle.info', '@jsonapi.resource_type.repository'] + access_check.jsonapi.relationship_field_access: + class: Drupal\jsonapi\Access\RelationshipFieldAccess + tags: + - { name: access_check, applies_to: _jsonapi_relationship_field_access, needs_incoming_request: TRUE } paramconverter.jsonapi.entity_uuid: class: Drupal\jsonapi\ParamConverter\EntityUuidConverter tags: diff --git a/src/Access/RelationshipFieldAccess.php b/src/Access/RelationshipFieldAccess.php new file mode 100644 index 0000000..0be984c --- /dev/null +++ b/src/Access/RelationshipFieldAccess.php @@ -0,0 +1,64 @@ +getRequirement(static::ROUTE_REQUIREMENT_KEY); + $field_operation = $request->isMethodCacheable() ? 'view' : 'edit'; + $entity_operation = $request->isMethodCacheable() ? 'view' : 'update'; + if ($resource_type = $request->get(Routes::RESOURCE_TYPE_KEY)) { + $entity = $request->get($resource_type->getEntityTypeId()); + if ($entity instanceof FieldableEntityInterface && $entity->hasField($relationship_field_name)) { + $entity_access = $entity->access($entity_operation, $account, TRUE); + $field_access = $entity->get($relationship_field_name)->access($field_operation, $account, TRUE); + $access_result = $entity_access->andIf($field_access); + if ($access_result instanceof AccessResultReasonInterface) { + $reason = "The current user is not allowed to {$field_operation} this relationship."; + if ($access_reason = $access_result->getReason()) { + $reason .= $access_reason; + }; + $access_result->setReason($reason); + } + return $access_result; + } + } + return AccessResult::neutral(); + } +} \ No newline at end of file diff --git a/src/Controller/EntityResource.php b/src/Controller/EntityResource.php index b76b201..f171f8e 100644 --- a/src/Controller/EntityResource.php +++ b/src/Controller/EntityResource.php @@ -383,7 +383,7 @@ class EntityResource { /** * Gets the related resource. * - * @param \Drupal\Core\Entity\EntityInterface $entity + * @param \Drupal\Core\Entity\FieldableEntityInterface $entity * The requested entity. * @param string $related_field * The related field name. @@ -393,12 +393,9 @@ class EntityResource { * @return \Drupal\jsonapi\ResourceResponse * The response. */ - public function getRelated(EntityInterface $entity, $related_field, Request $request) { - $related_field = $this->resourceType->getInternalName($related_field); - $this->relationshipAccess($entity, 'view', $related_field); + public function getRelated(FieldableEntityInterface $entity, $related_field, Request $request) { /* @var \Drupal\Core\Field\EntityReferenceFieldItemListInterface $field_list */ - $field_list = $entity->get($related_field); - $this->validateReferencedResource($field_list, $related_field); + $field_list = $entity->get($this->resourceType->getInternalName($related_field)); // Add the cacheable metadata from the host entity. $cacheable_metadata = CacheableMetadata::createFromObject($entity); $is_multiple = $field_list @@ -446,7 +443,7 @@ class EntityResource { /** * Gets the relationship of an entity. * - * @param \Drupal\Core\Entity\EntityInterface $entity + * @param \Drupal\Core\Entity\FieldableEntityInterface $entity * The requested entity. * @param string $related_field * The related field name. @@ -458,41 +455,13 @@ class EntityResource { * @return \Drupal\jsonapi\ResourceResponse * The response. */ - public function getRelationship(EntityInterface $entity, $related_field, Request $request, $response_code = 200) { - $related_field = $this->resourceType->getInternalName($related_field); - $this->relationshipAccess($entity, 'view', $related_field); + public function getRelationship(FieldableEntityInterface $entity, $related_field, Request $request, $response_code = 200) { /* @var \Drupal\Core\Field\FieldItemListInterface $field_list */ - $field_list = $entity->get($related_field); - $this->validateReferencedResource($field_list, $related_field); + $field_list = $entity->get($this->resourceType->getInternalName($related_field)); $response = $this->buildWrappedResponse($field_list, $response_code); return $response; } - /** - * Validates that the referenced field points to an enabled resource. - * - * @param \Drupal\Core\Field\EntityReferenceFieldItemListInterface|null $field_list - * The field list with the reference. - * @param string $related_field - * The internal name of the related field. - * - * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException - * If the field is not a reference or the target resource is disabled. - * @throws \Symfony\Component\HttpKernel\Exception\HttpException - * If the $field_list is of the incorrect type. - */ - protected function validateReferencedResource($field_list, $related_field) { - if ( - !is_null($field_list) && - !$field_list instanceof EntityReferenceFieldItemListInterface - ) { - throw new HttpException(500, 'Invalid internal structure for relationship field list.'); - } - if (!$field_list || !$this->isRelationshipField($field_list)) { - throw new NotFoundHttpException(sprintf('The relationship %s is not present in this resource.', $related_field)); - } - } - /** * Adds a relationship to a to-many relationship. * @@ -518,7 +487,6 @@ class EntityResource { public function createRelationship(EntityInterface $entity, $related_field, $parsed_field_list, Request $request) { $related_field = $this->resourceType->getInternalName($related_field); /* @var \Drupal\Core\Field\EntityReferenceFieldItemListInterface $parsed_field_list */ - $this->relationshipAccess($entity, 'update', $related_field); if ($parsed_field_list instanceof Response) { // This usually means that there was an error, so there is no point on // processing further. @@ -619,7 +587,6 @@ class EntityResource { return $parsed_field_list; } /* @var \Drupal\Core\Field\EntityReferenceFieldItemListInterface $parsed_field_list */ - $this->relationshipAccess($entity, 'update', $related_field); // According to the specification, PATCH works a little bit different if the // relationship is to-one or to-many. /* @var \Drupal\Core\Field\EntityReferenceFieldItemListInterface $field_list */ @@ -708,8 +675,6 @@ class EntityResource { throw new BadRequestHttpException(sprintf('You need to provide a body for DELETE operations on a relationship (%s).', $related_field)); } /* @var \Drupal\Core\Field\EntityReferenceFieldItemListInterface $parsed_field_list */ - $this->relationshipAccess($entity, 'update', $related_field); - $field_name = $parsed_field_list->getName(); $field_access = $parsed_field_list->access('edit', NULL, TRUE); if (!$field_access->isAllowed()) { @@ -879,9 +844,6 @@ class EntityResource { // @todo Is this really the right path? throw new EntityAccessDeniedHttpException($entity, $combined_access, $related_field, "The current user is not allowed to $operation this relationship."); } - if (!($field_list = $entity->get($related_field)) || !$this->isRelationshipField($field_list)) { - throw new NotFoundHttpException(sprintf('The relationship %s is not present in this resource.', $related_field)); - } } /** @@ -965,25 +927,6 @@ class EntityResource { throw new EntityAccessDeniedHttpException($original_field->getEntity(), $field_edit_access, '/data/attributes/' . $field_name, sprintf('The current user is not allowed to PATCH the selected field (%s).', $field_name)); } - /** - * Checks if is a relationship field. - * - * @param \Drupal\Core\Field\FieldItemListInterface $entity_field - * Entity field. - * - * @return bool - * Returns TRUE if entity field is a relationship field with non-internal - * target resource types, FALSE otherwise. - */ - protected function isRelationshipField(FieldItemListInterface $entity_field) { - $resource_types = $this->resourceType->getRelatableResourceTypesByField( - $this->resourceType->getInternalName($entity_field->getName()) - ); - return !empty($resource_types) && array_reduce($resource_types, function ($has_external, $resource_type) { - return $has_external ? TRUE : !$resource_type->isInternal(); - }, FALSE); - } - /** * Build a collection of the entities to respond with and access objects. * diff --git a/src/Normalizer/Value/RelationshipNormalizerValue.php b/src/Normalizer/Value/RelationshipNormalizerValue.php index 57798ec..f193afc 100644 --- a/src/Normalizer/Value/RelationshipNormalizerValue.php +++ b/src/Normalizer/Value/RelationshipNormalizerValue.php @@ -149,14 +149,15 @@ class RelationshipNormalizerValue extends FieldNormalizerValue { * An array of links to be rasterized. */ protected function getLinks($field_name) { + $relationship_field_name = $this->resourceType->getPublicName($field_name); $route_parameters = [ - 'related' => $this->resourceType->getPublicName($field_name), + 'related' => $relationship_field_name, ]; $links['self'] = $this->linkManager->getEntityLink( $this->hostEntityId, $this->resourceType, $route_parameters, - 'relationship' + "$relationship_field_name.relationship" ); $resource_types = $this->resourceType->getRelatableResourceTypesByField($field_name); if (static::hasNonInternalResourceType($resource_types)) { @@ -164,7 +165,7 @@ class RelationshipNormalizerValue extends FieldNormalizerValue { $this->hostEntityId, $this->resourceType, $route_parameters, - 'related' + "$relationship_field_name.related" ); } return $links; diff --git a/src/Routing/Routes.php b/src/Routing/Routes.php index dbd5c47..764a35b 100644 --- a/src/Routing/Routes.php +++ b/src/Routing/Routes.php @@ -3,6 +3,7 @@ namespace Drupal\jsonapi\Routing; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\jsonapi\Access\RelationshipFieldAccess; use Drupal\jsonapi\Controller\EntryPoint; use Drupal\jsonapi\Normalizer\Relationship; use Drupal\jsonapi\ParamConverter\ResourceTypeConverter; @@ -230,25 +231,35 @@ class Routes implements ContainerInjectionInterface { $routes->add(static::getRouteName($resource_type, 'individual.delete'), $individual_remove_route); } - // Get an individual resource's related resources. - $related_route = new Route("/{$path}/{{$entity_type_id}}/{related}"); - $related_route->setMethods(['GET']); - // @todo: remove this when each related route is defined per relationship field and access is no longer checked by the controller in https://www.drupal.org/project/jsonapi/issues/2953346. - $related_route->setRequirement('_access', 'TRUE'); - $routes->add(static::getRouteName($resource_type, 'related'), $related_route); - - // Read, update, add, or remove an individual resources relationships to - // other resources. - $relationship_route = new Route("/{$path}/{{$entity_type_id}}/relationships/{related}"); - $relationship_route->setMethods($resource_type->isMutable() - ? ['GET', 'POST', 'PATCH', 'DELETE'] - : ['GET'] - ); - // @todo: remove the _on_relationship default in https://www.drupal.org/project/jsonapi/issues/2953346. - $relationship_route->addDefaults(['_on_relationship' => TRUE]); - $relationship_route->addDefaults(['serialization_class' => Relationship::class]); - $relationship_route->setRequirement('_csrf_request_header_token', 'TRUE'); - $routes->add(static::getRouteName($resource_type, 'relationship'), $relationship_route); + foreach ($resource_type->getRelatableResourceTypes() as $relationship_field_name => $target_resource_types) { + // Do not create routes for relationships that only target internal + // resource types. + if (!static::hasNonInternalTargetResourceTypes($target_resource_types)) { + continue; + } + + // Get an individual resource's related resources. + $related_route = new Route("/{$path}/{{$entity_type_id}}/{$relationship_field_name}"); + $related_route->setMethods(['GET']); + $related_route->addDefaults(['related' => $relationship_field_name]); + $related_route->setRequirement(RelationshipFieldAccess::ROUTE_REQUIREMENT_KEY, $relationship_field_name); + $routes->add(static::getRouteName($resource_type, "$relationship_field_name.related"), $related_route); + + // Read, update, add, or remove an individual resources relationships to + // other resources. + $relationship_route = new Route("/{$path}/{{$entity_type_id}}/relationships/{$relationship_field_name}"); + $relationship_route->setMethods($resource_type->isMutable() + ? ['GET', 'POST', 'PATCH', 'DELETE'] + : ['GET'] + ); + // @todo: remove the _on_relationship default in https://www.drupal.org/project/jsonapi/issues/2953346. + $relationship_route->addDefaults(['_on_relationship' => TRUE]); + $relationship_route->addDefaults(['serialization_class' => Relationship::class]); + $relationship_route->addDefaults(['related' => $relationship_field_name]); + $relationship_route->setRequirement(RelationshipFieldAccess::ROUTE_REQUIREMENT_KEY, $relationship_field_name); + $relationship_route->setRequirement('_csrf_request_header_token', 'TRUE'); + $routes->add(static::getRouteName($resource_type, "$relationship_field_name.relationship"), $relationship_route); + } // Add entity parameter conversion to every route. $routes->addOptions(['parameters' => [$entity_type_id => ['type' => 'entity:' . $entity_type_id]]]); @@ -308,6 +319,22 @@ class Routes implements ContainerInjectionInterface { return sprintf('jsonapi.%s.%s', $resource_type->getTypeName(), $route_type); } + /** + * Determines if an array of resource types has any non-internal ones. + * + * @param \Drupal\jsonapi\ResourceType\ResourceType[] $resource_types + * The resource types to check. + * + * @return bool + * TRUE if there is at least one non-internal resource type in the given + * array; FALSE otherwise. + */ + protected static function hasNonInternalTargetResourceTypes(array $resource_types) { + return array_reduce($resource_types, function ($carry, ResourceType $target) { + return $carry || !$target->isInternal(); + }, FALSE); + } + /** * Gets the resource type from a route or request's parameters. * diff --git a/tests/src/Functional/ResourceResponseTestTrait.php b/tests/src/Functional/ResourceResponseTestTrait.php index f9e74b7..d5e1cd3 100644 --- a/tests/src/Functional/ResourceResponseTestTrait.php +++ b/tests/src/Functional/ResourceResponseTestTrait.php @@ -465,8 +465,9 @@ trait ResourceResponseTestTrait { * for testing related/relationship routes and includes. * @param string|null $detail * (optional) Details for the JSON API error object. - * @param string|null $pointer - * (optional) Document pointer for the JSON API error object. + * @param string|bool|null $pointer + * (optional) Document pointer for the JSON API error object. FALSE to omit + * the pointer. * @param string|null $id * (optional) ID for the JSON API error object. * @@ -493,7 +494,7 @@ trait ResourceResponseTestTrait { if (!is_null($id)) { $error['id'] = $id; } - if ($relationship_field_name || $pointer) { + if ($pointer || $pointer !== FALSE && $relationship_field_name) { $error['source']['pointer'] = ($pointer) ? $pointer : $relationship_field_name; } return (new ResourceResponse(['errors' => [$error]], 403))->addCacheableDependency($access); diff --git a/tests/src/Functional/ResourceTestBase.php b/tests/src/Functional/ResourceTestBase.php index 79d7730..2f62ad6 100644 --- a/tests/src/Functional/ResourceTestBase.php +++ b/tests/src/Functional/ResourceTestBase.php @@ -1415,9 +1415,8 @@ abstract class ResourceTestBase extends BrowserTestBase { $relationship_field_name = 'field_jsonapi_test_entity_ref'; /* @var \Drupal\Core\Access\AccessResultReasonInterface $update_access */ $update_access = static::entityAccess($resource, 'update', $this->account) - ->andIf(static::entityFieldAccess($resource, $relationship_field_name, 'update', $this->account)); - $url = Url::fromRoute(sprintf("jsonapi.{$resource_identifier['type']}.relationship"), [ - 'related' => $relationship_field_name, + ->andIf(static::entityFieldAccess($resource, $relationship_field_name, 'edit', $this->account)); + $url = Url::fromRoute(sprintf("jsonapi.{$resource_identifier['type']}.{$relationship_field_name}.relationship"), [ $resource->getEntityTypeId() => $resource->uuid(), ]); @@ -1571,13 +1570,13 @@ abstract class ResourceTestBase extends BrowserTestBase { else { $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('POST', $url, $request_options); - $message = 'The current user is not allowed to update this relationship.'; + $message = 'The current user is not allowed to edit this relationship.'; $message .= ($reason = $update_access->getReason()) ? ' ' . $reason : ''; - $this->assertResourceErrorResponse(403, $message, $response, $relationship_field_name); + $this->assertResourceErrorResponse(403, $message, $response); $response = $this->request('PATCH', $url, $request_options); - $this->assertResourceErrorResponse(403, $message, $response, $relationship_field_name); + $this->assertResourceErrorResponse(403, $message, $response); $response = $this->request('DELETE', $url, $request_options); - $this->assertResourceErrorResponse(403, $message, $response, $relationship_field_name); + $this->assertResourceErrorResponse(403, $message, $response); } // Remove the test entities that were created. @@ -1600,7 +1599,7 @@ abstract class ResourceTestBase extends BrowserTestBase { $entity = $entity ?: $this->entity; $access = static::entityFieldAccess($entity, $relationship_field_name, 'view', $this->account); if (!$access->isAllowed()) { - return static::getAccessDeniedResponse($this->entity, $access, $relationship_field_name, 'The current user is not allowed to view this relationship.'); + return static::getAccessDeniedResponse($this->entity, $access, $relationship_field_name, 'The current user is not allowed to view this relationship.', FALSE); } $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name); $status_code = isset($expected_document['errors'][0]['status']) ? $expected_document['errors'][0]['status'] : 200; @@ -1719,9 +1718,6 @@ abstract class ResourceTestBase extends BrowserTestBase { ], 'code' => 0, 'id' => '/' . $base_resource_identifier['type'] . '/' . $base_resource_identifier['id'], - 'source' => [ - 'pointer' => $relationship_field_name, - ], ], ], ], 403))->addCacheableDependency($access); @@ -2762,7 +2758,7 @@ abstract class ResourceTestBase extends BrowserTestBase { * The AccessResult. */ protected static function entityFieldAccess(EntityInterface $entity, $field_name, $operation, AccountInterface $account) { - $entity_access = static::entityAccess($entity, $operation, $account); + $entity_access = static::entityAccess($entity, $operation === 'edit' ? 'update' : 'view', $account); $field_access = $entity->{$field_name}->access($operation, $account, TRUE); return $entity_access->andIf($field_access); } diff --git a/tests/src/Unit/Routing/RoutesTest.php b/tests/src/Unit/Routing/RoutesTest.php index ba678dd..726bd08 100644 --- a/tests/src/Unit/Routing/RoutesTest.php +++ b/tests/src/Unit/Routing/RoutesTest.php @@ -66,9 +66,15 @@ class RoutesTest extends UnitTestCase { // Get the route collection and start making assertions. $routes = $this->routes['ok']->routes(); - // Make sure that there are 7 routes for the non-internal resource and 1 for - // the entry point. - $this->assertEquals(8, $routes->count()); + // - 2 collection routes; GET & POST for the non-internal resource type. + // - 3 individual routes; GET, PATCH & DELETE for the non-internal resource + // type. + // - 2 related routes; GET for the non-internal resource type relationships + // fields: external & both. + // - 2 relationship routes; all HTTP methods handled by the one route for + // the non-internal resource type. + // - 1 for the JSON API entry point. + $this->assertEquals(10, $routes->count()); $iterator = $routes->getIterator(); // Check the collection route. @@ -142,8 +148,8 @@ class RoutesTest extends UnitTestCase { // Check the related route. /** @var \Symfony\Component\Routing\Route $route */ - $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.related'); - $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/{related}', $route->getPath()); + $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.external.related'); + $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/external', $route->getPath()); $this->assertSame('entity_type_1--bundle_1_1', $route->getDefault(Routes::RESOURCE_TYPE_KEY)); $this->assertEquals(['GET'], $route->getMethods()); $this->assertSame(Routes::FRONT_CONTROLLER, $route->getDefault(RouteObjectInterface::CONTROLLER_NAME)); @@ -163,8 +169,8 @@ class RoutesTest extends UnitTestCase { // Check the relationships route. /** @var \Symfony\Component\Routing\Route $route */ - $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.relationship'); - $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/relationships/{related}', $route->getPath()); + $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.both.relationship'); + $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/relationships/both', $route->getPath()); $this->assertSame('entity_type_1--bundle_1_1', $route->getDefault(Routes::RESOURCE_TYPE_KEY)); $this->assertEquals(['GET', 'POST', 'PATCH', 'DELETE'], $route->getMethods()); $this->assertSame(Routes::FRONT_CONTROLLER, $route->getDefault(RouteObjectInterface::CONTROLLER_NAME)); @@ -192,8 +198,10 @@ class RoutesTest extends UnitTestCase { return [ ['jsonapi.entity_type_1--bundle_1_1.individual'], ['jsonapi.entity_type_1--bundle_1_1.collection'], - ['jsonapi.entity_type_1--bundle_1_1.related'], - ['jsonapi.entity_type_1--bundle_1_1.relationship'], + ['jsonapi.entity_type_1--bundle_1_1.external.related'], + ['jsonapi.entity_type_1--bundle_1_1.external.relationship'], + ['jsonapi.entity_type_1--bundle_1_1.both.related'], + ['jsonapi.entity_type_1--bundle_1_1.both.relationship'], ['jsonapi.resource_list'], ]; } @@ -215,8 +223,8 @@ class RoutesTest extends UnitTestCase { ['jsonapi.entity_type_2--bundle_2_1.individual'], ['jsonapi.entity_type_2--bundle_2_1.collection'], ['jsonapi.entity_type_2--bundle_2_1.collection.post'], - ['jsonapi.entity_type_2--bundle_2_1.related'], - ['jsonapi.entity_type_2--bundle_2_1.relationship'], + ['jsonapi.entity_type_2--bundle_2_1.internal.related'], + ['jsonapi.entity_type_2--bundle_2_1.internal.relationship'], ]; }