- Patch #537862 by Crell, Amitaibu, salvis: simplify and make more flexible the node access by converting hook_access() to hook_node_access().

5 files changed, 213 insertions, 196 deletions

diff --git a/modules/blog/blog.module b/modules/blog/blog.module
index af72d54f7..9ca7ff9af 100644
--- a/modules/blog/blog.module
+++ b/modules/blog/blog.module
@@ -20,28 +20,6 @@ function blog_node_info() {
 }
 
 /**
- * Implement hook_permission().
- */
-function blog_permission() {
-  return node_list_permissions('blog');
-}
-
-/**
- * Implement hook_access().
- */
-function blog_access($op, $node, $account) {
-  switch ($op) {
-    case 'create':
-      // Anonymous users cannot post even if they have the permission.
-      return user_access('create blog content', $account) && $account->uid;
-    case 'update':
-      return user_access('edit any blog content', $account) || (user_access('edit own blog content', $account) && ($node->uid == $account->uid));
-    case 'delete':
-      return user_access('delete any blog content', $account) || (user_access('delete own blog content', $account) && ($node->uid == $account->uid));
-  }
-}
-
-/**
  * Implement hook_user_view().
  */
 function blog_user_view($account) {
diff --git a/modules/forum/forum.module b/modules/forum/forum.module
index 9737e881e..29a6289b4 100644
--- a/modules/forum/forum.module
+++ b/modules/forum/forum.module
@@ -405,20 +405,6 @@ function forum_node_info() {
 }
 
 /**
- * Implement hook_access().
- */
-function forum_access($op, $node, $account) {
-  switch ($op) {
-    case 'create':
-      return user_access('create forum content', $account);
-    case 'update':
-      return user_access('edit any forum content', $account) || (user_access('edit own forum content', $account) && ($account->uid == $node->uid));
-    case 'delete':
-      return user_access('delete any forum content', $account) || (user_access('delete own forum content', $account) && ($account->uid == $node->uid));
-  }
-}
-
-/**
  * Implement hook_permission().
  */
 function forum_permission() {
@@ -428,7 +414,6 @@ function forum_permission() {
       'description' => t('Manage forums and configure forum administration settings.'),
     ),
   );
-  $perms += node_list_permissions('forum');
   return $perms;
 }
 
diff --git a/modules/node/node.api.php b/modules/node/node.api.php
index 1fc8d6d3a..2aa02c320 100644
--- a/modules/node/node.api.php
+++ b/modules/node/node.api.php
@@ -342,6 +342,67 @@ function hook_node_load($nodes, $types) {
 }
 
 /**
+ * Control access to a node.
+ *
+ * Modules may implement this hook if they want to have a say in whether or not
+ * a given user has access to perform a given operation on a node.
+ *
+ * The administrative account (user ID #1) always passes any access check,
+ * so this hook is not called in that case. Users with the "bypass node access"
+ * permission may always view and edit content through the administrative
+ * interface.
+ *
+ * Note that not all modules will want to influence access on all
+ * node types. If your module does not want to actively grant or
+ * block access, return NODE_ACCESS_IGNORE or simply return nothing.
+ * Blindly returning FALSE will break other node access modules.
+ *
+ * @link http://api.drupal.org/api/group/node_access/7 More on the node access system @endlink
+ * @ingroup node_access
+ * @param $node
+ *   The node on which the operation is to be performed, or, if it does
+ *   not yet exist, the type of node to be created.
+ * @param $op
+ *   The operation to be performed. Possible values:
+ *   - "create"
+ *   - "delete"
+ *   - "update"
+ *   - "view"
+ * @param $account
+ *   A user object representing the user for whom the operation is to be
+ *   performed.
+ * @return
+ *   NODE_ACCESS_ALLOW if the operation is to be allowed;
+ *   NODE_ACCESS_DENY if the operation is to be denied;
+ *   NODE_ACCESSS_IGNORE to not affect this operation at all.
+ */
+function hook_node_access($node, $op, $account) {
+  $type = is_string($node) ? $node : (is_array($node) ? $node['type'] : $node->type);
+
+  if (in_array($type, nodeperms_get_configured_types())) {
+    if ($op == 'create' && user_access('create ' . $type . ' content', $account)) {
+      return NODE_ACCESS_ALLOW;
+    }
+
+    if ($op == 'update') {
+      if (user_access('edit any ' . $type . ' content', $account) || (user_access('edit own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
+        return NODE_ACCESS_ALLOW;
+      }
+    }
+
+    if ($op == 'delete') {
+      if (user_access('delete any ' . $type . ' content', $account) || (user_access('delete own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
+        return NODE_ACCESS_ALLOW;
+      }
+    }
+  }
+
+  // Returning nothing from this function would have the same effect.
+  return NODE_ACCESS_IGNORE;
+}
+
+
+/**
  * The node is about to be shown on the add/edit form.
  *
  * @param $node
@@ -665,54 +726,6 @@ function hook_node_type_delete($info) {
 }
 
 /**
- * Define access restrictions.
- *
- * This hook allows node modules to limit access to the node types they
- * define.
- *
- * @param $op
- *   The operation to be performed. Possible values:
- *   - "create"
- *   - "delete"
- *   - "update"
- *   - "view"
- * @param $node
- *   The node on which the operation is to be performed, or, if it does
- *   not yet exist, the type of node to be created.
- * @param $account
- *   A user object representing the user for whom the operation is to be
- *   performed.
- * @return
- *   TRUE if the operation is  to be allowed;
- *   FALSE if the operation is to be denied;
- *   NULL to not override the settings in the node_access table, or access
- *     control modules.
- *
- * The administrative account (user ID #1) always passes any access check,
- * so this hook is not called in that case. If this hook is not defined for
- * a node type, all access checks will fail, so only the administrator will
- * be able to see content of that type. However, users with the "administer
- * nodes" permission may always view and edit content through the
- * administrative interface.
- * @see http://api.drupal.org/api/group/node_access/7
- *
- * For a detailed usage example, see node_example.module.
- *
- * @ingroup node_access
- */
-function hook_access($op, $node, $account) {
-  if ($op == 'create') {
-    return user_access('create stories', $account);
-  }
-
-  if ($op == 'update' || $op == 'delete') {
-    if (user_access('edit own stories', $account) && ($account->uid == $node->uid)) {
-      return TRUE;
-    }
-  }
-}
-
-/**
  * Respond to node deletion.
  *
  * This is a hook used by node modules. It is called to allow the module
@@ -929,11 +942,11 @@ function hook_validate($node, &$form) {
  * @return
  *   $node. The passed $node parameter should be modified as necessary and
  *   returned so it can be properly presented. Nodes are prepared for display
- *   by assembling a structured array, formatted as in the Form API, in 
- *   $node->content. As with Form API arrays, the #weight property can be 
+ *   by assembling a structured array, formatted as in the Form API, in
+ *   $node->content. As with Form API arrays, the #weight property can be
  *   used to control the relative positions of added elements. After this
  *   hook is invoked, node_build() calls field_attach_view() to add field
- *   views to $node->content, and then invokes hook_node_view() and 
+ *   views to $node->content, and then invokes hook_node_view() and
  *   hook_node_build_alter(), so if you want to affect the final
  *   view of the node, you might consider implementing one of these hooks
  *   instead.
diff --git a/modules/node/node.module b/modules/node/node.module
index 94373e834..1320a9d05 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -16,6 +16,21 @@
 define('NODE_NEW_LIMIT', REQUEST_TIME - 30 * 24 * 60 * 60);
 
 /**
+ * Modules should return this value from hook_node_access() to allow access to a node.
+ */
+define('NODE_ACCESS_ALLOW', 'allow');
+
+/**
+ * Modules should return this value from hook_node_access() to deny access to a node.
+ */
+define('NODE_ACCESS_DENY', 'deny');
+
+/**
+ * Modules should return this value from hook_node_access() to not affect node access.
+ */
+define('NODE_ACCESS_IGNORE', NULL);
+
+/**
  * Implement hook_help().
  */
 function node_help($path, $arg) {
@@ -1337,10 +1352,9 @@ function node_permission() {
     ),
   );
 
-  foreach (node_type_get_types() as $type) {
-    if ($type->base == 'node_content') {
-      $perms += node_list_permissions($type);
-    }
+  // Generate standard node permissions for all applicable node types.
+  foreach (node_permissions_get_configured_types() as $type) {
+    $perms += node_list_permissions($type);
   }
 
   return $perms;
@@ -2222,27 +2236,35 @@ function node_search_validate($form, &$form_state) {
  *
  * In determining access rights for a node, node_access() first checks
  * whether the user has the "bypass node access" permission. Such users have
- * unrestricted access to all nodes. Then the node module's hook_access()
- * is called, and a TRUE or FALSE return value will grant or deny access.
- * This allows, for example, the blog module to always grant access to the
- * blog author, and for the book module to always deny editing access to
- * PHP pages.
- *
- * If node module does not intervene (returns NULL), then the
- * node_access table is used to determine access. All node access
- * modules are queried using hook_node_grants() to assemble a list of
- * "grant IDs" for the user. This list is compared against the table.
- * If any row contains the node ID in question (or 0, which stands for "all
- * nodes"), one of the grant IDs returned, and a value of TRUE for the
- * operation in question, then access is granted. Note that this table is a
- * list of grants; any matching row is sufficient to grant access to the
- * node.
+ * unrestricted access to all nodes. user 1 will always pass this check.
+ *
+ * Next, all implementations of hook_node_access() will be called. Each
+ * implementation may explicitly allow, explicitly deny, or ignore the access
+ * request. If at least one module says to deny the request, it will be rejected.
+ * If no modules deny the request and at least one says to allow it, the request
+ * will be permitted.
+ *
+ * If all modules ignore the access request, then the node_access table is used
+ * to determine access. All node access modules are queried using
+ * hook_node_grants() to assemble a list of "grant IDs" for the user. This list
+ * is compared against the table. If any row contains the node ID in question
+ * (or 0, which stands for "all nodes"), one of the grant IDs returned, and a
+ * value of TRUE for the operation in question, then access is granted. Note
+ * that this table is a list of grants; any matching row is sufficient to
+ * grant access to the node.
  *
  * In node listings, the process above is followed except that
- * hook_access() is not called on each node for performance reasons and for
+ * hook_node_access() is not called on each node for performance reasons and for
  * proper functioning of the pager system. When adding a node listing to your
- * module, be sure to use db_rewrite_sql() to add
- * the appropriate clauses to your query for access checks.
+ * module, be sure to use a dynamic query created by db_select() and add a tag
+ * of "node_access" to ensure that only nodes to which the user has access
+ * are retrieved.
+ *
+ * Note: Even a single module returning NODE_ACCESS_DENY from hook_node_access()
+ * will block access to the node. Therefore, implementers should take care to
+ * not deny access unless they really intend to. Unless a module wishes to
+ * actively deny access it should return NODE_ACCESS_IGNORE (or simply return
+ * nothing) to allow other modules or the node_access table to control access.
  *
  * To see how to write a node access module of your own, see
  * node_access_example.module.
@@ -2292,12 +2314,17 @@ function node_access($op, $node, $account = NULL) {
     return FALSE;
   }
 
-  // Can't use node_invoke('access', $node), because the access hook takes the
-  // $op parameter before the $node parameter.
-  $base = node_type_get_base($node);
-  $access = module_invoke($base, 'access', $op, $node, $account);
-  if (!is_null($access)) {
-    return $access;
+  // We grant access to the node if both of the following conditions are met:
+  // - No modules say to deny access.
+  // - At least one module says to grant access.
+  // If no module specified either allow or deny, we fall back to the
+  // node_access table.
+  $access = module_invoke_all('node_access', $node, $op, $account);
+  if (in_array(NODE_ACCESS_DENY, $access, TRUE)) {
+    return FALSE;
+  }
+  elseif (in_array(NODE_ACCESS_ALLOW, $access, TRUE)) {
+    return TRUE;
   }
 
   // Check if authors can view their own unpublished nodes.
@@ -2338,6 +2365,99 @@ function node_access($op, $node, $account = NULL) {
 }
 
 /**
+ * Implement hook_node_access().
+ */
+function node_node_access($node, $op, $account) {
+  $type = is_string($node) ? $node : (is_array($node) ? $node['type'] : $node->type);
+
+  if (in_array($type, node_permissions_get_configured_types())) {
+    if ($op == 'create' && user_access('create ' . $type . ' content', $account)) {
+      return NODE_ACCESS_ALLOW;
+    }
+
+    if ($op == 'update') {
+      if (user_access('edit any ' . $type . ' content', $account) || (user_access('edit own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
+        return NODE_ACCESS_ALLOW;
+      }
+    }
+
+    if ($op == 'delete') {
+      if (user_access('delete any ' . $type . ' content', $account) || (user_access('delete own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
+        return NODE_ACCESS_ALLOW;
+      }
+    }
+  }
+
+  return NODE_ACCESS_IGNORE;
+}
+
+/**
+ * Helper function to generate standard node permission list for a given type.
+ *
+ * @param $type
+ *   The machine-readable name of the node type.
+ * @return array
+ *   An array of permission names and descriptions.
+ */
+function node_list_permissions($type) {
+  $info = node_type_get_type($type);
+  $type = check_plain($info->type);
+
+  // Build standard list of node permissions for this type.
+  $perms = array(
+    "create $type content" => array(
+      'title' => t('Create %type_name content', array('%type_name' => $info->name)),
+      'description' => t('Create new %type_name content.', array('%type_name' => $info->name)),
+    ),
+    "edit own $type content" => array(
+      'title' => t('Edit own %type_name content', array('%type_name' => $info->name)),
+      'description' => t('Edit %type_name content created by the user.', array('%type_name' => $info->name)),
+    ),
+    "edit any $type content" => array(
+      'title' => t('Edit any %type_name content', array('%type_name' => $info->name)),
+      'description' => t('Edit any %type_name content, regardless of its author.', array('%type_name' => $info->name)),
+    ),
+    "delete own $type content" => array(
+      'title' => t('Delete own %type_name content', array('%type_name' => $info->name)),
+      'description' => t('Delete %type_name content created by the user.', array('%type_name' => $info->name)),
+    ),
+    "delete any $type content" => array(
+      'title' => t('Delete any %type_name content', array('%type_name' => $info->name)),
+      'description' => t('Delete any %type_name content, regardless of its author.', array('%type_name' => $info->name)),
+    ),
+  );
+
+  return $perms;
+}
+
+/**
+ * Returns an array of node types that should be managed by permissions.
+ *
+ * By default, this will include all node types in the system. To exclude a
+ * specific node from getting permissions defined for it, set the
+ * node_permissions_$type variable to 0. Core does not provide an interface
+ * for doing so, however, contrib modules may exclude their own nodes in
+ * hook_install(). Alternatively, contrib modules may configure all node types
+ * at once, or decide to apply some other hook_node_access() implementation
+ * to some or all node types.
+ *
+ * @return
+ *   An array of node types managed by this module.
+ */
+function node_permissions_get_configured_types() {
+
+  $configured_types = array();
+
+  foreach (node_type_get_types() as $type => $info) {
+    if (variable_get('node_permissions_' . $type, 1)) {
+      $configured_types[] = $type;
+    }
+  }
+
+  return $configured_types;
+}
+
+/**
  * Generate an SQL join clause for use in fetching a node listing.
  *
  * @param $node_alias
@@ -2763,31 +2883,6 @@ function _node_access_rebuild_batch_finished($success, $results, $operations) {
  */
 
 /**
- * Implement hook_access().
- *
- * Named so as not to conflict with node_access()
- */
-function node_content_access($op, $node, $account) {
-  $type = is_string($node) ? $node : (is_array($node) ? $node['type'] : $node->type);
-
-  if ($op == 'create') {
-    return user_access('create ' . $type . ' content', $account);
-  }
-
-  if ($op == 'update') {
-    if (user_access('edit any ' . $type . ' content', $account) || (user_access('edit own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
-      return TRUE;
-    }
-  }
-
-  if ($op == 'delete') {
-    if (user_access('delete any ' . $type . ' content', $account) || (user_access('delete own ' . $type . ' content', $account) && ($account->uid == $node->uid))) {
-      return TRUE;
-    }
-  }
-}
-
-/**
  * Implement hook_form().
  */
 function node_content_form($node, $form_state) {
@@ -3121,45 +3216,6 @@ function node_unpublish_by_keyword_action($node, $context) {
 }
 
 /**
- * Helper function to generate standard node permission list for a given type.
- *
- * @param $type
- *   The machine-readable name of the node type.
- * @return array
- *   An array of permission names and descriptions.
- */
-function node_list_permissions($type) {
-  $info = node_type_get_type($type);
-  $type = check_plain($info->type);
-
-  // Build standard list of node permissions for this type.
-  $perms = array(
-    "create $type content" => array(
-      'title' => t('Create %type_name content', array('%type_name' => $info->name)),
-      'description' => t('Create new %type_name content.', array('%type_name' => $info->name)),
-    ),
-    "edit own $type content" => array(
-      'title' => t('Edit own %type_name content', array('%type_name' => $info->name)),
-      'description' => t('Edit %type_name content created by the user.', array('%type_name' => $info->name)),
-    ),
-    "edit any $type content" => array(
-      'title' => t('Edit any %type_name content', array('%type_name' => $info->name)),
-      'description' => t('Edit any %type_name content, regardless of its author.', array('%type_name' => $info->name)),
-    ),
-    "delete own $type content" => array(
-      'title' => t('Delete own %type_name content', array('%type_name' => $info->name)),
-      'description' => t('Delete %type_name content created by the user.', array('%type_name' => $info->name)),
-    ),
-    "delete any $type content" => array(
-      'title' => t('Delete any %type_name content', array('%type_name' => $info->name)),
-      'description' => t('Delete any %type_name content, regardless of its author.', array('%type_name' => $info->name)),
-    ),
-  );
-
-  return $perms;
-}
-
-/**
  * Implement hook_requirements().
  */
 function node_requirements($phase) {
diff --git a/modules/poll/poll.module b/modules/poll/poll.module
index bcd5d62b3..6c044b558 100644
--- a/modules/poll/poll.module
+++ b/modules/poll/poll.module
@@ -54,8 +54,7 @@ function poll_theme() {
  * Implement hook_permission().
  */
 function poll_permission() {
-  $perms = node_list_permissions('poll');
-  $perms += array(
+  $perms = array(
     'vote on polls' => array(
       'title' => t('Vote on polls'),
       'description' => t('Cast votes on polls.'),
@@ -74,20 +73,6 @@ function poll_permission() {
 }
 
 /**
- * Implement hook_access().
- */
-function poll_access($op, $node, $account) {
-  switch ($op) {
-    case 'create':
-      return user_access('create poll content', $account);
-    case 'update':
-      return user_access('edit any poll content', $account) || (user_access('edit own poll content', $account) && ($node->uid == $account->uid));
-    case 'delete':
-      return user_access('delete any poll content', $account) || (user_access('delete own poll content', $account) && ($node->uid == $account->uid));
-  }
-}
-
-/**
  * Implement hook_menu().
  */
 function poll_menu() {