#963656 by agentrickard, bfroehle: Fixed critical bug node_access_view_all_nodes() is never invoked.

3 files changed, 83 insertions, 2 deletions

diff --git a/modules/node/node.module b/modules/node/node.module
index 2097a915a..252d72278 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -3005,13 +3005,30 @@ function node_access_grants($op, $account = NULL) {
 }
 
 /**
- * Determine whether the user has a global viewing grant for all nodes.
+ * Determines whether the user has a global viewing grant for all nodes.
+ *
+ * Checks to see whether any module grants 'view' for nid = 0. The node module
+ * provides this record if no node access modules are enabled. Other modules
+ * can replicate this behavior by providing their own conditional grant for
+ * nid = 0. For example, hook_node_grants() can return the following array to
+ * give the 'view' privilege to all nodes:
+ * @code
+ * if ($op == 'view') {
+ *   $grants['example_realm'] = array(0);
+ * }
+ * @endcode
+ *
+ * @return
+ *   TRUE if 'view' access to all nodes is granted, FALSE otherwise.
+ *
+ * @see hook_node_grants()
+ * @see _node_query_node_access_alter()
  */
 function node_access_view_all_nodes() {
   $access = &drupal_static(__FUNCTION__);
 
   if (!isset($access)) {
-    // If no modules implement the node access system, access is always true.
+    // If no modules implement the node access system, access is always TRUE.
     if (!module_implements('node_grants')) {
       $access = TRUE;
     }
@@ -3099,6 +3116,10 @@ function _node_query_node_access_alter($query, $base_table, $type) {
   if (!count(module_implements('node_grants'))) {
     return;
   }
+  // If viewing nodes, make sure access rules should be enforced.
+  if ($op == 'view' && node_access_view_all_nodes()) {
+    return;
+  }
 
   // Prevent duplicate records.
   $query->distinct();
diff --git a/modules/node/node.test b/modules/node/node.test
index 66d6bf757..975728a82 100644
--- a/modules/node/node.test
+++ b/modules/node/node.test
@@ -1801,6 +1801,63 @@ class NodeQueryAlter extends DrupalWebTestCase {
       $this->fail(t('Altered query is malformed'));
     }
   }
+
+  /**
+   * Lower-level test of 'node_access' query alter override.
+   *
+   * Verifies that node_access_view_all_nodes() is called from
+   * node_query_node_access_alter().  We do this by checking that
+   * a user which normally would not have view privileges is able
+   * to view the nodes when we add a record to {node_access} paired
+   * with a corresponding privilege in hook_node_grants().
+   */
+  function testNodeQueryAlterOverride() {
+    $record = array(
+      'nid' => 0,
+      'gid' => 0,
+      'realm' => 'node_access_all',
+      'grant_view' => 1,
+      'grant_update' => 0,
+      'grant_delete' => 0,
+    );
+    drupal_write_record('node_access', $record);
+
+    // Test that the noAccessUser still doesn't have the 'view'
+    // privilege after adding the node_access record.
+    drupal_static_reset('node_access_view_all_nodes');
+    try {
+      $query = db_select('node', 'mytab')
+        ->fields('mytab');
+      $query->addTag('node_access');
+      $query->addMetaData('op', 'view');
+      $query->addMetaData('account', $this->noAccessUser);
+
+      $result = $query->execute()->fetchAll();
+      $this->assertEqual(count($result), 0, t('User view privileges are not overridden'));
+    }
+    catch (Exception $e) {
+      $this->fail(t('Altered query is malformed'));
+    }
+
+    // Have node_test_node_grants return a node_access_all privilege,
+    // to grant the noAccessUser 'view' access.
+    variable_set('node_test_node_access_all', 1);
+    drupal_static_reset('node_access_view_all_nodes');
+    try {
+      $query = db_select('node', 'mytab')
+        ->fields('mytab');
+      $query->addTag('node_access');
+      $query->addMetaData('op', 'view');
+      $query->addMetaData('account', $this->noAccessUser);
+
+      $result = $query->execute()->fetchAll();
+      $this->assertEqual(count($result), 4, t('User view privileges are overridden'));
+    }
+    catch (Exception $e) {
+      $this->fail(t('Altered query is malformed'));
+    }
+    variable_del('node_test_node_access_all');
+  }
 }
 
 
diff --git a/modules/node/tests/node_access_test.module b/modules/node/tests/node_access_test.module
index ac71667ef..33f7a01b4 100644
--- a/modules/node/tests/node_access_test.module
+++ b/modules/node/tests/node_access_test.module
@@ -16,6 +16,9 @@ function node_access_test_node_grants($account, $op) {
   if ($op == 'view' && user_access('node test view', $account)) {
     $grants['node_access_test'] = array(888);
   }
+  if ($op == 'view' && variable_get('node_test_node_access_all', 0)) {
+    $grants['node_access_all'] = array(0);
+  }
   return $grants;
 }