Problem/Motivation
Entity queries in theory have this concept of access checks that you are now forced to opt in or out.
Great, except that they in practice in most cases don't do anything. Node has the grant system, but core doesn't implement it, so it only becomes active if you use a module that does like domain, group. Most other entity type, including all in core, don't do anything.
When querying terms or media entities, specifying true or false there does nothing. It's false advertising and I think bad for security, because we went through the deprecation and pretty tedious change of making that required for very limited gains.
There's a whole list of workarounds in core to try and solve this on the upper layers for specific cases:
* jsonapi entity access filters control
* entity reference plugins for autocomplete fields
* views plugins like published or own or admin
* entity.module has its own semi-generic system that's very rarely used except in commerce and older versions of group
* custom hardcoded entity queries that add status conditions
Note, in many cases, these filters make sense separate from access. A public list of entities should probably never include unpublished ones, even for admins. But an autocomplete list should if the user is allowed to see them.
#3514221: QueryInterface::accessCheck does not perform access checking in core is probably basically a duplicate of this issue.
Steps to reproduce
Proposed resolution
Scope of this issue:
Attempt to implement query-level access restrictions using the *existing* means with the query alter tag. It's tedious, but possible. I did so with paragraphs_library_query_paragraphs_library_item_access_alter() in a recent paragraphs (paragraphs_library) security issue. And it's ultimately what node grants does as well. We know how to do it, it's just hard.
We can maybe add some helpers to make it a little bit less tedious, but what's out of scope for this issue is to implement a better API: We've been trying to do that since 2010 in #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter().
And if we actually get a new access system, we can just convert the ugly query alters to the less ugly fancy new API and everything should continue to work.
Limitations/Risks:
* This might break stuff, maybe just not return some entities anymore (possibly correctly, possibly not) or might even completely break some more complex queries.
* We likely need cacheability metadata from queries for it to work with caching: #3516034: Add cacheable metadata to SelectInterface and entity QueryInterface objects.
* A challenge is the view label and similar operations, that imply limited access to an entity. We could support that through metadata.
To handle that, I think we might need an opt-in setting that sites can start to adopt it and eventually we make it mandatory and then remove the setting. A bit like the html client validation for example.
The specific approach would be that we start implementing this for specify entity types. We implement the query alter with the switch (global or per entity type, maybe support both with helper QueryAlterAcccess::isEnabled('media')?), then we use the same setting to in turn disable the higher-level logic in jsonapi/entity reference plugins and see if things still work.
Comments