Closed (fixed)
Project:
Module Grants
Version:
6.x-3.5
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
23 Mar 2010 at 01:03 UTC
Updated:
10 Apr 2010 at 00:40 UTC
Jump to comment: Most recent file
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.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | uid-in-cache-key.patch | 2.65 KB | james.cartledge |
Comments
Comment #1
rdeboerGood 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....
Comment #2
james.cartledge commentedHi 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.
Comment #3
rdeboerUnderstand 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).
Comment #4
james.cartledge commentedContent access modules: just TAC_Lite.
Comment #5
rdeboerChecked into repository (HEAD). Will be part of next official release.