QueryInterface::accessCheck does nothing

NodeHooks1::queryNodeAccessAlter née node_query_node_access_alter exists and is tested by NodeQueryAlterTest. Also, among others ✨ Grant query level access to own unpublished nodes Active has the opposite claim. So I think more details are needed here on how to reproduce this.

Just like node grants, it only does something if you have modules implementing those query alters. It does something out of the box for commerce orders for example.

Yes, it's a problem, there are plenty of open issues about it, it's essentially about core not implementing its own access systems "for performance reasons".

NodeHooks1::queryNodeAccessAlter née node_query_node_access_alter exists and is tested by NodeQueryAlterTest. Also, among others #3452449: Grant query level access to own/any unpublished nodes has the opposite claim. So I think more details are needed here on how to reproduce this.

The implementation you mention performs a no-op if no modules implement hook_node_grants

if (!\Drupal::moduleHandler()->hasImplementations('node_grants')) {
  return;
}

https://git.drupalcode.org/project/drupal/-/blob/401a157fc120a0883964670...

As far as I can tell, there are no modules in core that implements that hook other than in a test for node and a test for path. Therefore, node_grants only works if you have a contrib module that utilizes it, otherwise it doesn't do anything.

Regardless, this issue isn't specific to node, the same problem happens with media where all media is returned/exposed regardless of whether you have the view media permission or not, despite that being a hypothetical query alter.

My assumption in ✨ Grant query level access to own unpublished nodes Active , based on the conversation in ✨ Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() Needs work is that the reporter has some sort of module that is "providing context" (read: removing the permission context) to the query. As I have argued multiple times in ✨ Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() Needs work , modules should only be restricting access, not granting additional access. Or it's possible that a contrib module isn't being granular enough when it is restricting access (i.e. the access is being restricted further than it ought to be and there is no way to undo that).

Yes, it's a problem, there are plenty of open issues about it, it's essentially about core not implementing its own access systems "for performance reasons".

That is wild to me. There are many cases (i.e. view media) where a query could simply no-op because the user doesn't have permission to view the entity in the first place. Likewise we don't have a system for adding cacheability metadata to acccess alterations on queries so node does some weird things.

This problem actually creates some really weird behavior like in DefaultMenuLinkTreeManipulators::checkNodeAccess where the node access query gets modified outside of the context of node

To be clear I agree it's a mess, but I don't think this issue will help with changing that.

For starters, It's factually wrong. What this method/API does is control whether or not the query access alter hooks are invoked or not. Whether or not you have those is an entirely different question. And that's I think is covered with existing issues.

For context, one useful resource are selection plugins, beside some entity type specific settings, the main reason they were moved into core together with the entity_reference module is access control. See \Drupal\node\Plugin\EntityReferenceSelection\NodeSelection and \Drupal\user\Plugin\EntityReferenceSelection\UserSelection for two examples.

Similar problems with views, which has custom hardcoded access plugins that don't work with some newer permissions from content moderation and so on.

The goal would essentially be to move that one layer deeper directly into entity query access. But between that and the node grants API and BC and very little interest by the general community, it's really hard to make meaningful improvements in this space.

I added some clarity to the title/description. You are correct that it does something it just doesn't do what I think most people expect it to do.

I'm going to take a stab and making an interface for entity query access that is at least somewhat consistent with our existing entity access patterns (for better or worse). I think there is perhaps a way to solve this problem for contrib/custom entities in 11.x and start being more strict in core in 12.x.

I agree though, there seems to be some fundamental disagreements on how access should or shouldn't work.

Leaving 🐛 EntityReferenceSelection handlers does not consider entity view/view label operations Active here as a reference that not everything is perfect in the entity selection handlers either, or precisely probably everything works as expected but better documentation is needed about which type access check should be applied where and how...