I have tested this module. Its working pretty good.

I'm attaching a small patch to restrict downloads if users cannot see the field where is stored the private file.

When I tested this module, I was expecting that, if I cannot see the file (as a field), I cannot download it.

This behaviour may be useful if different roles have access to the content but only a few users are allowed to download the files.

Comments

alan d.’s picture

This looks fine but is it needed?

I would have thought that the first call to 'file_download' would have triggered content permissions checks, and if not, then there may be an access error here. I have no local testing environment at the moment to follow this up.

Eg: The code that I'm taking about is here, about line 240 in filefield_private.module:

<?php

  if (file_exists($filepath)) {
    $headers = module_invoke_all('file_download', $filepath);
    $headers = array_merge($headers, module_invoke_all('file_private_download', $filepath));
    if (in_array(-1, $headers)) {
      return drupal_access_denied();
    }
    if (count($headers)) {
?>
dagmar’s picture

Category: feature » bug
Status: Needs review » Fixed
StatusFileSize
new1.57 KB

I have committed a different patch for this bug.

The problem was that when filefield_file_download() is called, $conf doesn't have the correct directory to download the fields assigned yet.

So, when file_check_directory is called the file is not found and filefield_file_download() doesn't returns -1 because filefield thinks that this file is not in the {files} table.

Fixed in: http://drupal.org/cvs?commit=325804

alan d.’s picture

Nice catch. This should be released asap, is it worth pushing through your other patch along with this one?

Status: Fixed » Closed (fixed)

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