acl_node_grants does not recognize $op (view, edit or delete) [#347027]

I'm using acl in combination with content_access and have noticed that acl does not distinguish between view. edit and delete. This may be simply because of my particular configuration of modules. I'm using Drupal 5.10 and have modified node.module (for multinode access), but I've tested it using the original node.module and on another site, as well, with the same result: acl does not seem to differentiate between view/edit/delete.

/**
 * Implementation of hook_node_grants
 */
function acl_node_grants($account, $op) {
  $array = array('acl' => array());
  $result = db_query("SELECT acl_id FROM {acl_user} WHERE uid = %d", $account->uid);
  while ($row = db_fetch_object($result)) {
    $array['acl'][] = $row->acl_id;
  }
  return !empty($array['acl']) ? $array : NULL;
}

It returns all grants for the user regardless of what they allow. I've found that when node_access_grants_sql processes this, it uses only the last permission found for that user:

      foreach ($rule as $gid) {
        // Grants ids must be numeric.
        if (is_numeric($gid)) {
          $grants[$group][$realm] = $gid;
        }  
      }

and so regardless of the individual user settings, the result (in the case of unmodified node.module) is view access only. With the multinode access patch installed this became full access only.

In order to get the behaviour I require I have rewritten the function as follows:

/**
 * Implementation of hook_node_grants
 */
function acl_node_grants($account, $op) {
  $array = array('acl' => array());
  $result = db_query("SELECT acl_id FROM {acl_user} WHERE uid = %d", $account->uid);
  while ($row = db_fetch_object($result)) {
     $grant_name = db_result(db_query("SELECT name FROM {acl} WHERE acl_id=%d", $row->acl_id));
     if(strpos('a'.$grant_name, $op)) $array['acl'][] = $row->acl_id;
  }
  return !empty($array['acl']) ? $array : NULL;
}

strpos('a'.$grant_name, $op) is like this because I'm danged if I can get strpos to read from the beginning of a string, so I add an 'a'.

Anyway this produces the desired behaviour: when I allow edit permissions, the user can edit, when I allow only view, the user can only view.

Comments

Comment #1

salvis

he/his

CreditAttribution: salvis commented 26 December 2008 at 13:50

Sorry for taking so long to respond.

I see that we're ignoring $op, which may cause problems, and I'd like to create a setup which lets me test this, but I'm not sure how to do this. What is node_access_grants_sql that you're referring to?

Your rewrite (with a query inside a fetch loop) is very wasteful, something we can't have deep down in the infrastructure. This needs to be rewritten as a join in the outer query.

Please adhere to the Drupal coding conventions and submit code changes as patches, so that they can be properly tracked.

Comment #2

NewZeal CreditAttribution: NewZeal commented 2 January 2009 at 23:59

node_access_grants_sql() is in node.module. In Drupal 5.10 it is written as follows:

function node_access_grants_sql($op = 'view', $node_access_alias = NULL, $account = NULL, $status = TRUE) {
  // Define the grants array and variable strings.
  $grants = array();
  $alias = '';
  $grants_sql = '';

  // First, acquire all the grants as an array of rules.
  $rules = node_access_grants($op, $account);

  // Prepare the table alias, if needed.
  if (!empty($node_access_alias)) {
    $alias = $node_access_alias .'.';
  }
  // It may be that only the default rule is active.  In that case, we can skip the
  // process below.
  if (count($rules) == 1 && key($rules) == 'all') {
    $grants_sql .= "AND (". $alias ."realm = 'all' AND ". $alias ."gid = 0)";    
  }
  else {
    // Process the grants into logical groupings.
    foreach ($rules as $realm => $rule) {
      // Set the status flag.
      if (isset($rule['check'])) {
        $status += $rule['check'];  
      }
      // Set the default clause group.  Then check for options.
      $group = 'all';
      if (!empty($rule['group'])) {
        $group = $rule['group'];
      }
      foreach ($rule as $gid) {
        // Grants ids must be numeric.
        if (is_numeric($gid)) {
          $grants[$group][$realm] = $gid;
        }  
      }  
    }
    // In most cases, node status must be set to TRUE.  However, there are use-cases where 
    // we allow access to unpublished nodes.  So we check the status to see if we need to 
    // continue with the logic or end this IF statement.  
    if (!$status) {
      return FALSE;
    }
    // Run throught the $grants, if needed and generate the query SQL.
    if (count($grants)) {
      foreach ($grants as $group => $grant) {
        $clauses = array();
        foreach ($grant as $key => $value) {
          $clauses[] = "(". $alias ."realm = '$key' AND ". $alias ."gid = $value)";
        }
        if ($group == 'all') {
          $grants_sql .= 'AND ('. implode(' OR ', $clauses) .') ';            
        }
        else {
          // Required elements must go into a subquery.  Will not work prior to MySQL 4.1.x.
          $subquery = "AND ". $alias ."nid IN (SELECT ". $alias ."nid FROM {node_access} ". $node_access_alias ." WHERE ";
          $grants_sql .= $subquery .'('. implode(' OR ', $clauses) .')) ';            
        }
      }  
    }
  }  
  return $grants_sql;
}

Yes, I agree the patch could be written more efficiently and I could use patch technology. So far I have not been able to implement patches on my PC and get it to work correctly. Do you have any links to tutorials for me to learn this?

Many thanks

Comment #3

salvis

he/his

CreditAttribution: salvis commented 3 January 2009 at 02:06

I'm looking at node.module from Drupal 5.10: // $Id: node.module,v 1.776.2.30 2008/07/16 19:04:21 drumm Exp $
It has 3097 lines but no such function...

Have you hacked core?

http://drupal.org/patch has extensive coverage of patching.

Comment #4

NewZeal CreditAttribution: NewZeal commented 4 January 2009 at 23:11

My apologies,

My copy of node.module in my main repository has been inadvertently replaced by the multinode access patched version. node_access_grants_sql comes from the patch.

I have reinstalled drupal 5.10 into the repository and checked acl with the unpatched node.module and it is working correctly because the sql is constructed as follows:

   $grants = array();
    foreach (node_access_grants($op) as $realm => $gids) {
      foreach ($gids as $gid) {
        $grants[] = "(gid = $gid AND realm = '$realm')";
      }
    }

in which the realm is included in the grants array.

This bug should be transferred to the multinode access patch: http://groups.drupal.org/node/5392 which needs to be modified in order to cater for acl as it claims.

I apologise for inconveniencing you. It is taking me a while to understand how modules interface with node_access.