Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Currently, file supports the access check by node_access, what is of course proper API. If you normaly want to extend the way the rights are granted, you use the node_access api to place rows in there.
But this solution has sensitive limits. Every row in there is "ORed", so if one of the node_access entries is granting rights, you cant denie it at any costs.
This behaviour is not optimal for every setup.
To get arround this, e.g. you can write your own "node_access" method, which uses node_access, but also maybe adds some special cases or whatever. Thats what i have done included providing a db_rewrite_qry and stuff.
Ok, what does it all have to do with File Framwork? Well to enable File Framework to let the user exchanged the method which is used, to check if the user has rights ( by default node_access ).
I used 2 triggers to implement it for file and file_attach. File defines a method, file_check_access which only checks, if the user currently implements the file_custom_access hook ( any module ). If its the case, it does not call node_access but calls all the hooks implemented.
If nothing implements such a hook ( default case for most of the users ) the default node_access check is done.
This way, no default behavior is changed for any user. It is just a new ability to exchange the call of node_access by a custom function, without hacking File Framework.
All direct calls to node_access have been replaced by file_check_access, which evaluates to node_access if no hook is implemented.
I implemented this hook in file_attach, which itself is using it, to export the same functionallity. So you can exchage the way file_attach checks the rights, what will affect file, as the hook is defined. If you dont`t implement the file_attach_custom_access, again, node_access is called and therefor, file_attach and file still are using node_access. The reason to implement it that way into file_attach is, that you will use the same access method for file and file_attache in 99% of all cases. Because it does not help you much, granting the access to the file(file_attach), while you cant view or download it ( file ).
Please review the patch and it would be nice, if this one would be merged with the file framework core.
the attached patch is agains 6.x-1.0-alpha5
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | patch_access_rights.patch | 9.08 KB | eugenmayer |
| patch_access_rights.patch | 10.35 KB | eugenmayer |
Comments
Comment #1
eugenmayer commentedSorry, it was the wrong patch ( outdated ).
Comment #2
miglius commentedI'm not sure what should be the behavior when several custom modules implements this access hook and one grants access while other denies.
Now your code denies access if at least one module returns FALSE (or permissions are ANDed in your words). Usual drupal access control works the other way - a user can have different permissions and if any of the permissions allows some action, a user gets the access.
Comment #3
eugenmayer commentedHello miglius,
thats an TODO in the code, i current use the method "no_false", which means, iam ANDing the rights. The drupal defaul policy is OR, as you stated.
As i wrote in the TODO, this should actually be a setting in the plugin OR something the hook returns like array( false, "AND")
In the last case, we need to have a proper parenthesis then.
But this is to implement, i dont think the current implementation is :
- wrong
- unsecure ( actually its more resctricting )
- hard to adjust ( change one single line.. )
What do you think about the patch in general?
Comment #4
miglius commentedIf some custom module implements the proposed hook, it completely takes over the access mechanism over the node_access().
That means that I might be able to open a file node page, see the file in the browser, etc, but cannot be able to view or download the file if a custom module decides not to allow me that, or is this the idea to make it more restrictive?
Comment #5
eugenmayer commentedActually i guess you need to once look into the patch an the comments..
2 hooks are provided, one hook is for file. This one you can use to exchange the access check by your own one, if you wish.
The second hook you talk about is actually a hook provided by file_attach, which itself is nothing more then an implementation of the file hook. It provides you the ability, to implement one single hook ( file_attach hook for access ) and this one will be passed to the file hook also, so you use exactly the same access checks for both, file and file_attach, which makes sense in 99% of all cases.
Take the second hook of file_attach is an concrete ( example ) implementation of how to properly use the file hook for customizing access checks.
In the end, if access should be checked by file, it calls the file_attach hook ( if the file_attach module is installed..else it uses node_access ). This file_attach hook looks for any modules, implementing the file_attach_access hook, if there is one, it uses it for itself AND passes it back to file, so file will use the same method. Else, if no hook implemented, node_access is used and again, passed back to file.
This is the way you keep rights in sync, file and all the modules you implement for file, like file_attach
Comment #6
miglius commentedI don't understand why access control has to be so strongly coupled. For example, if I open a file node page, your proposed access function file_check_access() would be triggered, which would invoke a hook file_attach_file_custom_access(), which would end up at file_attach_check_access() function, which on the other hand could implement some custom access rules for file attachments.
Now, I was opening just a file node page, which has nothing to do with file attachments. So why attachments access control should be applied?
Also there are file_browser, file_gallery, file_cck modules. If they all are refactored the same way as file_attach, it will be over complicated, as just looking at the file node would trigger all those access functions...
Comment #7
eugenmayer commentedHmm....
could we meet on IRC? Somehow i have the strong feeling, we miss eachother.
First off, i did not change the default behavior at ALL. So if i access a file node, it does not necessary check the rights defined by file_attach ( while this would make sense..as a file node in the idea of file-attach is a preview of the content .. a container )..which is used to get versioning and rights working..
But you dont need to do it that way, so if you take my code, without any further changes, you dont see any changes for your installation at all. It simply behaves like before.
It starts when you want to implement your own access methods, then you now actually have a chance to hook into file_attach OR file OR both, without hacking. This idea can be adopted to the other file plugins, if the authors like. so you can hook into all.
Cant see why we need so much argueing here :)
Comment #8
miglius commentedI agree that your code won't change the functionality if there is no other module implementing the hooks. However I see potential danger in the long run when there are one or especially more such modules.
Modules in the File Framework are loosely coupled, i.e., only the main file module is required, others are optional. Suppose one uses only the main file module and the access permissions on the file nodes are default. Now, if one enables the file_attach module and some other module implementing hook file_attach_custom_access(), the default access for the file node will change as the file_attach module with the custom implementation will take over the access control.
Now suppose there are a still another module which hooks into the file_gallery access permissions. Now the access control for the file node (which does not depend on the file_attach nor file_gallery modules) gets even more trickier since both modules changes the access control.
Comment #9
johanneshahn commentedNo activity