Hi there,

I've added a file field to a product type to store a PDF, the file field is 'private' mode.

The problem is, the files upload ok, but return the "Access Denied" page when clicking/downloading.

I'm testing this as both root/uid 1 and as an administrator with all permissions.

Is there something else that needs to be done to allow private file fields on product types?

Thoughts appreciated,

DT

CommentFileSizeAuthor
#5 commerce_product.module.v2.patch1.51 KBdavidwhthomas
#3 commerce_product.module.patch1.58 KBdavidwhthomas

Comments

davidwhthomas’s picture

davidwhthomas commented

Just a guess, but perhaps the commerce product module needs to implement hook_file_download?

As the product is a different entity from a node, it might need to explicitly define access via this hook.

DT

davidwhthomas’s picture

davidwhthomas commented

Update: After running some tests, it looks like the hook to implement is hook_file_download_access

I made a test implementation that returns TRUE and the file downloaded OK.

I'll work on a patch after the weekend when back at work that does any required access checking for the product entity field there.

DT

davidwhthomas’s picture

davidwhthomas commented
Status: Active » Needs review
StatusFileSize
new commerce_product.module.patch1.58 KB

Heh, couldn't resist, patch attached.

The patch implements hook_file_download_access

It first checks user_access to view products.

It then checks field access to view the file field.

If both checks are satisfied, download is allowed, otherwise download fails.

Please review and commit if possible.

Cheers,

DT

davidwhthomas’s picture

davidwhthomas commented

Update:

Just checking the 'access product' permission shows that it is associated with permission to view and reference products in admin and via product_reference fields.

It might therefore be sufficient to just do the field_access check, without the commerce_product_access check wrapper.

Please confirm if that's the case and I can re-roll

Otherwise, anon users will need 'access products' permission to download private files on the product.

An alternative might be a new file-specific permission 'download product files' which could be checked instead.

davidwhthomas’s picture

davidwhthomas commented
StatusFileSize
new commerce_product.module.v2.patch1.51 KB

Alternative patch without the 'access products' check attached.
DT

rszrama’s picture

rszrama commented
Status: Needs review » Closed (duplicate)

Ahh, you know, it looks like you posted your issue around the same time recrit posted his, and we didn't end up seeing your patches before an approach was taken in #1155682: support for private images that imitates the core accommodation for this issue. The solution comes straight from the node module implementation, which at least for now does not depend on field level access.

davidwhthomas’s picture

davidwhthomas commented

I'd prefer the field level access check as there are cases where a user may view a product, but not download an attached file and such a field check is perfect in that function context, parameters included.

Also, the additional hook documentation / comments would be good practice as the other patch neglects this.

Up to you though,

DT

davidwhthomas’s picture

davidwhthomas commented
Status: Closed (duplicate) » Needs review
davidwhthomas’s picture

davidwhthomas commented

The current patch from http://drupal.org/node/1155682#comment-4576950 fails on existing installs because the entity name has changed from product to commerce_product.
I prefer the patch on http://drupal.org/node/1157266#comment-4468168 which doesn't have this limitation

rszrama’s picture

rszrama commented
Version: 7.x-1.0-beta3 » 7.x-1.x-dev
Status: Needs review » Fixed

Researching this further, the other issue didn't actually seem to resolve the problem for me. This is because my anonymous user did not have access to view products... but we know in reality he should since he could view the node and order the product. The problem is in the hook_file_download_access(), we don't know the node context.

Regardless, I like the approach of this patch and the comments, but it is substantively different. With this patch, a user will have access to download any file attached to a product unless some module implements hook_field_access() and explicitly returns FALSE. In other words, this issues a blanket grant, assuming the user knows the URL of the file. I still think this is preferable than forcing people to rely on product view permissions, though, because realistically those permissions aren't being used to determine a customer's ability to find / browse products attached to nodes.

Committing patch in #5 with a small change: I'm keeping the check on the entity type that the other patch provided. No reason to give blanket access to any entity type's file fields from the Product module.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/5b7c8b4

davidwhthomas’s picture

davidwhthomas commented

Thanks! Looks good.

Thanks again for the awesome work on the commerce module suite - great stuff.

DT

Status: Fixed » Closed (fixed)

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