CVS revision 1.53 introduced a static var to module_grants_node_access to cache access requests keyed by nid and node op.

This fails to take into account the user for whom we're checking access. Example:


// Lookup update access for nid=x, uid=y
$node = node_load($x);
$account_y = user_load($y);
$access = module_grants_node_access('update', $node, $account_y);
// uid=y doesn't have update access to nid=x so $access["$x:update"] is set to FALSE.

// Later (in the same request lifecycle,)  lookup update access for nid=x, uid=z
$account_z = user_load($z);
$access = moudle_grants_node_access('update', $node, $account_z);
// $access["$x:update"] is set from the previous lookup, 
// so FALSE is returned without checking access for uid=z

The patch adds uid to the cache key.

CommentFileSizeAuthor
#2 uid-in-cache-key.patch2.65 KBjames.cartledge

Comments

rdeboer’s picture

Good point, James.
Is this only happening when used as above, i.e. with custom code or can you think of, say, a Revisioning scenario, where this applies?
Oh... the patch seems to be missing....

james.cartledge’s picture

StatusFileSize
new2.65 KB

Hi Rik,

No, I am not aware of where this would affect Revisioning. My use case is that I want to display a block of users who can edit a given page so I check each user against the node and operation (update.)

Sorry about the patch. Here it is.

rdeboer’s picture

Understand your use-case; good to realise that current users won't have to fear about permission issues when Module Grants is used "normally".
Just out of interest... what content access modules are at play in your setup?
Patch looks great (as I knew it would). Checked into the repository (CVS HEAD).

james.cartledge’s picture

Content access modules: just TAC_Lite.

rdeboer’s picture

Assigned: Unassigned » rdeboer
Status: Active » Fixed

Checked into repository (HEAD). Will be part of next official release.

Status: Fixed » Closed (fixed)

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