stephenmcd / filebrowser-safe

File manager for Mezzanine
Other
42 stars 104 forks source link

Permissions? #52

Open trolando opened 9 years ago

trolando commented 9 years ago

Currently there is no way to control who is allowed to upload/replace/rename/delete files.

It would be nice to add Permissions. There is a simple way to add the permissions to filebrowser_safe, as in https://github.com/trolando/filebrowser-safe/commit/fbec0b2983014732d1c600565bdfe1d63e6e0deb

This commit is based on how django.contrib.auth adds permissions for the models, using the post_migrate signal introduced in Django 1.7. (earlier versions use the post_syncdb signal).

The above commit simply adds some permissions, it does not enforce them. This would require adding decorators to the correct functions (mkdir, _upload_files, rename, delete), correctly handling overwriting files, and correctly hiding functions that are not allowed in the UI. I do not know filebrowser_safe well enough to be able to correctly protect files from being overwritten.

Another thing to consider is whether browsing the Media Library is always allowed or not. There could be reasons to introduce a permission for this as well, for example if someone is allowed to perform certain actions on another app in the website that is not related to the Media Library. Enforcing a browse permission would require additional changes in Mezzanine, since the Media Library is not connected to Mezzanine via an app, but via the 'fb_browse' reverse URL.

stephenmcd commented 9 years ago

Thanks Tom - I think this is a good idea, can you go ahead and set up a complete pull request?

I think browsing should always be allowed - restricting that would cause many problems, particularly when it's integrated with file fields in other apps/models.

trolando commented 9 years ago

For a complete pull request, including actual permission enforcement, I need to know how to really prevent overwriting.

Is my assumption correct that Django does not actually store the file on disk, but keeps it in memory in the request.FILES object? And that the actual "possible overwriting" occurs on line 333 in views.py when default_storage.save() is called? I am unsure what happens in lines 334--335 with the file move.

Then, I'd have to figure out how to integrate this with the UI. For example, instead of a warning "This will overwrite XXX" there should be an error message, and so on.

stephenmcd commented 9 years ago

Can you post these questions to the mailing list? You'll hopefully get more interaction there.