EntityReferenceSelection handlers does not consider entity view/view label operations

For historical reference, we had the following discussion in the security issue with mainly @berdir and @larowlan - please add your correction to my summary if necessary:

Originally, I have started the security thread with these three questions after opened 🐛 Fix filtering on Node's Authored by field (and on other entity reference fields) Active :

1. Should entity references respect either of these entity operation accesses?
2. What is the intended purpose of the "view label" entity operation if not for controlling label visibility?
3. How should this interact with node access, given that hook_node_access() can override permissions granted by hook_node_access_grants() (Proof: https://git.drupalcode.org/project/view_usernames_node_author/-/blob/1.0.0/tests/src/Kernel/NodeAccessTest.php?ref_type=tags#L94-106)?

TL;DR

• Entity Reference Selection handlers should perform query level access checks and avoid listing entities that a given user does not have access to. This can be achieved through (entity) query alters or via custom selection plugins, see \Drupal\user\Plugin\EntityReferenceSelection\UserSelection::buildEntityQuery() and \Drupal\node\Plugin\EntityReferenceSelection\NodeSelection::buildEntityQuery()
• $entity->access() performs access check on a single entity. When performed on a list of entities, it breaks pagination and could lead to performance issues.
• The "view label" operation was introduced as a subset of the "view" entity operation. The primary use case in core is being able to view user display names without having access to their profile. There is some overlap with field access, but a label does not have to be a field.

In addition: node/entity access is inconsistent. List and entity level access checking in Drupal core is complex, with several open issues addressing these concerns. Developers should be aware of how to use them properly.

Potential action items

During a security team triage meeting (@greggles, @mcdruid, @drumm, @larowlan), the following suggestions were made:

1. Add additional documentation to hook_entity_access() and hook_entity_access_alter() hook documentation, noting the gotchas around selection plugins.
2. Expand the documentation to explain that hook_entity_reference_selection_alter() can be used to modify the class for a given entity-type and add custom logic as needed.

These documentation improvements would help developers, especially those new to Drupal, understand the potential pitfalls related to access controls.

Additional documentation suggestions

Extend the documentation on \Drupal\Core\Entity\EntityAccessControlHandler::$viewLabelOperation to properly explain that "view label" is a subset of the "view" operation. Clarify that even if it grants access to a field/property on an entity or sometimes a dynamically generated value (e.g., $user->getDisplayName()), it is not equivalent to field access.

Explain that when a user has "view" operation access to an entity, they also have "view label" access. However, when they only have "view label" access, they literally only have access to the entity label and nothing more. This also means that they may not have access to the entity's canonical URL (see The "Label" entity reference field formatter now restricts links for inaccessible destinations → ).

It was also concluded some type of access checking cannot be implemented as a query level access check, only as a single entity level access check which means the $entity->access() calls remains a necessity even if they break pagination or may lead to performance issues.

> It was also concluded some type of access checking cannot be implemented as a query level access check, only as a single entity level access check which means the $entity->access() calls remains a necessity even if they break pagination or may lead to performance issues.

I don't agree with this. Core doesn't do it in hardcoded lists, views doesn't do it. Doing query access is expected to be sufficient. If that _were_ a necessity, then the fact that core doesn't do it would be a security issue.