I'm working in a system in which all files are uploaded using the private streamwrapper. Thanks to the 'view private files' permission, I thought I was good to go. However, while my users were able to edit files (via 'file/%fid/edit' path), I was perplexed that they kept getting access denied for viewing the file, especially since file_entity_access granted those users access with $op = 'view'. With the help of devincarlson in IRC, and reading through too much code, I eventually discovered the little todo at the beginning of file_view_page in file_entity.pages.inc, which explicitly checks for private files. I simply removed this code, and things worked as I expected based on the configuration. Since this code hasn't been updated since 2011, and since there are now permissions for the different file types and operations, would it make sense to permanently remove this?

Patch:

diff --git a/file_entity.pages.inc b/file_entity.pages.inc
index 76ffd91..2293791 100644
--- a/file_entity.pages.inc
+++ b/file_entity.pages.inc
@@ -9,17 +9,6 @@
  * Menu callback; view a single file entity.
  */
 function file_entity_view_page($file) {
-  // @todo Implement granular editorial access: http://drupal.org/node/696970.
-  //   In the meantime, protect information about private files from being
-  //   discovered by unprivileged users. File IDs are autoincrement, so one can
-  //   attempt discovery by trying to access different media/ID paths. See also
-  //   media_browser_list(). This logic potentially belongs within
-  //   media_access(), but that would require extending that function's
-  //   signature to accept a $file paramter, and this is temporary code anyway.
-  if (!user_access('administer files') && (file_uri_scheme($file->uri) === 'private')) {
-    return MENU_ACCESS_DENIED;
-  }
-
   drupal_set_title($file->filename);
 
   $uri = entity_uri('file', $file);
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Devin Carlson’s picture

Status: Active » Needs review
FileSize
1000 bytes

Removing the code sounds good to me.

Devin Carlson’s picture

It looks like #1973028: cannot download file on node page is a duplicate.

Devin Carlson’s picture

Status: Needs review » Fixed

Committed #1 to File entity 7.x-2.x.

Status: Fixed » Closed (fixed)

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