There seems to be an issue with downloading files as a someone with administrative role from user's (customer's) File downloads tab.

Here is what happens:

  • common user comes to the website, buys a file; file is being added to user's File downloads tab in profile
  • as user who bought the file, click the file and download should be offered
  • now login as superuser 1 (admin) and go to the customer's user profile (the one that bought file few steps above)
  • go to user's File downloads tab and click the file
  • instead of download you will be thrown to "Access denied" page

Why?
Here is a reason:

  • when you click on file function _uc_file_download in uc_file.pages.inc (line 187) triggers
  • on line 203 in mentioned function there is this line which is problematic:
    $file_download = uc_file_get_by_uid($user->uid, $fid);
  • function uc_file_get_by_uid is defined in uc_file.module on line 1557
  • it checks through query user and file and builds cache array which looks something like $cache[$uid][$fid], for example $cache[1][453] = false;
  • so it returns as a result in this array FALSE instead data needed for user to init download
  • after this function we return to _uc_file_download and there we check what was returned. This is a critical line that makes an issue:
    line 205: if (isset($file_download->filename)) {

Because $file_download contains content of $cache array from uc_file_get_by_uid() it wont have filename as its only returned false so execution jumps to else which says:
return MENU_ACCESS_DENIED;
which results in showing everyone (superuser 1 included) access denied error. No permission can help here, and I believe this is a bug in code logic. The reason I believe so is that the same feature works perfectly fine on D6. There superuser or any role with correct permission can download other user's (customer's) file with no problem.

I googled a lot this issue and couldn't find any hints, patches or topics about this issue, so pardon if I missed some already existing topic.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

milovan’s picture

So on D6 we used uc_file_get_by_key, which is also present in D7 but in D7 is ussed for file check uid instead of file key in _uc_file_download? Why this change?

How about patching uc_file_get_by_uid to check if user has a permission "Download file", or even make a new permission like "Administer" which would allow such roles to download user's files? Site administrators should be able to do it as well as store administrator roles and other part of authorized site staff.

I am not really sure why those regressions appear in D7 but I am more than willing to help fix it. I d even role some patch but I d need answers why the mechanism has changed so I can think in the right direction.

longwave’s picture

Version: 7.x-3.4 » 7.x-3.x-dev
Status: Active » Needs review
FileSize
1.01 KB

The attached patch should bypass the check for users with the "view all downloads" permission, which is the same permission that lets admins view other users' file listings.

longwave’s picture

Fixed a typo in #2

milovan’s picture

I tested a patch and confirm it works. It allowed me as administrator to download file from user's "File downloads" tab.
Tested also with user that has rights to access other user's files and it works.
Tested with user that has no access to other user's files and he gets unauthorized access (which is correct behaviour).
Also tried to let that user access download link for a file but it cant access file.

So I think this patch works correctly in a couple of cases I tested.

longwave’s picture

Status: Needs review » Fixed

Committed #3.

Status: Fixed » Closed (fixed)

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