Passing grants to referred nodes

This module is already confusing enough to configure without adding this ability, so I am reluctant to include it without thinking it through and perhaps trying to find a way to make it easier for the user setting up the fields to understand exactly what they're doing without being overwhelmed with options.

If you understand node access you will know there are two main components in a node access module:
- hook_node_access_grants: The hook that checks which grants the user has upon accessing a page.
- hook_node_access_records: The hook that check which grants a node has upon saving the node or rebuilding node access.

If both the user and node have a common grant then the access for that user to that node is given. Grants have an ID value and exist within a particular realm, and that is how they are defined.

If you understand the Node reference module, part of the References project, which I will use as an example there are two things to know:
- Given a node, you can dig down through the node object and find the node_reference fields and get the value out of them for which nodes it references. In this module this happens in hook_node_access_records.
- Given a node, you can perform a database query to find out which nodes reference the given node. In this module this happens in a helper function called during the hook_node_access_grants calculation.

All you'd have to do is reverse those.

So it might take a few minutes to do that.

Then hours and hours of testing, and a few years of people bitching at you for making the module too hard or inadvertently breaking something. In the meantime some genius will come along with yet another way to link nodes together and want you to add code for that too.

The other issue for me is that this module is kind of a spin-off of nodeaccess_userreference, which is a much simpler module. I tend to only add features to this module if they also make sense in that module. Just to keep things a bit consistent. If there was a more generic 'entity access' system, which I believe may come in Drupal 8, this module could really be generalised to be a lot more flexible. Especially by feeding off modules like 'relation' which allow creation of bi-directional relationships between entities.

An easier, and probably better performing, workaround would be to create a module which simply adds a reciprocal node reference when a node is saved.

I.e. create a field "reverse_reference" which is not editable by the user editing the node, using the nodereference module, in hook_nodeapi upon 'save', check the original reference field, get all reference nid values... then programatically add the reverse reference to the referenced nodes.

yeah I'm not up for working on this, maybe if more people request it or propose patches. I no longer believe the "node reference" module has a future in Drupal, as there are other methods being actively developed with more potential. By the maintainer's own admission, node reference will soon be deprecated.
Also I don't really like working with Drupal 6 anymore, it has totally plateaued in the last few months. Waste of time adding new features to it or to sites running it.

Any chance of providing this feature in D7 given that the deprecation of node_reference is partly due to the creation of entityreference?

I'm trying to build a structure where I have content types "Problem" and "Solution" and Solutions can be assigned to users.

A Problem can have many Solutions but a Solution can only resolve one Problem, so when adding a Solution, it must reference the Problem.

When a Solution is assigned to a user, I'm using nodeaccess_userreference to grant the referenced user "view" permission on the Solution. The referenced user obviously needs to view the Problem as well as the Solution, so their view permission on the Solution should propogate to the Problem.

Currently, to give the user access to the Problem, I would have to put a reference to all the Solutions on the Problem node, and edit the Problem node every time a new Solution was added. Perhaps I could automate that somehow, but it's a bit ugly. I think it would make much more sense for nodeaccess_nodereference to handle propogation in both directions.

Forgot to change status. See previous.

I suppose, but I'd like to try and assess the scope of something like this first before diving in.

Fair enough.

I'd like to be able to offer a patch, but I don't really know enough about Drupal's grants system yet. If I get some spare time I'll see if I can come up with anything useful.

This isn't a particularly ideal module to learn the node access system with either.

Just making some notes

We'll have to write a function to do the opposite of nodeaccess_nodereference_get_referenced(). Rather than getting the referenced nids of a nid, we want the referring nids of a nid. How exactly this will be used I'm not too clear about.

The trick is that upon updating (and we'll have to meditate on what 'update' actually means) a referencing node, we also have to update the referenced nodes, which we can of course use the existing nodeaccess_nodereference_get_referenced() to get - and then perhaps any nodes that they reference and so on. We probably shouldn't do that all in one hit in case it is a big operation. A problem just like this was already solved in Node Access Product using Node Access Rebuild Bonus, so a similar thing can be done here.

The big question is how we handle the hook_node_grants stuff, how we store settings so that they don't intefere with current functionality, whether we can alter the clusterfuck that is nodeaccess_nodereference_calculate_grants() or create a second pile of crap to do the opposite of that function.

Yesterday I was thinking about this in terms of "workflow" for calculating stuff and came up with the following:

1. Check if any nodes reference current node.
2. Check content types of referencing nodes.
3. Check nodeaccess_nodereference propogation settings for those content types.
4. If any referring content types are configured to propogate permissions, check user's access to the referring nodes.
5. If user has access to a referring node, propogate access to current node as dictated by the settings in the referring content type.

We'll have to write a function to do the opposite of nodeaccess_nodereference_get_referenced(). Rather than getting the referenced nids of a nid, we want the referring nids of a nid.

I came to exactly the same conclusion and that was the first thing I started trying to code. So far, I'm querying the field_config table for the field_names of all entityreference fields with a target_type of "node" (I'm unserializing the "data" column to find out the target_type). Once I have an array of field_names I'm trying to build a union select query of the target_ids entity_ids (and bundles) which reference the current nid in the appropriate field_data tables.

One potential problem I thought of was if a user configured an entityreference field to both inherit permissions from, and propogate permissions to the referenced node. Would it be sensible to restrict the field settings to either inherit or propogate but not both?

Edit: Minor correction where I mentioned the union select query.

Check if any nodes reference current node.
Check content types of referencing nodes.
Check nodeaccess_nodereference propogation settings for those content types.
If any referring content types are configured to propogate permissions, check user's access to the referring nodes.
If user has access to a referring node, propogate access to current node as dictated by the settings in the referring content type.

It isn't a straightforward as that. When we set the access configuration on a node, we have no information about the user. When we grant access to a user we don't have information about what node they are trying to view. Yeah... I know...

I came to exactly the same conclusion and that was the first thing I started trying to code. So far, I'm querying the field_config table for the field_names of all entityreference fields with a target_type of "node" (I'm unserializing the "data" column to find out the target_type). Once I have an array of field_names I'm trying to build a union select query of the target_ids (and bundles) which reference the current nid in the appropriate field_data tables.

That sounds pretty complicated, I imagine it would be rather simpler than that. Not sure though.

One potential problem I thought of was if a user configured an entityreference field to both inherit permissions from, and propogate permissions to the referenced node. Would it be sensible to restrict the field settings to either inherit or propogate but not both?

Hmm! Good point, I was hoping the user would just be smart enough to configure it to make sense, but that almost never works out.

It isn't a straightforward as that. When we set the access configuration on a node, we have no information about the user. When we grant access to a user we don't have information about what node they are trying to view. Yeah... I know...

Oh right. I was kinda hoping node_access() could be used to check the user's access to referencing nodes when the user tries to view the referenced node.

That sounds pretty complicated, I imagine it would be rather simpler than that. Not sure though.

I started with that SQL approach because entityreference doesn't appear to provide any kind of get_referencing_entities() function. As far as I can see, what makes it complicated is that a) a reference could come from any entityreference field so we need to check all of them, and b) entityreference can reference users as well as nodes so we need to ignore fields targetting users.

To my mind, the way to get the referencing nids of an nid, is to find which fields could reference it, then check which of those fields actually do reference it, and subsequently extract the referencing nids. I could be heading way off track though.

Edit: For reference, this is where I got with retrieving nids:

/**
 * Gets nids and bundles of nodes which reference a node.
 */
function nodeaccess_nodereference_get_references($nid) {
  // Get names and field data of all entityreference fields
  $sql = "SELECT field_name, data FROM {field_config} WHERE type = 'entityreference'";
  $result = db_query($sql);

  foreach ($result as $record) {
    // Get target_type of field, i.e. node, user, file
    $data = unserialize($record->data);
    $target_type = $data['settings']['target_type'];
    // Add to array of field names if field references nodes
    if ($target_type == 'node') {
      $field_names[] = $record->field_name;
    }
  }
  
  // If any fields reference nodes, build a query on their
  // data tables to find any references to our nid
  if ($field_name = array_shift($field_names)) {
    $sql = "SELECT entity_id AS nid, bundle, '" . $field_name . "' AS field_name " .
           "FROM {field_data_" . $field_name . "} " .
           "WHERE " . $field_name . "_target_id = :nid";

    foreach ($field_names as $key => $field_name) {
      $sql .= " UNION ALL " .
              "SELECT entity_id, bundle, '" . $field_name . "' AS field_name " .
              "FROM {field_data_" . $field_name . "} " .
              "WHERE " . $field_name . "_target_id = :nid";
    }
  }

  $args = array(
    ':nid' => $nid,
  );
  $result = db_query($sql, $args);

  return $result->fetchAll();
}

I did a bit more work on this and I seem to have some working code. Whether or not I've done it the right way is another matter.

I used the function I posted above, in an implementation of hook_node_access() which itself calls node_access() on referencing nodes where propogation settings are defined. This seems to take care of multiple levels of propogation quite nicely.

I didn't include a published/unpublished setting yet since I couldn't quite wrap my head around how that should behave.

Anyway, please take a look at this patch (and rip it to pieces if necessary).

Ok, first problem: Circular references.

If someone sets up two nodes to reference and propogate to each other, the node_access() checks will be circular. Doing this just killed my apache server. I can't think why anyone would want to do such a thing but it should probably be prevented in the code somehow.

I haven't had a look through that stuff yet, got too many other drupal ideas cooking up in my head atm. Pretty sure I had to solve a circular references problem in this module already though, it wasn't fun. I think a static variable is tracked in the function to make sure a call to node_access() doesn't trigger the exact same call to node_access(), which would cause an endless loop.

I added some code to stop circular references crashing the server. It basically records node_access() calls in a static array and if child invocations of hook_node_access() detect duplicate calls, they just reuse the previous result instead of executing a new call.

I also fixed the logic a bit since it wasn't checking update/delete propogation settings if propogation was enabled on view operations.

Forgot to roll the patch against the latest git code. Here's the correct patch.

Edit: I fail at creating patches. Use the next one instead of this one.

Sigh. I didn't include all the code in that one. Third time lucky!

http://en.wiktionary.org/wiki/propogate

I'm not a fan of anything that uses hook_node_access() to do view ops. It doesn't work for queries such as lists of nodes, only direct calls to node_access() like loading a node page.

Haha! Well that's me corrected!

Yes, I read about the differences between hook_node_access() and hook_node_grants() with respect to lists. I just didn't think I was up to the task of modifying your grants code, which looks pretty complex. I'll have another look if I have time, but I wanted to post what I'd done already in case it was helpful.

It is, thanks, I think I can combine it with the hook_node_grants technique.

I kinda half remade your patch then my eyes glossed over and now I just want to sleep
will have to wait till I have a free saturday night and sit down with a bottle of jim beam and smash this out, I don't see it happening any other way

With some of the open issues at the moment I shudder to think the extra overhead this will put in upon page loads :/

Yeah, I guess it could get pretty messy.

No hurry though. That hook_node_access() code is enough for my current project since I don't need my views output to respect node access and I don't have thousands of nodes to process.

Nobody else seems interested enough to post here anyway.

I'm sure we could work it out, even just have another variable_set thing that says whether 'referenced', or 'propagate', or both are in effect.

I've been having another crack at this since I discovered that comment displays don't respect hook_node_access() meaning that even if a user can view the node, the comments will be hidden.

I now understand how to use the node_access table, and I *think* I've got a working solution for permissions propagation.

The biggest problem I found was with hook_node_access_records() being invoked for the node being saved, not the node being referenced. The way I solved this was by calling node_access_acquire_grants() on the referenced node, from within hook_node_access_records(), and passing the grant data as an array in the referenced node object so that it's available to a hook_node_access_records_alter() implementation.

I then added a second block of iteration code in hook_node_grants() to handle the propagation.

I rolled a patch against the current git code so you can take a look. I'm sure it could do with some optimisation but so far it seems to work without breaking anything. Hopefully this is a bit more useful than my previous attempt.

P.S. I didn't know what the new $referenced_states code in the nodeaccess_nodereference_form_field_ui_field_edit_form_alter() function was for so I just added lines for my extra fields to mirror what was there already.

I found an issue when used in conjuction with the Content Access module, where a node_access_rebuild() bugs out trying to write a duplicate record to the node_access table.

I'll try and trace it tomorrow.

I traced the duplicate record problem to the SQL query I added on line 738 in hook_node_access_records() which was too broad.

That query is there to store a copy of existing node_access records, before node_access_acquire_grants() wipes them out. Otherwise, a node save destroys records from previous node saves. I wonder if there's a better way to handle that?

Attached patch fixes the bug and adds a few comments.

Hmm, not quite ready yet. Although it works fine when exisiting nodes are saved, the behaviour isn't right when nodes are created or deleted. I'll continue bug fixing on Monday!

A weekend off does wonders for clearing your head! I realised I was doing a bunch of stupid stuff in that solution so I rewrote quite a bit of it and removed some redundant code.

node_access records are now generated cleanly, both on node creation and node edits, so I got rid of the dumb SQL stuff in hook_node_access_records().

I also figured out how to clean up orphaned node_access records for referenced nodes when the referencing nodes are deleted, by adding a hook_node_delete() implementation.

Finally, I added a condition to account for circular references, though I haven't tested it out since it's so simple.

Check it out.

Patch looks good, I haven't had a chance to run it yet.

nodeaccess_nodereference_get_references() appears to totally ignore node references?

I think we can use nodeaccess_nodereference_get_referenced() to calculate nodeaccess_nodereference_get_references(), it's the same data just stored a different way.

The variable names are a bit wtf... 'import' ?

The way loops are nested often feels like too much work is done before decisions are made.

Also needs updates to nodeaccess_nodereference_reduce_variable() and nodeaccess_nodereference_check_cross_access()

Theres gotta be some way to reduce the code duplication in that big function.

There is a whole bunch of stuff added to $referenced_states in the settings page for no reason at all, and there is no $propagate_states created or used anywhere.

So yeah I don't like the patch.

And I'm still undecided if this functionality should even go into the module.

If I can figure out a way to get the same behaviour without creating this functionality, that would be the ideal solution for me.
#1293792: Entity Reference: Two-way referrence (synchronize 2 fields or use one storage for two-way link)
http://drupal.org/project/cnr

If CNR works as advertised I see no reason for this feature
http://drupal.org/project/cnr

nodeaccess_nodereference_get_references() appears to totally ignore node references?

I didn't bother adding support for node references since that module is supposedly being deprecated in favour of entityreference.

I think we can use nodeaccess_nodereference_get_referenced() to calculate nodeaccess_nodereference_get_references(), it's the same data just stored a different way.

Maybe I'm being stupid, but I don't see a way to supply a target nid and know where references to it are coming from, without iterating through all the possibilities to find out. As far as I can see, the logic has to be different to nodeaccess_nodereference_get_referenced(), so it's almost an arbitrary choice whether you create a new function or add some branching to the existing one.

The variable names are a bit wtf... 'import' ?

Umm, yeah... sorry about that. I couldn't think of something more appropriate at the time. I guess "infer" might be a better term to use?

The way loops are nested often feels like too much work is done before decisions are made.

Yeah, there are almost certainly optimisations to be made. As before, I'm just posting my work as a possible basis for a final solution.

Also needs updates to nodeaccess_nodereference_reduce_variable() and nodeaccess_nodereference_check_cross_access()

I didn't really examine what they do so there's not much I can say to that.

Theres gotta be some way to reduce the code duplication in that big function.

I tried a few things to avoid the duplication, but always ended up with either incorrect grants, or infinite recursion. I ended up separating the logic just to demonstrate that what we're trying to do can actually work.

There is a whole bunch of stuff added to $referenced_states in the settings page for no reason at all, and there is no $propagate_states created or used anywhere.

See the P.S. on comment #26. It can be removed if it serves no purpose.

And I'm still undecided if this functionality should even go into the module.
...
If CNR works as advertised I see no reason for this feature
http://drupal.org/project/cnr

Philosophically speaking, it seems a bit "not the Drupal way" to have to add redundant fields to referenced nodes to achieve what we want. What if 5 different child content types reference a parent content type, and you want permissions from all of them to propagate? You have to add 5 pointless fields to the parent for CNR to work with.

You also have the problem of those extra fields actually being editable by users when the parent node is edited, i.e. a user could remove the references that CNR created. To prevent that you'd have to install the field_permissions module to hide the extra fields from people. Again, not a very elegant solution.

As for actually implementing CNR, first of all, it only supports node references; not entity references. There is a similar module http://drupal.org/project/ref_field which only supports entity references and not node references.

However, when I did some testing with ref_field, where node B inferred update/delete permission from node A and node A inferred view permission from node B, nodeaccess_nodereference caused an infinite recursion crash, so there would still be work needed on this module to safeguard against that.

I did follow this thread, and after re-reading your initial requirements, perhaps these suggestions are helpful:
- indeed, reversing the relationship Contactperson --> Organisation would help. I suppose you have reasons to not choose this alternative.
- have you considered these modules? I am not sure if they can handle your 1:n relation.
http://drupal.org/project/ref_field (Entity)Reference Field Synchronization
http://drupal.org/project/cer Corresponding Entity References
- Try out the new RedHen project

Those are some good suggestions johnv, I think they should be suitable to give the necessary control to the site developer. I think the problem here is that the relationship should indeed be two-way or reversed, and since references are designed to be one-way it shouldn't be up to this module to re-interpret that relationship in reverse, there are other modules for that. Set the direction of the relationship up properly and allow this module to do it's job as normal. There is just too much overhead for this module to double it's workload on every node query. It's too much code to maintain for this module, and it steps on the toes of those modules that seek to add the reverse relationship.

Given the problems you.encounter.with.corresponding references, more alternatives:
-reverse the relationship. Rebuild theListOfContactpersons in a view (a tab,eva,or view reference) Apparently this is not an option.
- convert the Organization into a fielded vocabulary term, and use AccessByTerm or TermAccess. I use ABT, it might even be patched to grant access to shared nodes, not only shared terms.