Improve Node Access system

I've read that improved node access is on the wishlist for Drupal 7. After seeing another stab at this request here, I spent a few hours thinking. I don't think a logic patch is the right approach: it is difficult to understand even for admins, let alone users, it is difficult to make work right, and worst of all, it solves the problem by damaging current functionality.

Right now we have the node_access table, which is the source of the problem. We have two values we can write in it: 1+ for allow and 0 for deny. The problem is that deny is weak: if any other module allows access, the deny is overruled. Actually, deny should be read as "undecided, defaults to deny". Obviously, there are use cases where a strong deny is needed, where deny should overrule allow.

My first thought was to alter the node_access table to accept negative values and use those for deny. I'm certain it would work beautifully, and node access modules would be able to really say what they mean when they say no. The problem is that this would make the already inefficient node_access table even larger: currently 0 values don't have to be written, but -1 values would have to be. Therefore I considered an option which I think is even better: getting rid of the node_access table completely.

The following code is my alteration to node_access():

  // If the module did not override the access rights, call implementations of                                                                                                                    
  // hook_node_access() on all modules.                                                                                                                                                           
  if ($op != 'create' && $node->nid) {                                                                                                                                                            
    $grants = module_invoke_all('node_access', $op, $node, $node->status, $account);                                                                                                              
    $deny_count = 0;                                                                                                                                                                              
    $allow_count = 0;                                                                                                                                                                             
    foreach ($grants as $grant) {                                                                                                                                                                 
      if ($grant === FALSE) {                                                                                                                                                                     
        // Deny request.                                                                                                                                                                          
        $deny_count += 1;                                                                                                                                                                         
      } elseif ($grant === TRUE) {                                                                                                                                                                
        // Allow request, unless denied by another module.                                                                                                                                        
        $allow_count += 1;                                                                                                                                                                        
      } // Undecided, deny by default unless allowed by another module.                                                                                                                           
    }                                                                                                                                                                                             
    if ($deny_count) {                                                                                                                                                                            
      return FALSE;                                                                                                                                                                               
    } elseif ($allow_count) {                                                                                                                                                                     
      return TRUE;                                                                                                                                                                                
    } elseif (count($grants)) {                                                                                                                                                                   
      return FALSE;                                                                                                                                                                               
    } else {                                                                                                                                                                                      
      if ($op == 'view') {                                                                                                                                                                        
        return TRUE;                                                                                                                                                                              
      } else {                                                                                                                                                                                    
        return FALSE;                                                                                                                                                                             
      }                                                                                                                                                                                           
    }                                                                                                                                                                                             
  }                                                                                                                                                                                               

And here is the code for the new hook, hook_node_access():

?php
/**
 * Allow or deny a given operation on a node.
 *
 * When a node is accessed, a module implementing node access will be asked
 * for its opinion whether the operation should be allowed on the node.
 * The module must respond with either TRUE to allow the operation, FALSE
 * to deny it, or NULL if it is undecided.
 *
 * @param $op
 *   The node operation to be performed, such as "view", "update" or "delete".
 * @param $node
 *   The node object on which the operation is to be performed.
 * @param $published
 *   Indicates whether the node is published. Operations on unpublished nodes
 *   should generally not be allowed.
 * @param $account
 *   The user object performing the operation.
 *
 * @ingroup node_access
 */
function hook_node_access($op, $node, $published, $account) {
  // Allow viewing of all pages and deny deletion of pages.
  if ($node->type == 'page' && $published) {
    if ($op == 'view') {
      return TRUE;
    } elseif ($op == 'edit') {
      return NULL;
    } elseif ($op == 'delete') {
      return FALSE;
    }
  }
  return NULL;
}
?>

The way it would work is that each node access module must implement the new node_access hook. This hook implementation receives as arguments the operation, node, published status and user performing the operation. The published status is of course part of the node object, but I made it an argument to highlight its importance. Depending on this information - and its own settings - the node access module could return a clear decision: allow, deny or undecided, defaults to deny. If any module returns deny, it overrules any allows. Otherwise, allow overrules undecided. If all modules are undecided, access is denied. Only if no values are returned, meaning there are no node access modules active, do we use default permissions: allow view and deny everything else.

To me, this solution looks simple and potentially very efficient. Currently we have the bloated node_access table from which we have to search records - here we ask the node access modules directly what access permissions they have for the node. Of course, if the module requires complex and expensive database queries to determine its return value the efficience advantage is lost, or at least diminished. But, in other cases you wouldn't need large queries at all: you could make a node access module which always returns NULL for operations other than 'view' - to be used only for determining view access. Some modules could be made to manage access of a certain type of content only, and they could return NULL immediately without a single database operation.

The one undisputable advantage of this approach is obvious: no more need for expensive node access rebuilds. Also, the confusing system of priorities - which few modules ever used to their full potential - would become unnecessary.

Of course I am aware that a change like this would have far-reaching effects. I haven't even attempted to touch everything that would be affected by this - in this opening post, I simply wanted to explain my approach. The node_access table has another use for the db_rewrite_sql() function, so that would need to be patched also. Other functions would become deprecated, and would need to be removed.

So, let's hear some critique. I'm sure there is some due.