Granular editorial access control

Showing tabs in the browser should be an option passed in by the caller IMO. Showing or not showing isn't going to provide any level of access control because it can totally be faked.

WYSIWYG is protected by input filters, and I kinda feel it should stay that way. I'm not opposed to a check on submit though to make sure you didn't try to use something you aren't supposed to. Still, seems like duplication of effort.

Applying permissions to plugins is a simple enough matter though if we want to go that way. I think though that it is cleaner to have the callsite get a setting of how to launch the browser than to hard-bake permissions logic into what is just a display tool. (then you could show youtube in WYSIWYG, but not other places). We need this anyway because media as field needs to launch and pass in params.

Regarding how permissions are structured, this is dodgy. We don't want to replicate node access. That is a royal clusterf*ck and we all know it. Plus, it won't scale when you have a gallery. My feeling on it is we should investigate if entiies give us anything for free. IF not:

media type x role permissions is a good place to start, and will be fairly quick, can be cached as well.

When launching the WYSIWYG do we need to hardbake the role x type condition into the library callback? Or do we just pass in conditions for the library.

Anyway, that's not real validation:
1. WYSIWYG input filter will not render things you aren't supposed to pick
2. We need to pass what tabs should be included when the media browser launches. To accomplish this, we can simply pass in options. The downside is that this means the forms -upload, from url - get rendered anyway (Something I was trying to avoid by making it all load real-time), but at least the library gets ajax loaded.
3. The options should really be related to the input format (if that's possible), not global IMO... But if we have to do global, that works too.

In Sum: I'm open to whatever, I'm more interested in passing options to the browser than hard-baked rules and assumptions on the server side, and if we start calling a function everytime a piece of media gets loaded to check its perms, we're going to be in trouble.

Rambling,
Jacob

Hmm... yeah, this is a PITA. It seems to me a like a global entity_access API is in order or something. node_access is such a drag to work with and so slow, it makes me worried here when you have thousands of files.

I guess it's only for people who want to turn it on, which is probably not too many.

One thing we are not respecting though, which we probably should now is if someone gives access to anonymous users, they shouldn't see private:// files I suppose.

So I guess we have 3 approaches:

1). Call a function on every entity_load() with a hook to either load or not load the media based on arbitrary access rules

2). Maintain a potentially *massive* table of ACLs like node_access does - and a system for this.

3). Make a few permissions like: view_$type and edit_$type, and just use those perms in various places where we need to get media / show stuff relating to those types.

unless we have to do one of the first 2, the 3rd one is really attractive in that media types are flexible instruments and the check will be quick, and can be done w/ SQL instead of on a per media basis.

We can always do the first 2 in addition or later as well.

This becomes a lot easier to think about when we've got a custom EntityController I think.

I'm a little concerned here. I thought we discussed this and came to a decision, which was:

1). Someone is going to write a views plugin for this which will do everything we do

2). Making our code complicated or slow, or building a flexible framework around it won't help, because Views is Views and we don't want to do the same stuff

3). This code seems to be doing the same thing views does

4). We will have 3 *harcoded* filters, by type, by uid and by url LIKE "%search%".

This will support "youtube://%", it will support "%.jpg", and it will support "Only Images by me".

I don't understand hooks like this:

$streams = module_invoke_all('media_browser_streams');

  // Allow modules implementing hook_media_browser_streams_alter to alter
  // the set of browser stream filters.
  drupal_alter('media_browser_conditions_streams', $streams);

When you've already got a place for conditions, etc. Further, but specifying $streams, won't it just go on to specify extension? and then size? and then lyrics in the id3 tag, and then resolution, etc. etc.

Isn't this all better served by using an actual query builder?

See http://drupal.org/node/697106#comment-2567976

Thinking more about this, any type of permissioning should be done at the EntityController level or immediately above it through some API interface. But probably in the EntityController itself.

Because if you do something like what is written above, there is a not a consistent interface on which to deny access or grant access. I think first we need to determine what we are going to do access control for. I think I stated my idea that a first pass would be type X role and "My stuff" vs. everyone elses stuff. This is Core's default behavior for nodes.

But either way, we need to have it in the load mechanism, so whenever a media entity is loaded, the check is done.

I would however, recommend that we start with access control which can be incorporated into the SQL to select the fids. this would be in MediaEntityController::load(). Then, it's not a case like:

Load 5000 pieces of media
iterate through each one
call a function on each one to determine access
Return 3.

We can do uid and type easily in the ->load() method. "stream" could also be done with a LIKE condition.

Look at entity.inc in buildQuery() This is where the condition "magic" is happening. Most likely, we would want to implement load(), copy what is there currently, but after buildQuery, add permissions logic to the query if that module is enabled.

Tested this and thumbnails are not shown, only filename is visible.

I committed this patch to CVS HEAD to address #2/#10. This is good enough for a 1.0 release, IMO. A better access control system would be awesome though, so if anyone feels like working on that, ++!

In my case I'd like to restrict available/selectable media in the media gallery to those uploaded creating a specific content type.

I mean... if I upload an image creating content "cntfoo_1", I don't want to see that image in the gallery I can browse when I create a content "cntfoo_2" because it's useless in my case (and sometimes should be denied because of private content for example)

What's your opinion?
I think it isn't achievable at the moment...

It's great to see this patch in place. Thanks effulgentsia!

Using 'administer media' is a bit extreme. I have a scenario where folks need access to private files but should not have access to configure the media module settings.

Perhaps we could use a different permission to check? I would like to think that someone who can edit media could upload/view private files. Could we change it to 'edit media' or should there be a new permission for this sort of thing? 'private media' perhaps?

I'm happy to commit that change, but just wanted to get some confirmation first.

The solution currently in place also makes it impossible to allow read-only access to the information on those media/* pages. For example: I have a site that stores lots of image files, all of which have "private://" paths to keep them from being downloaded by unprivileged users. I'm also leveraging file_entity to add useful fields to the image file type, and the only place that information is exposed is on the media/* page for each file. Unfortunately, since the only users who can see media/* pages for private:// files are users with "administer media", I can't grant users the right to view those pages without also granting them the right to deface them.

I'm with robeano on this one; we need a permission other than "administer media" for this to work correctly.

Thanks!
Adam

subscribe

Has this conversation now shifted to #1227706: Add a file entity access API ?

Yes, let's mark this as a duplicate of #1227706: Add a file entity access API.