Files from private stream cannot be downloaded

This bug is probably not going to be solvable in Drupal core version 7, but maybe version 8. Perhaps the best solution is for File Entity module to implement hook_file_download() to apply simple access criteria in order to determine whether the file is accessible or not?

Steps:

1. Installed File Entity (latest from 7.x-2.x-dev).
2. Role authenticated user receive view own files, delete own files, view own private files, create files, edit own files permissions.
3. At admin/config/media/file-system set the Default download method to Private local files served by Drupal.
4. At admin/structure/file-types/manage/image/edit set the Allowed streams only to Private files.
5. As UID == 1 add an image (file/add).
6. Go to view (file/[file_id]). You can see that the image is not accessible.

Sorry. Accidentally moved the status.

The last submitted patch, file_entity-private-file-download-1420812-7.patch, failed testing.

Fixed some comments.

i like this approach.
only thing that remains is to remove all references from file_entity_file_entity_access to $grants, since grants wont be used now

Here's a patch according to #12.

This is hard to read.
Why dont you add the message in the array as well with a key message and format the array nicely?

doesnt this trigger two notices/warnings since if owner is empty both variables ($url and $message_file_info) are not initialized?

Everything else looks good

Reworking according to #17...

Here's only the test which should fail.

Here's the patch with test.

The last submitted patch, file_entity-private-file-download-1420812-21.patch, failed testing.

The last submitted patch, file_entity-private-file-download-1420812-21.patch, failed testing.

Well, the only unique thing that can act as a key is the message itself. Adding a new key is useless but we can make something in between by having the array as a vector array

Yeap, thats what i actually meant..sorry for typing it the wrong way, but we got in the same page eventually, much better now

No. That's how it was designed. The first test case will always create $url and $message_file_info. Same the second. The others will reuse $url and $message_file_info already created by the file owners. The meaning is: Non-owner access is tested against files owned by somebody else.

Yeah well, this is not documented in getPrivateDownloadAccessCases(). So if someone in the future adds a case in the top without owner it will trigger the error. Instead you should just initialize those variables before foreach and make sure you dont get any problems no matter what.

My point of view:

• There's nothing to initialize here. Non-owners should use previous created files.
• Developers that are creating tests should know what they are doing, better than other developers. They will see the notices and they will fix.
• Test cases are just cases. They are special, specific, custom cases and should be treated so (like manual testing).

Anyway, I will sort that array on owner assuring that "owner" cases will be always on top. IMO that is superfluous but I will fix it in that way so the order will have no relevance because every time owners will create files (and $url, $message_file_info already).

Well, this I think it's not a good idea. We're dealing here with file entity, not other entities that are bundling files as field or property. IMO the the whole entity (file + meta + fields) is a single thing: the File Entity. If we want different access level for file and fields, just bundle a file into a node and use an appropriate module to fine tune permissions into the bundle. That's why I won't separate access for file itself from file details (meta+fields).

I have to agree here. Care removing , not for downloading files from t('For viewing file details, not for downloading files.'),

I will not touch this in this issue. The whole hook_permission() implementation needs some attention because:

• Permissions description lacks
• Per-filetype permissions need to be implemented (like for nodes).

A separate ticket should be opened for hook_permission() refactoring.

I noticed in #13 you say that we shouldn't separate download vs. view. So I guess I do that in the patch I posted. I completely understand where you are coming from and I'm trying to come up with a use case where the separation makes a difference. Even if we can't come up with a solid use case what's the downside of separating the two?

Tried to apply #29 against 7.x-dev, but faild. Updated it manually and created a new patch.

Updated patch.

Changes:

• Left out "cleanup" changes.
• Minor language changes to various comments.
• Used file_uri_to_object() over file_load_multiple().
• Returned file_get_content_headers() instead of a "static" array of headers.
• Used the "view" operation instead of the "download" operation for checking access in file_entity_file_download(). I believe that "download" is reserved for file/%fid/download.

system/files/my_file_path/my_file_name.jpg

i guess thats indeed download

Marked #1919722: Files cannot be accessed by admin with file system set to private as a duplicate.

The last submitted patch, 1420812-file_download-34.patch, failed testing.

I can confirm that this patch fixes the issue I have with private content not working with the current Media 2.x module. Thanks for the hard work, this has been a nasty critical bug with private files that has persisted for far too long. I'm tempted to mark it as RTBC but not sure I've tested for any other side effects. So I'll run with this patch for a week or so and then post back if all looks good. Hopefully other people can also test.

Committed to 7.x-2.x.

now that this is done, what should happen to http://drupal.org/node/1414990

A patch to simply return FILE_ENTITY_ACCESS_IGNORE.

Automatically closed -- issue fixed for 2 weeks with no activity.