Allow entities to specify a "collection permission"

The only problem I see is that core has no method for generating permissions. Although I guess that is the case for "admin permission". It's used and not generated unless the developer explicitly provides .permissions.yml containing it.

Would choosing "collection" prevent us from landing in the 8.x cycle? Just because the community and core-ish pattern is to use overview.

access content overview:
  title: 'Access the Content overview page'
view own unpublished content:

and

access taxonomy overview:
  title: 'Access the taxonomy vocabulary overview page'
  description: 'Get an overview of all taxonomy vocabularies.'

While I agree collection is better. Maybe we should first get core to support "overview" permission for collection routes, and then follow up to fix naming pattern for D9?

The idea would be to make it an entity type annotation, just like admin_permission. Then the permission name can be anything, including the existing access XY overview permissions.

+1 on the idea.

+1 on #4, then!

Initial patch...

- Probably we can avoid those getAdminPermission and getCollectionPermission calls, but going to leave it as it's for now.
- Might worth adding few more dataset for the new condition.

Thanks for the patch. Fixed #10.2 , #10.3 still to be done.

Wow, I am barely awake and you already made my day :-)

awww... surely this one did mine :)

All scenarios in 10.3 addressed in this patch.

1. +++ b/core/lib/Drupal/Core/Entity/EntityTypeInterface.php
   @@ -385,6 +385,17 @@ public function setHandlerClass($handler_type, $value);
   +   * The default \Drupal\Core\Entity\EntityAccessControlHandler class checks
   +   * this permission for collection route.
   

   I would exchange this sentence simply with @see \Drupal\Core\Entity\Routing\DefaultHtmlRouteProvider::getCollectionRoute().

2. +++ b/core/lib/Drupal/Core/Entity/EntityTypeInterface.php
   @@ -385,6 +385,17 @@ public function setHandlerClass($handler_type, $value);
   +   *   String collection permission name, FALSE otherwise.
   

   This is a bit confusing. What about - "The collection permission name, or FALSE if none."?

Thanks @hchonov. Addressed both doc review comments.

Looks good to me as well, should we already set this here on the entity types that have a collection permission? Neither node nor taxonomy_term do currently use the default html route provider but this should simplify the transition?

Media does and does have an access overview permission. File also has an overview permission, but doesn't provide a non-views fallback lost builder/route at all currently.

+++ b/core/lib/Drupal/Core/Entity/EntityType.php
@@ -614,6 +621,14 @@ public function getAdminPermission() {
+    return $this->collection_permission ?: FALSE;

+++ b/core/lib/Drupal/Core/Entity/EntityTypeInterface.php
@@ -385,6 +385,16 @@ public function setHandlerClass($handler_type, $value);
+   * @return string|bool
+   *   The collection permission name, or FALSE if none.

I think we were trying to move away from this pattern of returning FALSE instead of the proper return type (NULL in this case).

Thanks @amateescu. I was thinking about on the initial patch and thought keeping it aligned with admin permission :)

+++ b/core/lib/Drupal/Core/Entity/EntityType.php
@@ -614,6 +621,14 @@ public function getAdminPermission() {
+    return $this->collection_permission ?: NULL;

Now when we return NULL if the property isn't set, then we can simply return it without any checks.

Sure!

OK, with #2702683: Add plural labels to entity types we have a precedent of these kinds of things being part of the annotation without actually being used (yet) in the route provider

+1

So I think that's an argument for adding the overview permissions for nodes (access content overview) and terms (access taxonomy overview).

Right, so we don't have collection link on node and term, added but not used. behaviour still the same.

I found that VocabularyRouteProvider::getCollectionRoute() actually implements this precisely. So I think we should add a collection permission to vocabularies as well and then remove that override. The behavior is then identical to HEAD.

Done, we have both admin and collection permission on override and removing it would have same permissions from generic parent.

I still feel strongly, though, that we should not add a collection permission to media, because that is actually a behavior change. And while it is a bug fix more than anything else, we will want to add test coverage for that, and I don't think that would be in scope here.

Probably a follow up?

I need to take a deeper look at this, but at first glance this seems like it will break JSON API module or at least require it to add new access checks. I.e. as soon as this is added, then theres a potential for an access bypass via JSON API collection routes.

This addition seems like it's trying to put things that should be route specific on the entity type definition.

Given that it's already only ever used in conjunction with an EntityListBuilder, perhaps it would be more appropriate to have a static method on that class.

Thinking a bit more...

What's conceptually different between "admin_permission" and "collection_permission" is that "admin_permission" can be used as a shorthand for "allow all operations on any entity of this type", but "collection_permission" has no such feature. It's simply an indirect way of adding a different _permission check to a automatically generated route.

However, given that it's on the entity type definition, I imagine it will be confused/interpreted to mean that there's a difference between 'view' and 'view in the context of many'.

IOW, if this is added, should JSON API be required to add a new matching route permission for our entity listings? If so, perhaps this breaks BC.

If not, then perhaps all this confusion could be added by reducing the scope of the addition by renaming it to list_builder_permission.

That's part of my concern in #3. In the Entity API module we have a permission provider and access handler which matches it. Drupal core does not. So it feels slightly awkward.

list_builder_permission would mitigate the semantics issue around the route being "collection" and permissions being named "overview". The use case we're working against is the list builder, not an API resource with a collection of entities.

> This addition seems like it's trying to put things that should be route specific on the entity type definition.

Isn't the whole point of the route provider to generate routes based on the entity type definition? We're just adding one more option/feature to it.

> IOW, if this is added, should JSON API be required to add a new matching route permission for our entity listings? If so, perhaps this breaks BC.

The same argument could be made about any key that we added to entity type definitions, that adding it requires modules to use and consider on it. We could never add anything with that argument. I don't see how this is differnt to adding new plural labels, metadata revision keys or additional handlers.

I don't think this opens up a security issue. If it does *not* respect that setting then it simply doesn't allow access and will only allow with the admin permission for example. That's a bug at best but not a security issue.

We should have a change record for it, so that modules know they can use this now, but then it should be fine IMHO.

If it does *not* respect that setting then it simply doesn't allow access and will only allow with the admin permission for example. That's a bug at best but not a security issue.

Currently there is no such permission and we've allowed access by default to the JSON API collection route. We rely on $entity->access('view') to filter out entities to which a user does not have access. This is why i said there's a potential access bypass if it's assumed that "collection_permission" applies to "viewing a collection of entities" when really it means "access the entity listing page.”

If it's the former meaning, then anything that lists entities will need to take this into account because they won't currently be checking access to listings at all.

A name change to list_builder_permission seems like a simple way to skirt this confusion. It's more accurate for its intended purpose anyway.

@tstoeckler: Wow. That might be the first time ever that someone agreed with me in what is basically a crosspost *and* used more words to explain that than I did. respect ;)

> Currently there is no such permission and we've allowed access by default to the JSON API collection route.

If there is a security issue then that's it. Just because there is no standard way to describe access to the collection doesn't mean that it is available for anyone.

If you do check per-entity view access then it's probably not a big deal either way (with that collection thing existing or not), although I wonder how you deal with paging then.

Sorry but I have to strongly disagree with your assessments. Not going straight back to RTBC because I know how it feels to be on the other side of the status ping-pong game but having to conjure some serious restraint to not do that, to be honest.

I really am sorry to swoop in and "un-RTBC" this. I too know that it can be really frustrating. But RTBC stands for "Reviewed and Tested by the Community". Even if JSON API is in contrib (at the moment), we're part of the community and this will introduce a question for us that hasn't been sufficiently addressed. I think that merits more review. I really didn't mean to be rude by doing it.

Can you elaborate on that? With just core this is 100% backwards-compatible and I'm having trouble coming up with any way this could break contrib, so let's just table this point until there is more information on what actually breaks.

I think you probably were writing this as I posted #31. Do you need more clarification since then? I would be happy to provide it :)

I disagree. The collection permission is a shorthand for "access a list of entities of this type".

So this is actually what I was worried it would be simply be confused to be.

So this doesn't make anything confusing that's not already confusing in the first place.

Unfortunately it does, or I wouldn't be confused.

Collection is the correct name for this, not in some theoretical software architecture sense, but given the APIs we already have. We currently generate a collection route, and that is determined by a collection link template, the list builder, and the collection label. ... The collection link template long preceded even the notion of GraphQL or JSON API or anything like it. It has denoted the path to the HTML list of entities since day 1.

This seems to contradict: "The collection permission is a shorthand for "access a list of entities of this type". Is this about adding a permission to control 'view many' or is it about adding a permission to the collection route (the path to the HTML list of entities)?

First off: what an excellent proposal and an even more impressively thoughtful discussion! Go Drupal community!

@tstoeckler: Wow. That might be the first time ever that someone agreed with me in what is basically a crosspost *and* used more words to explain that than I did. respect ;)

😄😄😄😄 LOVE THIS! Go Berdir & tstoeckler!

It's not clear to me what access a list of entities of this type means exactly. Does it mean you can view:

1. their IDs or UUIDs, i.e. the existence of a number of entities of a certain type?
2. their labels?
3. the entire entities (yet of course still respecting field access), i.e. if you have this permission, does that then override per-entity view access?

And how does it relate to the node grants API? And #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter()?

There is one statement that I have a hard time with:

The collection link template long preceded even the notion of GraphQL or JSON API or anything like it. It has denoted the path to the HTML list of entities since day 1. Maybe that was problematic in terms of naming, I'm not sure.

I understand where you're coming from and sympathize with it. But if we really want to make Drupal API-First, then this line of reasoning is dangerous. We cannot add permissions and assume it only applies to HTML responses/routes/controllers. If we do that, that would then mean HTTP API-providing modules (such as REST, JSON API and GraphQL) all need to invent their own permissions … That is not tenable. This is why all HTTP API-providing modules actually do rely on existing entity and field access: to be able to offer consistent, understandable, predictable behavior across HTML responses, REST responses, JSON API responses and GraphQL responses.

Tagging API-First Initiative because it is very important that we discuss and agree on how this should affect HTTP API behavior. Otherwise we take a step away from being API-First.

That's all I ask: that we carefully consider how it affects or should affect HTTP API behavior. It's not clear to me at the moment. I think it's important that we have clarity on this, if not, then this would be a step back. It seems that @tstoeckler has a clear idea in his head of how this would/should affect HTTP APIs? Could you explain, Tobias? I know you were enthusiastic about \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase, so I know you care about HTTP APIs. How would you see this affect that?

This should be documented in entity.api.php, just like admin_permission is already documented there.

So, to attempt to answer my questions, I reviewed the patch.

1. The two pre-existing permissions that are being "elevated" to become collection_permissions are access content overview (for Node entities) and access taxonomy overview (for Term and Vocabulary entities).
   These permissions are already being used for the Node, Term and Vocabulary listings.
   Let's look at the prime example: the views.view.content View (which is accessible at /admin/content). I have two initial questions that should get us all to a better joint understanding:
   1. This patch currently only affects the page (HTML) display of the view. How should this affect the rest_exportdisplay?
   2. The (Views-powered) listings can be customized by site builders, thus exposing additional fields. What if this is customized to show a different set of fields? (Some of which may contain sensitive data.)
2. \Drupal\taxonomy\VocabularyListBuilder::getDefaultOperations() has
   

       if ($entity->access('access taxonomy overview')) {
         $operations['list'] = [
           'title' => t('List terms'),
           'weight' => 0,
           'url' => $entity->toUrl('overview-form'),
         ];
       }
   

   This check AFAICT become obsolete.

Re #35:

I really am sorry to swoop in and "un-RTBC" this. I too know that it can be really frustrating. But RTBC stands for "Reviewed and Tested by the Community". Even if JSON API is in contrib (at the moment), we're part of the community and this will introduce a question for us that hasn't been sufficiently addressed. I think that merits more review. I really didn't mean to be rude by doing it.

I think that the answers from @Berdir and @tstoeckler are sufficient and I agree with them both, that there are no BC breaks and JSON API should be fine further doing the checks on the entity level.

"Needs change record" per #30:

We should have a change record for it, so that modules know they can use this now

Re #36:
Setting to needs work for

This should be documented in entity.api.php, just like admin_permission is already documented there.

and

\Drupal\taxonomy\VocabularyListBuilder::getDefaultOperations() has

if ($entity->access('access taxonomy overview')) {
      $operations['list'] = [
        'title' => t('List terms'),
        'weight' => 0,
        'url' => $entity->toUrl('overview-form'),
      ];
    }

This check AFAICT become obsolete.

Re #39:
Well that operation checks against the same permission in \Drupal\taxonomy\VocabularyAccessControlHandler::checkAccess():

protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) {
    switch ($operation) {
      case 'access taxonomy overview':
      case 'view':
        return AccessResult::allowedIfHasPermissions($account, ['access taxonomy overview', 'administer taxonomy'], 'OR');

Therefore I think that the check in \Drupal\taxonomy\VocabularyListBuilder::getDefaultOperations() isn't needed anymore as it will be performed as part of the route access checking.

> Re #41: But that is a separate access operation. So a module could implement hook_entity_access() and grant or deny access for the 'access taxonomy overview' operation on a per-entity basis. That is a bit far-fetched, but still removing that check would be a behavior change which is completely unnecessary in general and certainly out of scope here.

That's actually not that far-fetched. That's exactly why I implemented it like that and the reason we decided that it is enough for core to have a single permission that grants access to the taxonomy overviews.

Edit: it is also explicitly documented like that on https://www.drupal.org/node/2902390

Thank you both for the clarification. Then I agree on everything with @Berdir and @tstoeckler.

So - just like the 'view' operation is ambiguous and not clearly defined

How is this ambiguous at all?

- such a 'view many' operation is also very ambiguous

The 'view many' operation (collection permission) is here intended to override whatever the "view" operation says: even if you can't access an individual entity, you still can access the entity listing. How this overriding works exactly is what is ambiguous.
Yes, the ambiguity already exists for access content overview and access taxonomy overview. But that's just those two permissions for specific entity types. This issue is making that a standard for all entity types. That's why extra scrutiny is necessary.

That way the API First Initiative can rely on it in some way if it chooses to do so.

This is what makes no sense: "rely on it some way" and "IF it chooses to do so". It's important that listings/collections behave the same, regardless of whether you access them via HTML, RSS, JSON, XML or whatever else.
To make it more concrete: this issue should define how rest_export displays are affected.

This patch introduces a generic concept with a first usage. We can then […]

Once people start using it, we can't change it anymore. That's why I argue it doesn't make sense to do this only with a single usage.

How so? This patch should not affect Views in any way.

core/modules/node/config/optional/views.view.content.yml contains:

display:
  default:
    display_options:
      access:
        type: perm
        options:
          perm: 'access content overview'

Even though this is slightly confusing (because the naming collides), the check is actually checking an entity operation, not a permission, so no, this is not affected.

You're right! :)

The (Views-powered) listings can be customized by site builders, thus exposing additional fields. What if this is customized to show a different set of fields? (Some of which may contain sensitive data.)

Similarly I don't understand how that is in any way relevant to this patch.

… and I don't understand how you don't understand that it is relevant :)

It's all really quite simple:

1. Entity/field access describe requirements to access certain data.
2. We respect this when individual entities as HTML or RSS or JSON or XML (or JSON API/GraphQL in contrib).
3. Respecting this is crucial, because otherwise you could perhaps see for example an image field in RSS but not in HTML, which would easily lead to unintentional access bypass issues.
4. Entity access control handlers already respect the admin_permissions consistently and automatically. This means that everything in the entity type definition is respected and also applies to HTTP APIs: both any custom access control handler and any admin permission.
5. This issue is making a prominent addition to entity access logic: collection_permission. Just like we're making sure entity access and field access work consistently everywhere, we also need to make sure that entity collection access logic works consistently everywhere. If we'd add this just for the (HTML) collection route, then it'd be the very first access-related thing that we cannot automatically respect in HTTP APIs. This is dangerous, because people rely on access control to protect their data.

That's all.

From the IS:

Entity types such as Node and others provide a separate "overview" permission that is useful for granting people access the entity view, even if they do not have the admin permission. For example you may want to allow some people to edit entities, but not delete them or people may have access to edit and delete some entities but not others. In such cases it's necessary to access the entity overview but not have the admin permission.

I've strengthened the phrases that I think are the explain the original motivation for this issue.

It is very clearly about access to the view builder route because of the operations that are accessible there. Not about the distinction between "allowed to view one entity at a time" and "allowed to view more than one entity at a time".

The proposed resolution has been to provide a permission for "allowed to view more than one entity at a time" and then use it to grant people access the entity view.

However, I think introducing a concept of "allowed to view more than one entity at a time" is far too broad to solve this need. That is the heart of what Wim and I are getting at. This feels like a conceptual Trojan horse.

The relatively small missing feature of a default route permission is being used to introduce a much much broader concept. Indeed, similar reasoning for why this is okay is already in the comments explicitly:

OK, with #2702683: Add plural labels to entity types we have a precedent of these kinds of things being part of the annotation without actually being used (yet) in the route provider.

Which suffers from exactly the same Trojan horse logic: Plural labels are obviously good on their own and it was never stated in that issue that it would be setting a precedent. I doubt the committers thought about it that way. The idea that somehow the floodgates to add things that aren't used yet is now open can't be extrapolated from that issue. These things need to be considered on a case by case basis. WRT plural labels the only "bug" that might be introduced if modules don't use them is purely cosmetic. However, when you're talking about access control, the stakes are much higher.

The idea that this change could introduce a behavioral change is even acknowledged in the following paragraph of the same comment:

we should not add a collection permission to media, because that is actually a behavior change. And while it is a bug fix more than anything else, we will want to add test coverage for that, and I don't think that would be in scope here.

However, when I came into the issue to say that this permission will be a behavioral change for every site using JSON API, the answer is essentially "well, you can just ignore the permission."

This is not an acceptable answer if the introduced permission is about "allowed to view more than one entity at a time." It is only acceptable if the issue is about adding a permission for the list builder page.

So, which is it?

We can't move this discussion forward without answering that question.

If it's the former, I can't just ignore the permission. That would be an access bypass as soon as people start using the permission as you seem to intend it be used.

If it's about the latter, then let's just add a list builder permission or come up with a more targeted resolution that's within the scope of the present issue summary.

We can't introduce a completely new access control concept without understanding its consequences while pretending that we're only solving a relatively innocuous thing like a simple route default permission.

That's obviously the fundamental disagreement in this issue:

I still fail to see why not having a clear vision is a problem. This patch introduces a generic concept with a first usage.

Because while that makes perfect sense to you, I think that attitude is deeply concerning.

So, how do I propose we break out of this stalemate?

1. If this issue is about a consistent way to permit access to an "overview" page. Then we need to be explicit about limiting the scope of the permission to this need or come up with an alternative resolution (I've proposed a couple already).
2. If this issue is about adding an access control mechanism for "view many". Then we need to either update the issue summary or create a new issue.

I would welcome both, but I think we're going to loop endlessly if we say we "need to do #2 to solve #1".

If you'd like to argue that we should do #2 and have a discussion about the need for a generic permission for "view many", then we need to have that discussion, and with the potential for "hey, this will also be useful as a list builder permission too!"

Playing the devil's advocate here, but if the worry is that JSON API is currently providing a collection route that assumes access based on individual view permissions and said route would now become "insecure" because there is a new view collection permission which is not being checked, is that not a problem specific to JSON API?

I would assume that it was up to JSON API in the first place to either use the existing convention (admin permission needed) or to add a new permission that grants access to said route (as node does, among others) and then check for said permission in the first place.

Aren't modules ultimately responsible for using the right permissions for the right routes or am I missing something here? Don't mean to offend, but I too am a bit puzzled why something so seemingly simple is being discussed so elaborately.

The answer is 1. or in other words "this issue is about a consistent way to permit access to an "overview" page". I have asked above and will ask here again what specifically you have in mind when you say "limiting the scope of the permission". If you could clarify this point maybe we can move forward here.

Great. I'm glad we've finally agreed on a path forward :)

I'm sorry, I didn't realize that I had not already made that point clear in my many comments. So, now, here it is unambiguously:

To limit its scope, I believe that the permission should be named or heavily documented in such a way that it cannot be conflated with "access to view more than one entity at a time". IOW, It should be very clear that it is about accessing the administrative overview page, not viewing lists of entities in general. To do that, I propose renaming it to list_builder_permission or overview_permission since that name will be more familiar to Drupal site builders.

Prior to you last comment, it seemed pointless to make concrete recommendations because it seemed we fundamentally disagreed about the meaning of the permission. In #32, you said:

The collection permission is a shorthand for "access a list of entities of this type".

Thankfully, I think we are no longer in conflict, because you say in #51:

[In] other words "this issue is about a consistent way to permit access to an "overview" page".

@kristiaanvandeneynde. No offense taken! Hopefully my explanation will be clarifying for everyone.

JSON API does not have Views exports. It has "collection resources", they live at /jsonapi/{entity_type}/{bundle}.

This resource is what one would use to fetch a "recent blog posts" listing: /jsonapi/node/post?sort=-created

Any anonymous or authenticated visitor should have access to that resource—it's essentially the RESTful version of the "Frontpage" view.

Notably, the frontpage view is protected by the view published content permission not access content overview permission.

We decided not to require the view published content permission to access that resource for three reasons:

First, it would simply be duplicative. We check individual access to each entity in the returned collection. IOW, if the user does not have the view published content permission, the response is empty, there's just no point in sending a 403 Forbidden.

Second, there's no equivalent of admin_permission on entity type annotations for a view permission. There's just no way for us to determine which view permission should be used across all entity types. We'd have to hardcode an appropriate access check for every entity type in core and in contrib.

Finally, node itself has view published content but it also has view own unpublished content. What if a user only has the view own unpublished content permission? If we guarded the collection resource with the view published content, it would be impossible for a user to get a list of their own content.

With that established, let's think about what it would mean if a collection_permission were added.

There's two obvious interpretations:

1. It's a permission controlling access to a listing of entities
2. It's a permission controlling access to the overview page

If it's to be interpreted as #1. Then it would be wrong of us to not respect it. It is a new access restriction on listings that did not previously exist. What should we do? Respect it, and we break the thousands of sites which will not have this permission assigned to their users. Disrespect it and we are bypassing something that could rightly be understood as an access restriction.

If it's to be interpreted as #2. Then we should ignore it. It's irrelevant to us. But of course, as it's currently named, I fear some people will expect that it means #1 and will mistakenly believe it's providing an access restriction. Thus, I proposed to name it differently to preclude that possibility.

The worst possible case is that it ended up that both were true. I.e., that the collection permission is documented as #1 (forcing JSON API to respect it) and then also used as #2. That unfortunate combination would mean that any site wishing to use JSON API would need to give every one of their users the collection_permission (JSON API is effectively useless without it). In doing so, they'd then be giving all of their users access to the administrative overview page! Something that I think we would all agree would be an unintended outcome.

And in #32, @tstoeckler confirmed that this third possible case was actually the reality:

The collection permission is a shorthand for "access a list of entities of this type".

Thankfully, @tstoeckler seems to have changed his mind. In #51, he said:

[In] other words "this issue is about a consistent way to permit access to an "overview" page".

All of that means... I think that this issue is no longer blocked since we've appeared to be in consensus about the permission's meaning! 🎉

Now, how do we guarantee that it's documented and understood as such? I've proposed some ideas above. What does everyone think of those?

Thanks for the elaborate explanation Gabe.

I think you're reading too much into it, though. You're arguing a case using /node as an example, whereas the "collection" link template for nodes has always been /admin/content*. Node even has a collection (overview) permission which it uses for exactly that page.

For all intents and purposes, core uses the "collection" link template as "here you may find the overview page", so in my book there can be no confusion as to what a collection permission means: Something to access the collection link template with.

The fact that Node or JSON API expose multiple listings has zero to do with the collection link template; that one always remains the same. I'd even argue that JSON API is free to choose whether or not to respect said permission when building its own listings: If it mimics /node, don't do anything; if it mimics /admin/content, check for the permission. (Even though both are really similar. Thanks, Node.)

In short: A "collection permission" granting access to the page defined in the "collection" link template seems perfectly clear to me. Would you see any issue in an "edit form permission" granting access to the "edit-form" link template?

* Core does not literally state it as such, but there seem to be a couple core content entity types which forget to specify a collection link template. Node and Comment don't specify one, User and BlockContent do...

This is only a reroll of #2953566-25: Allow entities to specify a "collection permission" for 8.8.x

Re #56:

--- a/core/lib/Drupal/Core/Entity/Routing/DefaultHtmlRouteProvider.php
+++ b/core/lib/Drupal/Core/Entity/Routing/DefaultHtmlRouteProvider.php
@@ -318,7 +318,11 @@ protected function getDeleteFormRoute(EntityTypeInterface $entity_type) {
   protected function getCollectionRoute(EntityTypeInterface $entity_type) {
     // If the entity type does not provide an admin permission, there is no way
     // to control access, so we cannot provide a route in a sensible way.

Documentation here needs updating to indicate this is now collection or admin permission.

The patch adds collection_permission to Vocabulary, are there any other Core Entity Types which we can modify in this patch? @Berdir provided some insight in #17, although in #23 @tstoeckler indicated we might be better to use follow-up issues. Personally I don't mind if this patch addressed it for some Core Entity Types where it's a simple drop-in replacement (i.e. those Entity Types already provide a simple collection/overview permission where they only apply it to their collection route, even if they use their own route provider class). I wonder how easy Node and User would be, given those also use Views where available. In #24 @tstoeckler strongly objected to this being added to Media in this issue (citing behaviour changes which would need a follow-up), but also pointed out that we could still add the key to Entity Annotations without actually implementing the usage in that Entity Type's custom Routes/Route Provider, I'm not against doing that but leaving it half implemented leaves room for confusion further down the road, so that leads me back to just using follow-ups for each Entity Type where it's not a simple drop-in replacement.

Adding #2954866: Add a collection permission to media entities to the related issues.

After reading through the lengthy discussion from #27 through to #54, I agree with the final comment in that series by @kristiaanvandeneynde (#54) which I think helps to bring some context to parts of the discussion.

When it comes down to it granting access to the collection route (whether that be HTML, JSON API, or another form) has no real security implications, because unless you also grant access to view or edit or delete the Entities which are supposed to be listed, the user will always see an empty list, regardless of whether they happen to be viewing a HTML table or a JSON list. We're also not introducing any new concepts here, we're simply making some code changes which make it easier for participating Entity Types to provide a permission for "access X collection/overview page". With or without the patch, permissions for these existing pages stay the same and JSON API still returns a populated or empty list, this patch and the scope of this issue don't change that. So the implications for JSON API and others is to actively choose whether or not to alter their processes to check collection_permission, and that's an active choice which should be done in follow-ups, the passive choice for these modules of doing nothing means that nothing changes, everything works the same.

The above is true in all cases, even a contrib module which provides its own Entity Type, the collection permission for that Entity Type will apply to the collection route provided by that module, that's true with or without this patch, the only difference is that with the patch the module can provide the collection permission in the Entity Annotation and save on a few lines of custom code. Again, even in this case, there are no changes for how JSON API behaves, with or without the patch JSON API will still return either a populated or empty list based on if the user has access to view those Entities. In the event that JSON API (and others) make an active choice to change that behaviour from returning an empty list to doing something else is entirely up to JSON API (and those others), but again it's an active choice which must be made in follow-up issues, the passive (non BC breaking) option is to still return the empty list.

I'm hopeful we're at a point where everyone is satisfied, and we can progress this.

I know it is probably outdated, but think can be useful for someone

For some cases you can just add collection route to .routing.yml

entity.ENTITY_NAME.collection:
  path: 'PATH_TO_LIST_FROM_ENTITY_ANNOTATION'
  defaults:
    _entity_list:  'ENTITY_ID'
    _title: 'TITLE'
  requirements:
    _permission: 'YOUR CUSTOM PERMISSION'

And in ENTITYAccessControlHandler.php check access for update/delete/view

  protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account): AccessResultInterface {

    switch ($operation) {

      case 'update':
        return AccessResult::allowedIfHasPermissions(
          $account,
          [
            'YOUR CUSTOM EDIT PERMISSION',
            'administer ENTITY',
          ],
          'OR'
        );

      case 'delete':
        return AccessResult::allowedIfHasPermissions(
          $account,
          [
            'YOUR CUSTOM DELETE PERMISSION',
            'administer ENTITY',
          ],
          'OR'
        );

      case 'view':

        return AccessResult::allowedIfHasPermissions(
          $account,
          [
            'YOUR CUSTOM VIEW PERMISSION',
            'administer ENTITY',
          ],
          'OR'
        );

      default:
        return AccessResult::neutral();
    }

  }

For creating another method

  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
    return AccessResult::allowedIfHasPermissions(
      $account,
      [
        'YOUR CUSTOM CREATE PERMISSION',
        'administer ENTITY',
      ],
      'OR'
    );
  }

And of course add all custom permissions to .permissions.yml, aplly to any role, so that role will get access for view/edit/create/update and don't get administer access

I'd like to bump this issue again, I see it's been silent for a long time now and yet this issue persists in Drupal 9.5.x and I'm guessing 10.x.x as well.

Not being able to simply allow editors/site builders to see the media collection is a *huge* issue in my opinion, granting them "administer media"-permissions is far from ideal and in fact a completely ridiculous idea.

I've "solved" this in one of our projects by simply altering the entity.media.collection route by subscribing to the routes alter event as shown below.

Yet, this is not the solution. I refuse to apply this to all our projects, this is something so trivial that simply needs to land in Core.

Can we make any progress with this issue and hopefully come up with a solution that works for everyone?

<?php

namespace Drupal\[MODULE_NAME]\Routing;

use Drupal\Core\Routing\RouteSubscriberBase;
use Symfony\Component\Routing\RouteCollection;

/**
 * Subscribes to route alter events.
 */
class RouteSubscriber extends RouteSubscriberBase {

  /**
   * {@inheritDoc}
   */
  protected function alterRoutes(RouteCollection $collection) {
    if ($route = $collection->get('entity.media.collection')) {
      $route->setRequirement('_permission', $route->getRequirement('_permission') . '+access media overview');
    }
  }

}

Also want to bump this. The semantics arguments around viewing one vs many entities was a complete departure from the actual goal of this issue: to provide a permission for granting access to the entity.type.collection route. This really had nothing to do with json api or general entity access.

@mglaman had a perfect solution for all of this in #29

list_builder_permission would mitigate the semantics issue around the route being "collection" and permissions being named "overview". The use case we're working against is the list builder, not an API resource with a collection of entities.

Or even just...collection_route_permission. IMO this would be even more specific.

Alright, I've created an MR that includes the patch from #56 and a fix for the documentation issue noted in #57. After reading through things again today, I think that @AaronMcHale was spot on in #57, so I've left the permission name as-is instead of changing it to list_builder_permission or whatever else. I agree that followups are probably the place to handle additional core entity types. If tests pass then I will mark this RTBC in the hopes that we can move things forward.

Alright, given that tests are passing, and reiterating that I think #57 perfectly sums up why this approach is okay, I'm marking this RTBC. Let's see if we can't get this in after 5 years.

#1975064: Add more granular block content permissions introduce a access block library permission for block_content in 10.1.x, so we could also add that to the block content entity type annotation.

Can the change record be updated please.

Was last updated 5 years ago to be introduced in 8.6

Thanks! Will try and review this evening or tomorrow if no one beats me to it.

One nit picky I see is if the new get function could be type hinted. Probably can be handled during commit but if you get time.

Hiding patches as the fix is in MR 4201

Test coverage appears to cover the different combos of collection and admin permission.

I don't see any lose for those using just admin permission either.

Think this is ready for committer review.

Just had a quick look at the MR and it also looks fine to me.

Committed/pushed to 11.x, thanks!

This is very API focused but tagging for 10.2 release highlights in case there's something to group it with.

Thank you so much @tstoeckler for your detailed write-up in #76! I was wondering how this impacted JSON:API and was surprised to see no mention at all in either the diff or the change record. I do think it'd be valuable to update the change record with how exactly this does or does not impact "the API use case" — no matter whether it's REST, Views REST export displays, JSON:API or GraphQL.

Would it be possible to still add that? 🙏

I think explicitly documenting that they are not affected would be very valuable! People reading this will wonder "how does this affect me?", and seeing JSON:API + routes-for-your-entity-type information is great, but I think the first 2 bullets you listed in #88 would make excellent additions to the change record! 🤩

if you have issues with the permission this maybe can help:

entity.ENTITY_NAME.collection:
  path: 'PATH_TO_LIST_FROM_ENTITY_ANNOTATION'
  defaults:
    _entity_list:  'ENTITY_ID'
    _title: 'TITLE'
  requirements:
    _permission: 'YOUR CUSTOM PERMISSION'
    _role: YOUR ROLE

it's very like to the one shared by vlad.dribas but with you don't pass the role sometimes don't work correctly.