[PP-1] Referencing new media requires "administer media" permission

Have the same issue in my current project so I created a patch which works for me.

@AbrahamE - Working great for me, thanks for the patch!

1. +++ b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
   @@ -25,23 +25,28 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS')
   +    if ($this->currentUser->hasPermission('administer media')) {
   +      return $query;
   +    }
   +    elseif (!$this->currentUser->hasPermission('administer media')
   +      && !$this->currentUser->hasPermission('view own unpublished media')
   +    ) {
   

   I think it would be better if this method kept the current layout of having just one return.

   Also, the second check to the admin permission isn't necessary.

2. +++ b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
   @@ -25,23 +25,28 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS')
   -    // In order to create a referenceable media, it needs to published.
   

   Won't we still need this if the current user doesn't have the permission to view an unpublished entity they own?

3. +++ b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
   @@ -25,23 +25,28 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS')
   +    // Permission to "view own unpublished content" allows
   

   Should say 'media' not 'content', as content means nodes.

4. +++ b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
   @@ -50,10 +55,16 @@ public function createNewEntity($entity_type_id, $bundle, $label, $uid) {
   +    if ($this->currentUser->hasPermission('view own unpublished media')) {
   

   Do we need to handle the case where the user doesn't have the permission?

Created patch against 9.4.x and address comment #12

Updating patch #12 as custom command failed and attaching the reroll file

I’m changing this to a Bug Report since I would expect this behavior from the view own unpublished media permission, but let me know if I’m wrong here.

I cleaned things up and addressed comment #12 concerns. Also, I noticed the existing patches don’t apply to the Media Library Grid/Table Widget which also has this issue. I updated the Views config to mirror what is being used on the Media Library admin page with the media_status plugin. Everything appears to be working with the view own unpublished media permission now.

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Try again...

Unsure, maybe another permission to check when the user doesn't have "administer media" permission.

Don't think this matches the solution in the MR

As a bug will need a failing test case showing the issue.

This appears to not only be limited to referencing a users own unpublished media. We have also experienced users not being able to reference unpublished media from other users. In this case, the user has the permissions to 'create media' and 'update any media'.

Related issue #2845144: Users without 'bypass node access' permission can't reference unpublished content even if they have access to it is talking along similar lines but for content, not media. To me this feels like a shared issue around unpublished entities being hidden from users who have access to them.

I think similar to the related issue - this check should also take the permission view any unpublished content into account since view any unpublished content also applies to media

Perhaps this is realistically postponed until #2845144: Users without 'bypass node access' permission can't reference unpublished content even if they have access to it lands so that the BC impact can be the same, once that config in the related issue is added it can be used here>

I have started expanding test coverage for the selection handler - end of the day for me so assigning to me as I intent to continue working on this tomorrow, still need find where the validate new references is tested.

Added additional permission check mentioned in #28 - have not added this to the validate method yet - that is still to be done.

I still believe this will need to wait for #2845144: Users without 'bypass node access' permission can't reference unpublished content even if they have access to it to land as that introduces a BC layer for this logic change to the base handler - I think it makes sense to wait for that issue to land rather than trying to work that in now and likely this could be postponed as a follow up - but I will try complete the remaining work soon as I need this functionality now and do not have a concern about it being a BC break.

I have expanded some tests - from a quick look I couldn't find existing tests covering the new entities behaviour - the validation constraint looked to be the appropriate place to add that.

Once #2845144: Users without 'bypass node access' permission can't reference unpublished content even if they have access to it lands with the include_unpublished_entities setting on the base handler that can be incorporated here. Setting to postponed based on that.