Add 'View media browser library' permissions

omg yes! We need this badly. Will review this week.

Looks good to me.

However, the upload media form has this on it:
if (user_access('administer media') || user_access('edit media')) {
$plugins['upload'] = array(
'#weight' => -10,
);
}

So a user wouldn't be able to upload anyway unless we do something about this. Do you want to solve the larger problem here, or just commit this as is and then address that?

Yes, we definitely want this. But I'm scrubbing the queue right now to make sure bugs and feature requests are correctly categorized.

While I support this, I'm nervous about committing it w/o tests since once you introduce permissions, you better be sure they work in all cases, and I don't trust our code enough to say that.

I'm leaving as needs review though in case other maintainers +1 just throwing it in there.

The view own media should be a nice one to had too! Is there a way to restrict the view of media only for the current user. That way users only see what they upload to the website when adding media to content ?!?

@becw, whattya think, should we commit this and hope for the best? :)

Subscribing

Kinda related #1109534: Folder Permissions

Okay, tentatively RTBC. :)

How would users create media (i.e. content with a multimedia asset field) in the first place? Unless I'm missing something, they would need the 'edit media' permission in order to access the media field widget.

I think #6 is almost good to go, but needs to be re-rolled for latest 7.x-1.x, and #3/#4/#14 should be added to the patch. Note that the bundled media_internet module has a permission for adding via a remote source, so we're already good there. If anyone's motivated to see this committed to Media, please submit an updated patch. Thanks.

subscribe (waiting for teh patch to come :-))

subscribe

Subscribe (and sorry for the useless post)

sub

subscribe

I've updated the patch becw provided with the following:
* Rerolled Bec's patch (#6) with the latest version of 7.x-1.x-dev
* Added an 'Upload media' permission (Addresses comment #4)
* Added a 'View media browser library' permission (I needed this too)
* Modified includes/media.browser.inc to respect 'Upload media' permission (Addresses #3, I think)
* Modified includes/media.browser.inc to respect 'View media browser library' permission (Yay!)

It wasn't clear to me what should happen in order to address the issue expressed in comment #14, so I didn't do anything with that.

I "lightly" tested this, so it would be great if others would do so as well. Also, I didn't add any tests.

Hopefully this is a step forward though!

Erik.

Rerolled the patch in #21 so it should apply now. Although, looks like it has some whitespace issues (tabs).

Differences from #23:

1. Only add plugin tab if its #title element is defined. Fixes Notice: Undefined index: #title in media_browser() (line 74 of includes/media.browser.inc).
2. Implements rather than Implementation of hook_media_browser_plugin_view().
3. Fix whitespace issues (tabs).
4. Check for either edit media or upload media permission in media_field_widget_form().

Subscribe. Will this be committed soon? I'm working on a site that needs the media popup tabs controlled per role, I'll apply this patch and hope it works. Thanks!

Had whitespace errors applying this patch with Git against Beta5. However, it does seem to work against that branch just fine.

Should this be re-rolled for 2.x ?

+1

Seeing that 2.x will take a while, hope to see this in 1.x ... any plans to commit before RC ?

thanks

Subscribing!

i'm pretty sure that most feature requests are going into 2.x at this point, although since the patch at #24 looks fairly good, maybe we can backport this one after it goes in. note we're pushing forward with development of 2.x at the sprint next week, so it's not as long away as you might expect, @rafamd.

subscribe

I tested #24 and works as expected.

I was wondering if this function is put in the latest -dev version? I am using Media Gallery and in order to let users create their own galleries this permission must be added first before further development of Media Gallery can be made.

Well, one more thing, I think user should only upload images to their own galleries. This function seems not included in the current version.

Anyone there?

subscribe - One more interesting consideration. In the context of organic groups, would group members be able to see media files that they have access to view/edit within the contexts of their groups?

This should be postponed pending #1227706: Add a file entity access API which provides a major update to permissions in file_entity itself.

@legendm33066, @CarbonPig those permissions are beyond the scope of the media module. However, with the new views integration you can easily build views which support the permissions that you want.

changing scope of issue as with #1227706: Add a file entity access API we'd have 'edit own files' and 'create files' permissions in file_entity.

Nice job!!! Hope a stable 2.x release will be out soon!!!

In the meantime here's a reroll of #24 for 1.x

ps. don't expect tests to pass

reroll again.

siwtch to 2.x again since tests passed.

+1

Patch does not apply to 7.x-2.x.

#44: media-992978.patch queued for re-testing.

per #38 I did a reroll of #24 for 1.x

Patches need to be applied to 7.x-2.x first, then backported to 7.x-1.x.

Anything holding this up?

I am not a coder, so my knowledge is limited here, but what about integration with http://drupal.org/project/access(formerly conditional roles)?

Hi, g76. As the author of the module you mentioned, I can tell you that it's much too early to be asking Media to integrate with ACK, if it even makes sense to do so at all.

ACK is being designed to add a layer of access control between existing Drupal permissions and the objects to which they apply... Thus, while I intend for ACK to work with Media, Media ideally shouldn't have to do anything to work with ACK. Nor would I expect the Media maintainers to have any such interest at this time...

While I have high hopes for ACK, it remains to be seen whether it will be embraced by the community. I also don't expect it to see a production release for another couple of months. Media, on the other hand, is already production-ready with a distinguished development team and proven community support. Let's not sidetrack their work with requests to give consideration to a project that's just getting off the ground. :)

Currently using the 1.x patch. Seems fine but had to make an adjustment to stop error in media selection window. Sorry, haven't figured out creating git patches yet.
Changed
function media_access($op, $media)
to
function media_access($op, $media=NULL)

I noticed that a User can still see everyone's media in the media library. Would be nice if there was a view own media in media library + view all media in media library permission. Don't know if that's easily doable. Would be necessary when using og module or when you have media files which should be private. Keep up the good work. :-)

I agree with @robwithhair that there needs to be a permission for view own media in library and view all media in library. I'm setting up an artist application and I can't have applicants seeing or using what other applicants have posted.

It'd be great to see media files integrate somehow with private group content options. I have og setup with private content and don't want groups seeing each other's media files.

Ok, so patch needs to be rolled against 2.x first - but I still need this now on 1.x, so patch updated to apply against latest 1.x code.

can someone clarify/update the issue title and summary to better indicate what is going to ultimately be done in this issue? i think/hope it's going to not only add an "edit own media" permission to enable the library plugin but only for the user's files but also to add an "upload media" permission so all users don't necessarily have access to their files but can still upload. just my 2 cents, but it would make sense to follow the fields_permissions module's approach of "create (add) media", "edit own media", "edit all media", "delete own media" and "delete all media", but i don't know how much that complicates the changes. thanks for all the great work on this module so far, i'm really enjoying the power it provides so far and look forward to all the improvements to come!

re #59: I think Dave's comment in #40 should clarify things a good bit.

I changed the priority to reflect the priority of the corresponding issue over at #1227706: Add a file entity access API. Imho these permissions are release blocker.

Also: closed #1679530: /media/browser is accessible for anonymous users which exactly asked for this.

Regarding mrfelton's patch in comment #58:

I tried your patch, and am getting an error:

Warning: Missing argument 2 for media_access() in media_access() (line 571 of /sites/all/modules/media/media.module).

Everything else seems to be working nicely.

A quick fix:

571 function media_access($op, $media = NULL) {

+1 to get this functionality, would be nice to have granular control over what roles can use

See this module I just wrote to limit media to group context: #1830734: Create new hook to alter media browser list query

#42: media-992978.patch queued for re-testing.

Cant see a valid patch against 2.x
Dropping to NW

a brand new one

PS: I didn't check how kind it is regarding the Media browser Views > "Access options"
I don't know which method is preferable which one is the more obvious, which use-cases they match.
In my case it's all about disabling the media browser for anonymous user uploads.

In aparté: I don't know about file permissions and Drupal sessions, but the best thing ever would be to provide a way for anonymous to get a restricted media-library offering files recently uploaded by "him" (it's IP).
Obviously this is not the best place to talk about that, but maybe such a case could be considered from a media-browser permission POV.

I started to reroll this to fix some issues (code style, incorrect permission name, $account not being checked in the user_access() call), but then wondered... is adding this permission really a good idea at all?

Somehow it doesn't seem 100% right to me to hardcode it in the access() method... And if it's not hardcoded in the access method, then it's just doing a normal Views access check, which Views already gives you the ability to override (on a view-by-view basis) with whatever permission you want.

But in any case, here's the rerolled patch. I guess there may be a use case here to have a single permission which is absolutely 100% required in order to view the media browser (that is why I thought I needed this patch in the first place myself).

Maybe "View the media browser library" would be better named "Use the media browser library" ?
Other than that, since I found a way to use views built-in permissions handling, I'm fine with this patch pushed or not.

Yeah, I was just following the issue title here, but I think you might be right about the permission name :)

It's probably a good idea to make the machine name of the permission match the human name pretty closely, though, so in that case the machine name could be changed to "use media browser library" instead too.

#69: 992978-views-browser-permissions-69.patch queued for re-testing.

I downloaded the patch from #69 to the Media folder and applied,but no luck.Which file should I apply the patch?
thanks

$ sed -i 's/view media browser library/use media browser library/' includes/MediaBrowserView.inc media.module

About permission granularity, I would like to mention, here: #1904108: 'create files' permissions should not forbid creating file for a specific content type

the above bug has just been closed as "up to media module": would be useful if adding a media to a node didn't depend upon the 'create files' as it's the case for images (see hook_media_browser_plugin_info)

I'm a bit concerned about change being painful for people who don't realize they need to update role permissions. That being said it makes sense. I added a permission check on the actual wysiwyg plugin to hide the browser button if the user does not have access.

After reviewing this, the patches from #67 forward don't seem to add anything that can't be done using permissions in the view itself- you can restrict access to the default media by role with the view which also prevents the rendering of the tab.

Agreed..i dont see any use for it as well..just modify Views access settings, i think its far more flexible

Which view should I configure permissions for the media library?

It's a view called media_default or "Media Browser".