Add "view unpublished $bundle media" permissions for each media bundle

See attached patch

Added test and removed the setReason() since we now don't have a single reason why the access can return FALSE. Also, I checked other modules in core and setReason() isn't used often.

Test failed due to the MediaResourceTestBase class that was validating the getExpectedUnauthorizedAccessMessage(), but the message was removed. I updated this class accordingly.

+++ b/core/modules/media/src/MediaAccessControlHandler.php
@@ -26,10 +26,8 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
+          ->orIf(AccessResult::allowedIf($account->hasPermission('view any unpublished media') && !$entity->isPublished()));

Nitpick, but for readability I would prefer to have the allowedIf() and orIf() together.

Updated the patch with the review comment in #10

After some digging we found out that the message couldn't be deleted because of the different REST tests that validate CRUD access to entities. But we made the message more general since it doesn't only check on the 'view media' permission.

Looks good to me! If the testbot agrees I would say this is RTBC!

Hm, not sure about this, for a few reasons:

1. We intend to deprecate the "any" media permissions (create any, edit any, etc.) in favor of per-bundle permissions. I'm not sure why a blanket permission for this would be different from those?
2. Even nodes do not provide a blanket permission like this separate from administrator-level permissions, so there is not a core precedent for even having this permission.
3. I'm not sure if it belongs in core. Contrib could easily provide such a feature for modules that require more advanced access control.

Tagging for subsystem maintainer review and setting NR for further discussion.

Also, this is a feature request, so retitling and changing the category accordingly. Thanks!

Re #16:

Even nodes do not provide a blanket permission like this separate from administrator-level permissions, so there is not a core precedent for even having this permission.

Note that there is a very old feature request for adding a "view any unpublished" permission for nodes at #273595: Move permission "view any unpublished content" from Content Moderation to Node, and many people have expressed the need for having it in core in that issue.

Should we focus instead on #2809177: Introduce entity permission providers ?

Hello guys!

The patch from #14 no longer applied to latest Drupal core. I do understand the concerns/inputs that were given since the comment #14. I think the idea of entity permissions provider is really nice, but for now we just needed a permission to view unpublished media :)

The only difference I introduced compared to #14 is to make it per bundle (especially in the light that it is the desired direction for all the access system). I haven't worked on tests since first I just wanted to get a confirmation whether this set up of permissions is acceptable.

Applied patch#25 on Drupal 8.9 and Drupal 9.1 and its working for both. +1 for RTBC

Steps:
1. Visit /admin/modules
2. Install Media and Media Library on D8.9 and D9.1
3. Now Visit /admin/people/permissions
4. Check whether "View any unpublished media" permission is visible
5. Then Create an authenticated user with permissions "Image: View any unpublished media".
6. Keep this permission unchecked for anonymous user.
7. As an admin, create a node with 2 image files using entity reference field.
8. Now unpublish one of the media file.
9. Open the created node as an anonymous, only 1 image will be visible.
10. Now open the same node as authenticated user, both the image can be seen.

Screenshots are attached for the tested scenarios.

Applied patch #25 . The new permissions for items in media bundle are available after the patch
Screenshots:
before:

after:

RTBC

Thank you for your contributions and the thorough manual testing!

This is not quite ready to go, though, because it needs automated test coverage. If anyone would like to write it but doesn't know how, the documentation at https://www.drupal.org/docs/automated-testing/phpunit-in-drupal is a good place to start. At a minimum, we need a test that proves the functionality works as intended -- that is, roles with the new permission(s) can see unpublished media of various types. It would also be useful to test that the permissions work correctly when Content Moderation is enabled, and media entities have moderation applied.

In light of this, I'm going to mark this issue as needing tests, and kick it back to "Needs work" for that. :)

Patch #25 is working fine. The new permissions for items in media are appearing after apply the patch.

I've tested #35 with dev versions of 8.9 and 9.2.
I've done the same test as in #27 and checked the list of permissions as in #29.
Everything worked as expected.

I've also tested #35 with 8.9.13 and found no problems.

Changing to RTBC after feedbacks.

@xjm's comment on whether this is suitable for core is from 2018, but I'm inclined to ask the same question - per-bundle permissions means a very big permissions form usually. Tagging for product manager review as well (either subsystem or product feedback would be fine here, but think we need one).

EDIT: Never mind; need to ponder this a bit more.

Re-titling.

I am a core product manager.

I understand @acbramley's use case, however that the use case is not possible to implement with core only permissions does not mean core should provide the feature. Content (nodes) have "view own unpublished content" but it does not even have permissions for not-own unpublished items. Only if #273595: Move permission "view any unpublished content" from Content Moderation to Node lands would it have more of a blanket permission but even then not per bundle.

Why are media items special that *core* should provide this feature? Is it not possible to provide this feature as a contributed/custom project?

The node permission's pair "view own unpublished media" already exists in core.

BTW the "any" media permissions deprecation issue is #2925459: Deprecate generic media permissions.

Applied patch #25 . successfully applied
After patch new permissions for items in media bundle are added
Thanks for the patch

Until the subsystem maintainer can review.

Sorry for the propably dumb question, but after searching for a core issue or an answer to this requirement (I only found good solutions for nodes), I'm asking myself, why only "view unpublished $bundle media"?

If you have "private" media types (with private download fields) that should only be accessible for certain roles, there doesn't seem to be a way, as there's only "View media" and "View own media".

So shouldn't this issue also solve that part and add "View" and "View own" for media?

What am I missing?

Edit:
For that reason I now created #3353503: Add "view $bundle media" permission
As written in #22 I'd vote to instead of creating another custom solution we should all focus on #2809177: Introduce entity permission providers and postpone this issue on that one... shouldn't we?

@Anybody re: #52: See #3213506-12: Add a way to limit the available media items when embedding into other entities and other comments in there for why a "View own media items" permission isn't the right approach (IMHO).

@dww: Thanks! You can see our use-case in #3353503: Add "view $bundle media" permission. "View own" isn't really relevant for us, but we need to limit (only certain) media types to elevated roles. I think that's not an untypical use-case. I got your point in #52. In our case, it's expected behavior, that lower roles can neither see the media nor download the file. But let's better proceed with the discussion in #3353503: Add "view $bundle media" permission? If you'd like to have a look.

--

If anyone needs this (view own) in the meantime, the functionality could perhaps be added into the https://www.drupal.org/project/media_private_access module. Just create an issue and a MR there, so there's a solution until this might be (or not) fixed in core?

rerolled for 10.3.10, didn't include the tests, since expecting this to get a more general solution

#58 doesn't apply because of windows lineend char

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.