More info for why user is allowed/denied access

whoops one line should be changed:
if (db_result($result))
return t('ALLOW: node_access lookup, rows found: '). check_plain($sql);//not caching each string since sql will be different each
else
return t('DENY: node_access lookup, no rows found: '). check_plain($sql);

btw not thinking this patch should go into module, just as needed can add easily to doublecheck your perms.

this actually helped me find a problem that was confusing me with perms...
devel was reporting wrong node access rights by user...
i saw that a anonymous user could update/delete according to devel! wahhh! but not really when actually tested that user?
realized my patch said it was image_access giving rights, basically issue was this that i posted in IRC:

"For example: in Image module you do a check for user has perms to 'edit images' and then 'edit own images' for delete/update $op.... For the super user testing access things out, he has both rights, and when devel checks node_access()'s image_access(), it cuts short after first test for 'edit images'. For another user with just 'edit own images', devel's node access will check only the current logged in user aka superuser and report falsey that user can access the node. Fix is everyone should update modules so uses 3rd arg $account in hook_access (Image doesnt), and do something like "$account = ($account!==NULL) ? $account : $user;" and use that for user checks so devel can work better. am i wrong or that makes sense?"

hey salvis,
yeh i had the DNA debug mode on. i was looking at the Access Perms by User for that node, and noticed no matter what id do for regular user, that it said they had 'yes' to update and delete ops. but in real life they did not. it made sense to me why Devel reported it that way. Cause when it checks node_access to return true/false, it passes the $account that it is checking, which is good. but in many modules, it does not use the $account param, and hence uses the global $user implicit or sometimes explicitly there, (which in turn user_access() will use) which is in most peoples cases testing the superuser. So in my case, the Image module's image_access() function just checks for user_access('edit images') and not user_access('edit images', $account) so hence why i think wrong info is reporting in Devel DNA... it checks to see if superuser has access to edit images and not the other account.

am i missing something? after changing Image module, Devel reported what drupal was doing in real life and what i thought should be correct access perms.

[whoops sorry. i just made that patch with a cygwin diff (and extra module code), maybe i did diff wrong. very simple code modifying node_access to output its info. i would have to say also that i should post small 1 line of changes to Image module as then you would see different perms show up as that is the heart of the problem and not Devel. my patch just outputs to screen the info on why user was denied or allowed]

okay, ill check that node out tomorrow and post that info. but theoretically does it not make sense the first paragraph i posted here in this post? hoping i understood what was happening, spent all day trying to figure it out )

oh yes i agree, the module usually wants to know the current user, but Devel wants to know how module reacts for other users in its 'Access Perms by User' section like you said, so user_access for a module will return for current superuser, when in that case it really wants the user being inspected. In other words for any module that uses user_access without passing in $account, will have good chance of info being displayed incorrectly in Devel unless put tiny fix the module's hook_access function. I confirmed this again this morning that user shouldnt have access, and rights are set so doesnt have access, but DNA says yes unless fix module.

Just confirming, are we agreeing that DNA might display wrong info for small minority of modules, though modules which more likely that DNA wants to look at, content-type modules like Image (not Devel's fault, but cause modules dont check which to use, $user or $account in hook_access() )?

okay here is a Image type DNA output in the attachment pics, and extra info accompanying it:
i need help debugging one more thing which i think is not correct in Access info, ill add to that at the end of this post...

So in pic 1 that is attached, u see the test1, system, contenteditor roles should have edit and update access, everyone else can only view.

in pics 2 and 3, i just included that info to show you what is going on, it doesnt really apply in my case since case i am looking at is when hook_access perms win, and not {node_access} entries......

in pic2, just making sure db got written correctly, node access is view for all, and edit/update added for those 3 roles for content_access acl module
in pic3, Devel confirms for those roles they only can access it (of course only if node_access() will end up using {node_access} entries, system can still bypass these like if user has 'administer nodes' rights or, hook_access() from module has something to say about the node's access before)

in pic4, i just add my info at the bottom of each row, i hope u dont mind as it clarifies what going on behind scenes, like what exactly gives access.
Pay attention to mbarret user, that user, has no update/delete rights in User Perms in Drupal for Images (as she is 'employee' role and they dont get 'edit [all] images' rights or 'edit own images' rights so shouldnt be able to access node,
since hook_access is being used, something is funky in there that Devel thinks allows her access, when she doesnt.

this code in drupal's Image module is useful to understand problem:

function image_access($op, $node) {//not passed $account from node_access function, 1st problem to Devel
  global $user;

  if ($op == 'create' && user_access('create images')) {
    return TRUE;
  }

  if ($op == 'update' || $op == 'delete') {
    if (user_access('edit images')) {//2nd problem for Devel, when i run Devel node access, no $account is passed to test mbarret's rights, so user_access uses what is available to determine rights. it will run as superuser, and superuser will have more rights, so can access this node. Devel recieves wrong info 
      return TRUE;
    }
    if (user_access('edit own images') && ($user->uid == $node->uid)) {//same as above
      return TRUE;
    }
  }
}

this is what content-type modules with an hook_access function like image_access should be so Devel can run correctly i think....

function image_access($op, $node, $u_account=NULL) {//need to add 3rd $account arg if module like Devel passing in info
  global $user;
  $u_account = ($u_account!==NULL) ? $u_account : $user;//correctly checks if Devel or regular module checking for perms

  if ($op == 'create' && user_access('create images', $u_account)) {
    return TRUE;
  }

  if ($op == 'update' || $op == 'delete') {
    if (user_access('edit images', $u_account)) {
      return TRUE;
    }
    if (user_access('edit own images', $u_account) && ($u_account->uid == $node->uid)) {
      return TRUE;
    }	
  }
}

now in pic 5, user rights are correct, she doesnt have access in real life, and Devel reports it that way.
on a sidenote, if she had 'edit images' rights, or (is the author of that pic and she has 'edit own images' rights), hook_access will say she is allowed access as that it will truthfully report back what she can access.
So image_access should have go through and doesnt return TRUE, by default, {node_access} checked next and shows user also doesnt have rights there.

I am having one other similar issue with devel reporting back something i think is wrong, if you want to me debug that one after this issue, specifically saying user has 'admin nodes' but user clearly doesnt. EDIT: this last new issue might be fixed. looks like maybe perms were being cached for this one. avoid this paragraph unless i followup that happens again.

Thanks for any help,
Arian Hojat

hey salvis, sorry about that, reposted in last post.

okay ill let them know. Didnt realize $account was a required field anyway, thanks for pointing that out.

well possibly propose to have similar output in the 4/5 pics i attached above. see how row below the normal Devel Node Access, it says why it was Allowed. I just output the SQL for node_access as that could be useful to user to see what is allowing/rejecting for that person. or which module giving access, or maybe user is administrator. this was helpful for me. but my output can be cleaned up, maybe expandable link so that for example that long sql doesnt need to be shown until user clicks on something; or maybe stays hidden until you hit 'yes' so output can stay minimal unless user is curious about why user not given access.

Attached is the function i use to output that info in a .module file attached (actually i couldnt upload .module file, so i changed filename to .txt if thats alright, its just outputting same stuff node_access() does, nothing fancy ).

and all i did was add it to Devel Node Access, was another row (not sure why my patch wasnt working so here it is):

            while ($data = db_fetch_object($result)) {
              $account = user_load(array('uid' => $data->uid));
              $rows[] = array(theme('username', $data),
                              theme('dna_permission', node_access('view', $node, $account)),
                              theme('dna_permission', node_access('update', $node, $account)),
                              theme('dna_permission', node_access('delete', $node, $account)),
              );
              +//following lines added to devel_node_access_block() function in devel_node_access.module, didnt theme or do anything fancy to output
	      +$rows[] = array('',
		+		node_access_moreinfo('view', $node, $account),
		+		node_access_moreinfo('update', $node, $account),
		+		node_access_moreinfo('delete', $node, $account),
	       +);		
            }

yes i am not the best at constructing sentences to display ;).
Maybe can keep everything hidden like current UI is, until click that row, then expands row underneith it with more info and if hit row again collapses... But definately an expand/collapse all link somewhere in there might help to see all at once, maybe above top row? not sure what is good UI. I can start looking at that Monday maybe.

yeh 'create' $op is good too... node_access() uses only hook_access() to check that, right?
I never understood why drupal/{node_access}table doesnt just include 'create' op, but thats another issue.
That should be simple change to devel_node_access_block ()...
+ theme('dna_permission', node_access('create', $node, $account)),

>What was your approach for working around Image's shortcomings?
i just modified Image as shown in post #7 so Devel can get correct info; but I'd prob modify it more as just realized global $user check i added not needed. Since as you said $account is not optional, i was wondering why... node_access() does the work of checking $user/$account and passing correct info into hook_access so that line i put in is redundant. so this will do i think:

function image_access($op, $node, $account) {//need to add 3rd $account arg if module like Devel passing in info

  if ($op == 'create' && user_access('create images', $account)) {// all user_access() should get $account arg that recieves automatically from node_access()
    return TRUE;
  }

  if ($op == 'update' || $op == 'delete') {
    if (user_access('edit images', $account)) {
      return TRUE;
    }
    if (user_access('edit own images', $account) && ($account->uid == $node->uid)) {
      return TRUE;
    }
  }
}

hey salvis,

Here is addition to Devel if you want to try it/comment if should be cleaned up and added to Devel.
Any code suggestions welcome. I moved all the verbose stuff to title tag when hover over. Can take out if you want.
Checks for 'create' and verbose rows hidden. Click the row to get more info about it, jquery will unhide the moreinfo row.
Currently I added the 'expand/collapse all' functionality to just clicking Access Permissions by User heading (wasnt sure where to put +- collapse buttons).

Image attached of how it looks.

Attached is a patch for devel-6.x-1.10. I moved it out of custom function and into Devel for now on my system.

k thanks for tips, Ill go through Coder sometime this week...

> What's the point of displaying a potentially huge "SELECT COUNT(*)" statement?
i thought that the 'node_access entries for nodes shown on this page' table is useful for debugging, but wasnt sure if those entries will be different for different people... is node_access entries same for all users, as its just looking up grants for the node? if so I can understand why not useful, esp when gets big sql. I do like knowing why exactly those entries are being returned but i can get rid of sql if it gets huge and ugly.

>You've taken the full node_access() method from node.module, but the first few tests never apply in our context.
the 'not a node' test i guess can take out. what other one(s),?