After unpublish & publish revisions, update node_access records

I ran into the same issue… node_access entries for the revisioning nodes didn’t seem to be created or destroyed unless the node_access table is rebuilt via some other mechanism. node_access_acquire_grants() checks the status of the node and if it’s not published it never adds the realm ‘all’ grant. It looks like _revisioning_publish_revision() and _revisioning_unpublish_node() bypass node_save() and a call to node_access_acquire_grants() doesn’t work because the $node->status doesn’t reflect the published state.

Something like the following could be done… in _revisioning_publish_revision() and _revisioning_unpublish_node() to ensure the node_access entries are created/destroyed

  // update node_access table
  $node->status = NODE_PUBLISHED;
  node_access_acquire_grants($node, FALSE);

  // update node_access table
  $node->status = NODE_NOT_PUBLISHED;
  node_access_acquire_grants($node, TRUE);

I can provide a patch if there is interest.

Here's the patch.

Seems the issue still occurred when a new revision was created and unpublished (moderated). Updated patch to include setting the node status to the current_status in revisioning_node_update() which occurs just before node_save()’s call to node_access_acquire_grants().

Thanks cda!
Will check into Git asap.
Rik

Found a couple of small problems with the last patch... third time is a charm.

Last patch did not check that current_status existed, which caused a problem on non-revisioning nodes. This should be the final patch...

Patch from #7 worked great.

Patch from #7 is clearing up problems for me. Being used with Taxonomy Access Control.

See also the solution at #1282084: Revisioning may not be compatible with certain modules that use hook_node_access_records() (e.g. Domain Access), which may kill two birds with one stone, as joevansteen points out in that issue.

Patch #7 resolved my problem, where i had to manually update access control settings each time i published a revision.
Is this patch going to be merged with the module — this might be the most stupid question you ever heard, but i just want to be sure :)

Thank you for all the work on one of the greatest modules!

@Mangostaniko
Yep as soon as I have time to do the next batch of Revisioning patches and fixes.
Rik

Will do this before the end of 2011...

Committed to 7.x-1.x branch. Will be available in 7.x-1.4.
Credited to c.d.a: http://drupal.org/node/407994/committers.
Congrats on your first credited patch, Chris!

Note: The fix is not available in 7.x-1.3 (which only include a fix for https://drupal.org/node/1409268), I expect it will be available with 7.x-1.4.

That's right mongilo404!.

This patch appears to work but there is a problem with Taxonomy Access Control.

If I go in and change a TAC controlled term on a node, and save then publish it, then come back in as another user who does not have access to that term, I can still edit the node and its revisions. I should only be able to do this if I have permission through TAC (in node_access).

Bugger! Will investigate.

I also want to note that I am using Revisioning, TAC, and Workflow in conjunction.

Revisioning should maintain versions of content.

TAC needs to control if a user can edit the content/revisions. (term assigned to roles)

Workflow needs to control the state of the content.

I am thinking maybe Workflow Access is in the way here, but just wanted to throw it out there. Since WA seems to always put grant_x to 1 in the database, it might be winning out over TAC.

Glad you mentioned that Kevin,
Other modules that control access like TAC and Workflow Access certainly pack a punch in determining out what the resulting outcome will be....
Would you have any other details that may lead to a reproduceable test case?
Rik

After disabling WA, behavior is a bit closer to what is needed, with one odd side effect:

A user cannot seem to edit older revisions, even if the term is in their Allowed TAC list- it seems access is dictated by the current published revision. Can I conclude then, that this is because node_access is not tracking revision access, but access to the master node?

Pretty much.
I think you'll continue to have these problems when using TAC.

Ahh, okay. That's a little confusing and counterintuitive to me for true collaborative editing, but, I guess thats the way it is.

Kevin,
Have you looked at or tried the TAC Unpublished module? I was playing with TAC and Revisioning and it helped clear up some issues for me. I can't say they were the same issue you are having, but you might try it before you go elsewhere or give up. It extends TAc access control over unpublished nodes. My impression is that it should be merged with the TAC module over time. It is a bridge/patch for some problems in this area with the current module.

Oh, no, I am still using Revisioning/TAC/Workflow, but thought it was odd that Workflow/TAC access was not revisioned too, allowing multiple access to revisions, not just one access check. I can see where that would become huge, though.

TAC Unpublished is something to use IN ADDITION to TAC to provide access control via TAC for "unpublished" nodes. You might want to review the documentation there and the issue documentation that is linked from the main page. Just a thought. It might help you out.

Hi,

I have been having problems with the change implemented by this patch, which is now committed to 7.x-1.4.

The problem occurs when I use the a rule (from Rules module) with the action 'Publish the most recent pending revision' to publish a new node when certain conditions are met.

When creating a node using drupal's node admin page, and triggering rules to publish the node, node_save() seems to be called after the _revisioning_publish_revision() has run the node_access_acquire_grants() function call. Hence, when the function is called in node_save() again, you'll get an integrity constraint violation error for duplicate entry inserting to node_access table because the row has already been inserted by the node_access_acquire_grants() function call in _revisioning_publish_revision() earlier.

This is only a problem with new nodes. In node_save(), when a node is new, the $delete argument is set to false in the node_access_acquire_grants() function call as it assumes there is no existing row in the node_access table for the node since the node is new. This is not a problem with existing nodes as node_access_acquire_grants() will just delete existing rows by default when the $delete argument is not explicitly set to false.

To replicate this, you'll need:

• Drupal 7.14
• Revisioning 7.x-1.4 (or dev)
• Rules 7.x-2.1
• A module that sets node_access_records, such as View Unpublished 7.x-1.0
• A patch to fix another bug with the "Publish the most recent pending revision" action which I have raised separately.
• Then you'll need to create a rule to publish the latest revision of a new unpublished node (in content type, set the node to be not published by default). Eg rule:
  

  { "rules_test_approve_new_node" : {
      "LABEL" : "Test approve new node",
      "PLUGIN" : "reaction rule",
      "REQUIRES" : [ "revisioning", "rules" ],
      "ON" : [ "node_insert" ],
      "DO" : [
        { "revisioning_rules_action_publish_latest" : { "node" : [ "node" ] } }
      ]
    }
  }
  

To trigger the error:

Create a new node using Drupal node admin interface and save it (that will trigger the rule).

Here is an example of the error I got:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '2598-1-view_unpublished_content' for key 'PRIMARY': INSERT INTO {node_access} (nid, realm, gid, grant_view, grant_update, grant_delete) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5), (:db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11); Array ( [:db_insert_placeholder_0] => 2598 [:db_insert_placeholder_1] => view_unpublished_content [:db_insert_placeholder_2] => 1 [:db_insert_placeholder_3] => 1 [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => 0 [:db_insert_placeholder_6] => 2598 [:db_insert_placeholder_7] => view_unpublished_page_content [:db_insert_placeholder_8] => 1 [:db_insert_placeholder_9] => 1 [:db_insert_placeholder_10] => 0 [:db_insert_placeholder_11] => 0 ) in node_access_write_grants() (line 3429 of /private/var/www/drupal7-demo/modules/node/node.module).

Possible fix:

As this only affects new nodes, a quick fix would be to only call node_access_acquire_grants() in _revisioning_publish_revision() when the node is NOT new. When the node is newly created via the node/add/ page, the node_access_acquire_grants() will be called by node_save() anyway.

I have attached a patch for this (to be applied to Revisioning 7.x-1.4 with the earlier patch on this issue already committed), and will be great if someone can look at it and see if there's a better fix or if this fix has any side effects, as I have to admit that after investigating this problem for hours, my mind might not be in its clearest state at the moment!

Thanks!

Patch applied. Better late than never. Thanks victoriachan!
Rik