I am having issues with Organic Groups and Node Access User Reference. Specifically, the Node Access User Reference module adds node grants on a node to users who are referenced in a user reference field. After installing Organic Groups, the node grants no longer work as expected for edit and delete operations. After reading the Node access rights page, I think I understand why.

Just for some background for other people reading this issue, there are two node access control systems in D7. One is node grants where permissions are stored statically in the database, and the other is evaluated on runtime via hook_node_access() implementations. Interestingly, if any module explicitly denies or approves access in hook_node_access(), the node grants are never even evaluated.

Therefore, modules that implement node grants such as Node Access User Reference are effectively superseded by OG for edit and delete operation since OG always explicitly allows or denies access for edit / delete operations. The attached screenshot shows the Devel Node Access tables for a standard group post both with / without OG to further illustrate this point.

I ended up removing the explicit $return = NODE_ACCESS_DENY; initializations in og_node_access() and everything works as I would expect it to in my specific instance, however I am sure this probably has security implications for more general application.

Thanks for all your hard work on a much needed revamp of this module,
Chris

CommentFileSizeAuthor
without-og.png118.81 KBcpliakas
with-og.png118.06 KBcpliakas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FatherShawn’s picture

Further, it looks like hook_og_user_access_alter only fires if the user is a group member. I'm using Content Access, which includes an "Edit Any [node type]" grant, but the problem described in this issue prevents this from functioning if the node is assigned to a group and there's no way to use the og api to alter the access if the user is not a member of the group.

OK - I was wrong about this, the hooks are firing for non-members. Not sure why my tests using dpm() calls weren't outputting messages before. I can write code for the business logic of my use case. Still clearly a problem for non-coders.

spouilly’s picture

Version: 7.x-1.x-dev » 7.x-1.3

Hi there,

I am having the same issue. I am actually building a hierarchy of groups, where I want to give some admin from a "parent" group, full editing power on all "children"groups, even if they are not members of the "children" groups.

As described by cpliakas, the main issue is that in og_node_access, in some circumstances, the function would return a NODE_ACCESS_DENY. From what I understand, as soon as this is done, the game is over, and no other method of granting access (such as through hook_node_grants) will be evaluated.

This is a serious issue that make it difficult for custom modules to grant/deny extra permisions on group.

Instead of returning NODE_ACCESS_DENY, the function should return NODE_ACCESS_IGNORE, and doing so delegating the issue of deciding whethr to grant/deny access to another module. By default, if no module specifically grant access to the group type, then the user will be denied access (so the default behavior is the same as having a NODE_ACCESS_DENY). On the other hand, this change would allows custom module developers to define their own precise node access behavior through node_access_grants & co.

I can provide a patch if needed (simply replace all NODE_ACCESS_DENY by NODE_ACCESS_IGNORE in og_node_access).

Cheers,

Sylvain

cpliakas’s picture

Status: Active » Fixed

This has been fixed by the "Strict node access permissions" setting added at #1317164: Use "any" core node permission so that non-members can modify or delete content. Disabling the checkbox effectively does the same thing as the technique mentioned in the original post, i.e. commenting out $return = NODE_ACCESS_DENY; initializations in og_node_access().

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.