[META] Implement stricter access checking for the media library

Uneducated question (sorry): Does this view display need to be a page display for a particular reason? I wonder if it wouldn't make things easier to have a display type without route access (as Entity Browser does), so it could only be used inside our dialog system.

This would also make it easier to detect which view/displays are selectable in #2971209: Allow the MediaLibraryUiBuilder service to use an alternative view display

#8: Thanks for clarifying!

Discussed this with @SeanB today as part of the media sprint, it seems like for now just a new permission to access the media library may be in scope here, and then the widget will use that as part of the different access checks it will need to perform.

I'll work on this.

This implements a new select media library items permission as a first step to approach access to the Media Library in a better way.

+++ b/core/modules/media_library/media_library.permissions.yml
@@ -0,0 +1,3 @@
+select media library items:
+  title: 'Select media library items'
+  description: 'Users with this permission can select entities from the Media Library in the library widget'

So … if you don't have this permission, can you access the media library but in a read-only mode?

After some more discussion, we are arriving to the conclusion that this new permission isn't perhaps the best approach in the end.

The media library widget will have a controller with a custom access handler that will:

1- When opened from an entity reference field:
a) Grant access to the media library view display if the user has access to the field the widget is attached to. (Because we assume if a user can edit a field that allows referencing media items of a given type, they should be able to see (and hence select) those media items)
b) Grant access to the add form if the user has access to create new entities of that particular type.

2- When opened from the CKEditor button:
a) Grant access to the media library view display if the user has permission view media and has access to the text format the button was in.
b) Grant access to the add form if the user has access to create new entities of that particular type.

(That's my understanding of it at least :) )

The current Widget view display will then not be a page display anymore, so the controller's route would be the only point of access to the media library widget display.

With that, we can just close this issue as outdated. (I'm holding off on doing so until we have an issue number to refer to where the work described above is being done).

@seanB and I got in a Slack discussion with @alexpott today about the best way to approach this. We decided that we need three things:

1. Right now, the media library depends on a state value object derived from URL query parameters. Users are not supposed to tamper with those parameters, but we have no mechanism in place to prevent that. So we need to implement a tamper-proof hash, similar to what Media does in its IframeUrlHelper class, to prevent people from messing with the query parameters in the first place.
2. Secondly, as @marcoscano points out, a permission might be too blunt of an instrument for controlling access to the media library, simply because it can potentially be used in a bunch of different places and in different ways. With a permission, a site builder would need to grant at least two permissions to authors in order to allow them to use the media library at all -- they'd need the "view media" permission, and permission to access the media reference field at all. That's a big fat DrupalWTF that we should do our best to avoid. Instead, we will implement some sort of system, probably based on the event system, to ask the opener (i.e., the field or the WYSIWYG editor in question) to allow or deny access to the media library, based on the parameters in the state object. Since the state object is central to computing access this way, we'll need the tamper-proofing in place first.
3. Finally, the media library view has two displays (grid and table), and neither can really be used without a valid state object. Yet there is nothing enforcing that constraint -- if you have the "view media" permission, you can see the media library. So we want to deny access to all displays of the media library view (except the standalone ones at /admin/content/media) if there is no valid state object present.

We decided that since spreading this work over multiple issues might increase the possibility of security holes, @seanB will implement all of this in a patch in this issue as a proof of concept, and then we'll split it up into separate issues to make it easier to review and commit.

Created issues and updated IS:
#3038241: Implement a tamper-proof hash for the media library state
#3038254: Delegate media library access to the "thing" that opened the library
#3038350: Deny access to all media library View Displays if there is no valid state object

#3038241: Implement a tamper-proof hash for the media library state was completed!

#3038350: Deny access to all media library View Displays if there is no valid state object was also completed.

I created #3063216: Inaccessible Media entities still have rows generated for them in the Media Library view, containing form wrapper markup *and* the media label based on #3038254-62: Delegate media library access to the "thing" that opened the library.

#3038254: Delegate media library access to the "thing" that opened the library landed this morning, which concludes this meta. :)

I unfortunately have to disagree 😐 Sorry!

#3063216: Inaccessible Media entities still have rows generated for them in the Media Library view, containing form wrapper markup *and* the media label was newly discovered in #22. While that may not be exactly what the intent was of this meta originally, because that was focusing on certain aspects of the media library, it definitely falls under the umbrella of "stricter access checking". Right now #3063216: Inaccessible Media entities still have rows generated for them in the Media Library view, containing form wrapper markup *and* the media label arguably is disclosing sensitive information.

Given the issue Wim references in #24 ( #3063216: Inaccessible Media entities still have rows generated for them in the Media Library view, containing form wrapper markup *and* the media label ) has been fixed, I believe this can be closed.

Yay, thanks, @azinck!