[BUGFIX] Check for individual entity access in listings

Again, thanks for reporting this. You are killing it @dagmar!

Added a test that proves the error.

Making the testbot execute the tests and seeing them fail.

Since the access handlers are not executed during the entity queries, we need to manually check for the errors in the collection.

Then we need to treat the errors as if they were includes and bubble them up to the top level. That way we can effectively have something like:

{
  "data": [{…}],
  "errors": [{…}]
}

It turns out that for it to be valid JSON API we need something like:

We'll need an extension to document the opinionated stance towards errored entities in collections.

This one was tricky.

Added a new extension for this: https://gist.github.com/e0ipso/732712c3e573a6af1d83b25b9f0269c8

1. +++ b/src/Normalizer/Value/DocumentRootNormalizerValue.php
   @@ -107,7 +108,14 @@ class DocumentRootNormalizerValue implements DocumentRootNormalizerValueInterfac
   +      if ($normalizer_value instanceof HttpExceptionNormalizerValue) {
   +        $previous_errors = NestedArray::getValue($rasterized, ['meta', 'errors']) ?: [];
   +        // Add the errors to the pre-existing errors.
   +        $rasterized['meta']['errors'] = array_merge($previous_errors, $normalizer_value->rasterizeValue());
   +      }
   

   I totally love how this patch encodes more things in pure return objects than other potential solutions, like setting some data somewhere.

2. +++ b/src/Resource/EntityResource.php
   @@ -226,9 +228,17 @@ class EntityResource implements EntityResourceInterface {
   +    $access_info = array_column($collection_data, 'access');
   +    array_walk($access_info, function ($access) use ($response) {
   +      $response->addCacheableDependency($access);
   +    });
   

   This is just lovely. For better readability I would have separated access related stuff with new lines.

#777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() would help with this a lot; then you wouldn't need to call access() on every individual entity anymore.

Which makes me wonder whether this should actually avoid calling $entity->access() if $entity instanceof NodeInterface? Listings are always per entity type, so if the entity type is 'node', then JSON API should rely on that instead of this?

I'm not sure but this might be the same issue as #2807185: [BUGFIX] Handle relationships when target resource is not accessible or at least related with it.

Addressed the feedback provided by @dawehner.

@Wim Leers afaik the logic in \Drupal\Node\NodeAccessControlHandler::access is not checked during the EntityQuery. If that is the case then this check is indeed needed and additional to the grants system. I may be wrong though, I'm a bit fuzzy on this area.

There is a question still open for @Wim Leers, but this code will still be needed atm regardless of the answer.