Asset and Drupal's "Private Download method"
| Project: | Asset |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | wmostrey |
| Status: | needs review |
Problem Description:
Once a node with an asset is created, clicking on the asset link in the node results in a Page not Found error (for any role, including Admin!). On top of that any auth. user is allowed to attach Private Assets, for which their role has no access permissions, to nodes (via Asset Wizard, when creating Content).
To Reproduce
1. Login as admin and enable Private Download method in admin/settings/file-system. In the System File Path field enter a directory not accessible from the browser, such as for example ../../files . You might need to actually create this directory, can't remember if Drupal does this for you...
2. Follow 'reproduce' steps in http://drupal.org/node/239885, up to and including "Allow regular ..."
3. Enable 'access asset wizard' and 'browse assets' permissions for regular users
4. Go to Create Assets and create one Public File and one Private File (do not check the authenticated users role check box at the bottom)
5. Logout and login as a regular user.
6. Create Page, then click on the Asset Wizard, then on Browse. You will see both files (even though one is Private and should not be accessible to any role aside from admin). Both assets can now be embedded into the Page (which is wrong!). Embed them both.
7. Save the page. Now view the page and click on the assets links. This will result in Page not Found error.
8. Logout and login as Admin and view the page and click on the assets links. This will ALSO result in a Page not Found error.
P.S. There's two issues here jammed into one (unauthorized users allowed to create nodes with Private assets and embedded assets links being broken). I can create a separate issue for the first one? Would Roger López or someone else who is a higher authority on bug-reporting please comment if this is desirable.
#1
To be sure: are you using the latest dev or beta-1 version? The private download method is supported since 2 weeks now.
#2
Alexander, please make sure you tag the right versions for these tickets.
#3
yes, sorry, my bad. I'll try and be more careful in the future.
I think I got them mixed up because I started with the beta version but had troubles with it, followed this thread http://drupal.org/node/233968#comment-785994 and switched to the latest dev version and got confused.
#4
There are definitely a couple of things at play here.
The page not found messages are definitely something I will need to look into. I have tried my best to use Drupal's built-in file handling so that both public and private download methods would work, but obviously i missed something.
The second issue has to do with merging file download permissions and asset usage permissions. I am opening a separate issue (#243047) for this.
#5
Asset 5.x-2 has been discontinued, please use 5.x-1.
#6
Has anyone been successful using Asset with the Private Download method? I have done some basic tests which have failed so far, but the results are inconsistent so I'm not sure yet whether it's because of my particular setup.
#7
I'm using the private download method on a couple of sites and have no issues with it. Could you provide more information as to what exactly fails? For instance does it fail for assets that existed before you switch to private downloads, does it fail for new uploads, ...
#8
Same issue here. Going to debug it later. Subscribing for now.
#9
I have 3 people reporting various issues with the private download method, I'm looking into it. It would be great if everyone could post a step-by-step guide on what's going wrong.
#10
Here is what I found.
The /system/files/filename --> invokes the hook_file_download. If you enable the Upload module that gets us part of the way there. But the nid in the {files} table for the "uploaded" file is set to zero. As a result the upload_file_download does think you have access to the file since you don't have access to node.nid=0. Further it puts a second / in the path if you don't upload to a sub directory. That needs to be fixed too.
db_query("INSERT INTO {files} (fid, nid, filename, filepath, filemime, filesize) VALUES (%d, %d, '%s', '%s', '%s', %d)", $fid, 0, $info['basename'], file_directory_path() ."/". $info['dirname'] ."/". $info['basename'], $mime, $asset->filesize);
See the "/" . $info['dirname'] . "/" . If that variable is empty you get // in the files table filepath column for the upload file.
I also noticed there are asset roles and public and private settings. Not sure if this is too be implemented or what. Bottom line I would offer a patch but I am not sure which way you are going to take it.
So here is my take and guess:
* I don't see how adding to files table is useful. Usually that file is associated with a node and the node_access controls access to the file. Perhaps there are other modules that some how extend this I dont know. But if we do continue to use it, we should fix the // problem.
* The next thing we should do is implement the asset_file_download. This will control who as access and when. I am not sure how it will interact with the upload_file_download. I think if there is any restriction it fails. So it might not interact with upload module properly.
#11
I found the culprit.
http://drupal.org/node/167601
With nid of zero, I am not sure if that make node_access return true or not. It wasn't working for me, so I assume it doesn't.
#12
I see what is happening. You need to load a node to do node_access. Well node_load on nid 0 returns false. So it doesn't work.
I see there is an interface for seting private and public. But it doesn't see to be used anyway where in code, other than during save. I don't see any role association or permissions for private. I am assuming that isn't finished yet.
At this point, I would suggest not putting anything in files. And just make your own hook_file_download. or implement your own all together like webfm (though I think that has pros and cons too)
#13
I fixed the double slashes in http://drupal.org/cvs?commit=145874. I'm a bit confused about the nid issue. Assets aren't linked to nodes, you can upload assets without ever creating a node and you can enter them in textareas in a user profile or log fields for instance.
What exactly is the consequence of having nid 0? Are there only issues when using asset in combination with the upload module so that the files don't appear in the attachment list? How does adding a hook_file_download() fix this if there still would be no nid for instance? The upload_file_download() would return FALSE on the $file but asset_file_download() would return TRUE.
#14
Christian, I've done some more research and with the help of Drewish and your comment #10 it looks like I've found a way out.
The only reason I'm using the files table is because I thought that was the best way to support the private download method. Now it looks like we don't need any entry in the files table at all, we only need to implement hook_file_download(). Since asset itself isn't doing any checks currently, it would always return TRUE.
And as for my final question:
[01:21] wmostrey: drewish: so if one module's hook_file_download returns FALSE and the asset_file_download would return TRUE, would the user have access to that $file or not?[01:21] drewish: wmostrey: exactly
I'll implement this on dev very soon!
#15
I removed the manual {files} inserts and implemented hook_file_download in http://drupal.org/cvs?commit=149378. Can everyone check if this now works as expected?
#16
Marking as fixed for now.
#17
Automatically closed -- issue fixed for two weeks with no activity.
#18
As for your fix of the double slashes, it doesn't work for me. $info['dirname'] is empty when I'm in the root. So I added as a condition for prefixing it with a slash that it may not be empty (see attached file).
#19
And here's a .patch file for that.