Dear all,
Some weeks ago I ask here http://drupal.org/node/20577 about OG and TA compatibility, because I was trying to use the both on my site. If I follow Moshe answer it appears that they are not, to say the least, very compatible ! Searchnig drupal.org about that shows that there are many requests about using together several modules implementing node_access api. And quite always, people are desapointed because they don't work well together and they need to choose between one or an other. This is obviously not a right way.
To come back, having TA and OG work together should be a great enhancement. It will give us control over whom can access node and whom can use this or this vocabulary to tag them witch will open doors to new applications.
As I don't know many things about php by now, I can't be useful in coding anything but I would be happy to make extensive testing.
Regards,
Eric

CommentFileSizeAuthor
#7 node-access-spec.txt8.96 KBrobb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Version: 4.6.0 »

One reason that this problem persists is because it is hard to fix. I talked with JonBob (the implementor of node permissions) and neither of us came up with a great solution.

The main problem to solve is that a node permission module often cares only about some posts and not others. In the case of organic groups, og only cares about the node types specified by the admin. The natural expectation of the admin is that other node types will behave the same as they always did before og was activated. In order to for og to make that happen, it has to grant view access to all nodes which it doesn't care about. If it didn't do that, those nodes would be inaccessible. Thats because the 'universal grant' which provides access must be deleted by any permission module.

So imagine that a taxonomy access control module is also activated. What if it wants to deny access to certain terms. It can't do so on certain nodes because og has unknowingly granetd access already. Node permissions are current only 'grant' based. There is no notion of 'deny'. Whats needed is a way for all node permissions modules to coordinate so that the right entries are written. Alternatively, we need to support both 'grant' and 'deny'. This gets very tricky because node_access can get very large very fast when you have lots of users or lots of nodes or lots of groups, etc. Node permissions get called very frequently so performance is crucial.

Hopefully someone will become interested in this and propose a good solution.

robb’s picture

I worked on this a few hours today, seemed like a good way to learn more about Drupal. You are correct that this is not an easy problem to solve. My best solution was to modify the WHERE clause generator in node.module, refactoring code along the way. But the bad news is it requires the use of SQL sub selects.

Some advantages:

* Would allow site-admins to select the way node permissions are checked. My idea was to create a default using OR (backward compatible) and let admins use a string template of the form 'module_name AND|OR module_name' where parens would be allowed. When a module with grant_access hooks was found that was NOT in the template the template would be modified to be '(......) OR (new-module). Of course this means site-admins would have to figure out the proper order but for many cases AND connectors would do the expected thing. I have a lot more details in my interface design notes that I can clean up later if needed.

* Code proto-type works

* I think the speed will be acceptable and in any case will be no slower for the current case of only one node_access module.

* Modules do not need to be changed. At least OG, TAC and NodePerm would seem to work base don a code review.

* Core appears to be unaffected EXCEPT node.module

Now for the bad news

* Requires sub-selects to be effective. That requires MySQL 4.1 or PostgreSQL.

* Administrators need to understand how to make the node_access template query. Not a huge problem as the default will work for normal cases and module writers can include

My concern is that there are too few Drupal users that have MySQL 4.1+ or PostgreSQL and that would make the module/patch useful to only a few. I switched Drupal from PostgreSQL to MySQL due to various incompatibilities with certain modules. This was an annoying concession to make as I vastly prefer PostgreSQL.

So my question is: Should I continue to work on this patch/module or is the target audience too small and the limitations of the patch too confusing?

bomarmonk’s picture

I'm stuck with mySQL 4.0.18, but I would love to be able to use organic groups with TAC! It would be a perfect solution.

Although another possibility (for my purposes) would be to use a taxonomy module that only effects what taxonomy terms are accessed (or selected) during node creation -- and not mess with the actual node access permissions (if I understand it, this would be possible?). This way I can make node creation user-friendly and specific to user roles. If I used this trimmed down version of a "taxonomy selection" module with OG, I would also get close to what I need to implement. Maybe a module called organic taxonomy?

I'm not sure, however, that this would answer the needs of all user who wish they could use OG and TAC together.

moshe weitzman’s picture

We simply cannot patch core to *require* subselects. It would be OK IMO if we performed a subselect on capabable DBMS and automatically performed 2 queries (using a temp table) for older DBMS. Your description of a 'template' sounds unlike anything in current drupal (and thus a bit suspicious), but i will reserve judgement until we see it.

Thanks for working on this issue.

BomaMark - that taxonomy_selection module is doable. It has to use the db_rewrite_sql() hook to exclude the unwanted terms.

robertDouglass’s picture

I too have great need for this. Could we see the code?

While the interface is (like Moshe said) suspicious, what you are doing at the query level is assigning a hierarchy of precedence for all node-level permissions modules, right?

If module A trumps moudle B and module A doesn't have a GRANT on a node but B does, what happens? Is A's lack of opinion on the matter taken as a DENY?

robb’s picture

OK, I will take the response as a YES.

I am still working on this solution, my early experiments were just that, experiments. I am still working through the entire node access parts of Drupal and how they all interrelate. So I may find something that blows this solution out of the water.

I agree that sub-selects should not be required. I was going to try and detect if the SQL DB had the proper capabilities and if not fall back to original code. In this case multiple node_access modules would work (fail) they way they do today.

I will post more details as I learn more. My time analyzing Drupal's code is all of 4 hours and PHP is quite new to me. I'm a Perl programmer and the transition from Object Oriented extended methods back to flat programming has been a bit frustrating.

robb’s picture

FileSize
8.96 KB

I have code working that I think does what is wanted. I am still playing with it both in terms of debugging and in just how useful it is.

Attached is my current progress notes. I am not prepared to release code publicly but those interested enough to try out ALPHA code are welcome to contact me.

Thank you for your comments and suggestions. If all goes well in the next few days, and there is interest, I will have some BETA code ready to try out. Those wishing to try my ALPHA code may contact me privately. The responses I have had so far have been encouraging.

ec’s picture

Hy robb many thanks for working on this, I think it could a great add-on for drupal to have it works. I would be happy to try your code on a test site and send you some feedbacks as a user and not a coder as I don't know (helas) many thinks about php. You don't have enable your contact form so I can't send you an email but you can use mine. regards, eric.

moshe weitzman’s picture

very nice writeup.

on the one hand, it is great that you are working hard to make a patch which allows this in 4.6 Drupal. thanks.

ultimately though, i would prefer you look at the node_access system itself and change the API to make this cleaner. None of the existing code is sacred - we could change the notion of a grant, the structure of node_access table, etc. i'm not sure if you are hoping that this patch be accepted into core. if so, i think you will be disappointed.

use http://drupaldocs.org/hook_settings to make a module settings page

robb’s picture

I considered adding API hooks but felt that some real-world experiments were needed to see just what those hooks should do. Now that I have tried it I like it!

It's good to know that changes to the existing API can be considered. Altering the API may improve the speed issue but I am not sure it would alter the idea that only a site-admin knows the proper way to cascade security modules together for their site.

I of course would like to see my contribution included in the core. I am a bit unsure what it is you mean by "this", what specifically are you unhappy with in the API?

I would prefer you look at the node_access system itself and change the API to make this cleaner

I have been through the entire node_access section of node.module. Most, if not all, the functions had to be modified to support the functionality I am working on. In essence this is less of a module than a rewrite of the node_access section of node.module. Of course I had to examine OG and TAC as well to learn what they were doing with the API.

The current API has, in my mind, a couple of limitations:

  • Inability to override lower level permissions, such as 'edit my own' types
  • Cascading permissions is not controllable, but my proposal resolves that
  • Inability to remove a permission granted by another module. My proposal allows a degree of control over this by carefully crafted templates
  • SLOW when using cascade control. I will see if changing the current API would help this.
  • Callers to node_access expect a where/join clause back but do not provide any filter information. Adding this as an option may improve the speed when cascading permissions. For example my changes assume ALL (for complex templates) nids are required since it cannot assume anything else.<.li>

The node_access changes do enhance the current usage of the API to some degree but the API itself is unchanged:

  • Modules no longer need, or should, alter the 'all' realm
  • New methods allow for more control over SQL fragments. The 'join', 'from' and 'where' components of access control are now available. The 'from' and 'join' are mutually exclusive fragments where 'from' is used when no inter-table links are being done.

Any guidance or suggestions you have that would make this module useful enough to be included in the core would be appreciated. I have worked on this many hours so far and would not mind adding more effort to make it globally useful.

robb’s picture

ec,

I'm afraid that in the current ALPHA stage some editing of PHP is required to define the templates. Once I get the user interface elements up I will let you know. Oh, and I will turn on contact permission.

robb’s picture

I am nearing release of this module in an experimental form. The idea is to give people a chance to try it and see if its basic concepts are valid. I am also, per a previous suggestion, working on a new/extended API for node access. This is in the preliminary stages only. As I become more familiar with node access processing and the various modules that extend I am feeling more confident it proposing some changes. BUT isn't there an author for this module? I would hate to be duplicating their work or worse stepping on someone's toes. What started off as an internal experimentation project for my first experience with Drupal is starting to get a bit out of hand!

Some questions since I am very new to Drupal's community:

1. Where should I post this patch? It will be a patch file, a new module file and a small SQL script all in ALPHA status.

2. What features would you like to see or don't like in the current hook_nodes_grant and friends?

node_access module status:

* Speed for OR (simple) queries has not changed and is the same as in the current Drupal.

* Very simple UI developed with help page (I am having problems getting the [more help] link to work properly on a settings page so it may go out the door a bit broken.)

* Bugs squashed, more are to be found I am sure

* SQL server side memory improvements. Less tables potentially created and the ones that are created are smaller

* Speed improvements, about 10-20% on my tests but see next item

* Potential for much greater improvements on less contrived test caases due to tying temporary tables to unique grant permissions and reusing the tables when grants are identical.

* I am starting to get the hang of PHP. But still find myself falling back to Perl strategies (foreach COPIES values, AGGGG!!!). The code probably suffers from some over simplified coding constructs. But at least it is easier to maintain. If I could become fluent in Perl, SmallTalk and Forth I am sure I will eventually "get" PHP.

robb’s picture

ALPHA code posted to: http://drupal.org/node/24868

ec’s picture

The new beta module is now available here http://drupal.org/node/24868. It's a great job and works fine.

moshe weitzman’s picture

Status: Active » Closed (fixed)

FYI, the na_arbitrator module now does this in a sane way. og will soon be converted to its way. see http://drupal.org/node/55594. closing this issue for now.

robb’s picture

Title: Better node_access API so, for example, OG work with TA » node access available for 4.7

I disagree that na_arbitrator replaces node_access. I kind of wish it did since converting node_access to 4.7 was a bit of a pain!

na_arbitrator appears to provide a nicer access to maintaining the node_access table(s). It also supports node types having their own access tables and using those as needed. But it does NOT allow multiple node access modules to work together in a user defined manner. I am still sorting through the code as there is not a written API gude yet.

Node_access has been updated for Drupal 4.7 and renamed to na_multi. It has had limited testing but is working for others. It can be found at:

http://www.lab.canfield.com/projects/na_multi

If na_arbitrator does do what na_multi does and I missed it I would love to know how. Combining my efforts with another's is always preferred. In any case I am examining if na_multi can be built on top of na_arbitrator. If possible it would save me the effort of designing a new API (saner interface).

robb’s picture

Title: node access available for 4.7 » Better node_access API so, for example, OG work with TA

Sorry, I forgot that changign titles changed the master title. Too many forums, written in too many different software packages. Restoring title to original.

Zjooji’s picture

Hey ^^. First of all sorry if i'm not supposed to post here but I'm new to this whole forum thing.

I'm fairly new to drupal and have been looking into into node_access a bit to get my site up and running and I found a way (which seems) to integrate both "allow" and "deny" priveleges into drupal while still allowing for backwards compatability and seemingly not affecting the effiecency of the drupal access system. (I say seemingly as I have not tested it's effiiecency but if you look at the code you'll see the change is really relatively minor)

All i did was change the node_access table by increasing the size of each of the the grant_* columns by 1 (to avoid any issues on the next change) and set the columns to signed ints.

Then from there i basically used the idea that a negative value in the grant_* is considered a deny 0 values are ignored and positive grant_* are considered as allows.

For my purposes all I wanted was that deny entries take precedence over allow entries so I merely changed the following code in the node_access function to the code below. (Left the old lines in there but commented out)

    //From here on out commented out the old access method and added in the new...
    //$sql = "SELECT COUNT(*) FROM {node_access} WHERE (nid = 0 OR nid = %d) $grants_sql AND grant_$op >= 1";
    
    // Gives precedence to deny entries over allows, if no deny exists the first allow is taken
    $sql = "SELECT MIN(grant_$op) AS access FROM {node_access} WHERE (nid = 0 OR nid = %d) $grants_sql AND grant_$op <> 0";
    
    //$result = db_query($sql, $node->nid);
    
    $result = db_fetch_object(db_query($sql, $node->nid));
    if (empty($result)) {
      //Defaults to deny....
      return FALSE;
    }
    return ($result->access > 0) ? TRUE : FALSE;
      
    //return (db_result($result));

I think you could quite easily change this sort of approach to work with a sort of priority structure as well instead of simply deny taking precedence over allow. Basically instead of selecting just the minimum you could select both the min and max and compare the weights (their absolute value possibly?) of each anf whichever had the heighest weight would decide the access rights.

While I know this (probably) won't help the current node_access systems in working together but the ability to both allow and deny should (I hope) address the underlying problem of having to insert a universal grant for nodes not dealt with by your permission system.

Anyway I hope this helps at least in some small way, or at least you can point out something I've obviously overlooked.

Cheers.

Zjooji’s picture

Looking over the original post again I think i may have to apologise as I think I may have misunderstood the problem of effiecency described by moshe. I thought originally thought he was stating that it would be ineffiecient to support both deny and allow in terms of the code to allow for both but now understand he meant in terms of the potential number of entries in the node_access table. I can see how having a way for modules to work together to decide the entries in the node_table would help to keep entries in proportion to the number of nodes that exist as opposed to the number of nodes * number of permission modules they have installed.

The only claims that I can make in regards to the potentially increasing number of node_access entries would be that by supporting both allow and deny permission modules that can forsee (or adjust to ) their own use would be able to reduce the entries in the node_access table by choosing the appropriate technique (allow or deny).

However the more I look at it, the less it seems to be a stand alone solution. Perhaps this method in tandem with some of the other methods you mentioned would work. Or perhaps just throw it to the scrap heap ^^, anyway at least it works for my needs and I can hope others may find it useful.

Sorry for any inconvenience and keep on truckin' ;)