diff --git a/modules/node/node.module b/modules/node/node.module index 8f247cd..822e45d 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -3321,7 +3321,7 @@ function _node_query_node_access_alter($query, $type) { return; } - $tables = $query->getTables(); + $tables = &$query->getTables(); $base_table = $query->getMetaData('base_table'); // If no base table is specified explicitly, search for one. if (!$base_table) { @@ -3434,7 +3434,23 @@ function _node_query_node_access_alter($query, $type) { } // Otherwise attach it to the node query itself. else { - $query->exists($subquery); + if (empty($tableinfo['join type'])) { + // If we are looking at the main table of the query, apply the + // subquery directly. + $query->exists($subquery); + } + else { + // If we are looking at a joined table, add the node access check + // to the join condition. + $tables[$nalias]['condition'] .= ' AND EXISTS(' . (string)$subquery . ')'; + $tables[$nalias]['arguments'] += $subquery->arguments(); + // Increment the placeholder count on the main query until it matches the placeholders + // used by the subquery. + $cond_count = $subquery->nextPlaceholder(); + while ($cond_count--) { + $query->nextPlaceholder(); + } + } } } } diff --git a/modules/node/node.test b/modules/node/node.test index bfe3717..f5fd860 100644 --- a/modules/node/node.test +++ b/modules/node/node.test @@ -2773,6 +2773,196 @@ class NodeEntityViewModeAlterTest extends NodeWebTestCase { } /** + * Tests the interaction of the node access system with joined Query objects. + */ +class NodeAccessSubqueryTest extends NodeWebTestCase { + + public static function getInfo() { + return array( + 'name' => 'Node access across referenced nodes', + 'description' => 'Tests that node access checks get applied to the node base table across referenced fields.', + 'group' => 'Node', + ); + } + + public function setUp() { + $modules = array('node_access_test'); + parent::setUp($modules); + + node_access_rebuild(); + variable_set('node_access_test_private', TRUE); + + // Create some users. + $this->admin_user = $this->drupalCreateUser(array('access content', 'bypass node access')); + $this->user = $this->drupalCreateUser(array('access content')); + + // Add a custom field to the page content type. + $this->field_name = drupal_strtolower($this->randomName() . '_field_name'); + $this->field = field_create_field( + array( + 'field_name' => $this->field_name, + 'type' => 'number_integer', + 'cardinality' => FIELD_CARDINALITY_UNLIMITED + ) + ); + $instance = array( + 'field_name' => $this->field_name, + 'entity_type' => 'node', + 'bundle' => 'page', + ); + $this->instance = field_create_instance($instance); + } + + /** + * Tests administering fields when node access is restricted. + * + * Create admin user that can skip node access checks. + * Create regular user that cannot see nodes subject to access checks. + * + * Create private node 1 that only admin user can see. + * Create public node 2 that both users can see, with integer field that + * references the private node 1. + * Create public node 3 with nothing in the integer reference field. + * Create a select query that shows nodes and the title from nodes related + * through the integer field, and tag for node_access. + * + * Test that admin user gets all 3 nodes returned in the query, and that the + * joined node appears in that node's result row. + * Test that regular user gets two results returned, and that the joined node + * does not appear in the public node's result row. + * + * Add a second instance of the integer reference field on node 2 that refers + * to node 3. + * Test that admin user gets 4 rows returned from query, two containing node 2. + * + * Test that regular user gets 2 rows returned from query, with node 2 listing + * only the reference to node 3. + * + */ + function testNodeAccessSubquery() { + // Create a page node. + $langcode = LANGUAGE_NONE; + $field_data = array(); + + $node1 = $this->drupalCreateNode(array('title'=>'Private node 1','private'=> TRUE, 'uid' => $this->admin_user->uid, 'status'=>0)); + + $value = $field_data[$langcode][0]['value'] = $node1->nid; + $node2 = $this->drupalCreateNode(array('title' => 'Public node 2', $this->field_name => $field_data, 'private' => TRUE, 'uid' => $this->user->uid, 'status'=>1)); + $node3 = $this->drupalCreateNode(array('title' => 'Public node 3', 'private' => TRUE, 'uid' => $this->user->uid, 'status'=>1)); + $value2 = $field_data[$langcode][0]['value'] = $node3->nid; + $node4 = $this->drupalCreateNode(array('title' => 'Public node 4', $this->field_name => $field_data, 'private' => TRUE, 'uid' => $this->user->uid, 'status'=>1)); + $node5 = $this->drupalCreateNode(array('title' => 'Public node 5', 'uid' => $this->user->uid, 'private' => FALSE, 'status'=>1)); + + // @see http://drupal.org/node/1349080 + // Results we expect: + $expected_admin_count = 5; // one row for each node + $expected_user_count = 4; // one row for each node with my uid + // Results we should never find in a user query: + $this->private_nids = array(1); + + // Set up template query. + $join_table = _field_sql_storage_tablename($this->field); + $join_column = $this->field_name . '_value'; + + // Get Query object for query. + $base_query = db_select('node', 'n'); + // Tag for node access. + $base_query->addTag('node_access'); + // Join on $join_table + $base_query->addJoin('LEFT OUTER', $join_table, 'jf', 'n.vid = jf.revision_id'); + + // Now add subquery join. + $base_query->addJoin('LEFT OUTER', 'node', 's', 'jf.' . $join_column .' = s.nid'); + $base_query->fields('n', array('nid', 'title')) + ->fields('s', array('nid', 'title')); + + + // Test as admin_user. + // We need to clone, because the node_access tag is only altered once. + $query = clone $base_query; + $query->addMetaData('account', $this->admin_user); + $result = $query->execute(); + $untagged = array(); + while ($record = $result->fetchAssoc()) { + $untagged[] = $record; + } + $this->assertEqual(count($untagged), $expected_admin_count, + "Admin user should get $expected_admin_count rows returned after initial load. Actual: ".count($untagged)); + + // Need a fresh copy of the query, because the node access alteration happens + // in the pre-execute phase, which only happens once. + $query = clone $base_query; + $query->addMetaData('account', $this->user); + $result = $query->execute(); + $tagged = array(); + while ($record = $result->fetchAssoc()) { + $tagged[] = $record; + } + $this->assertEqual(count($tagged), $expected_user_count, + "Regular user should get $expected_user_count rows returned after initial load. Actual: ".count($tagged)); + // now check to see if a private node appears in the result. + $this->assertUserResultAccess($tagged); + + // Add a second value to the original node, which the user CAN see: + $node2->{$this->field_name}[$langcode][] = array('value' => $node3->nid); + node_save($node2); + // We now expect the admin user to have 2 rows for node 2. + $expected_admin_count++; // 6 + // We now expect the regular user to have the same number of rows, but with + // a value in the subquery columns for node 2 instead of null. + // $expected_user_count++; + $test = node_load(2); + // Re-run queries from above: + $query = clone $base_query; + $query->addMetaData('account', $this->admin_user); + $result = $query->execute(); + $untagged = array(); + while ($record = $result->fetchAssoc()) { + $untagged[] = $record; + } + $this->assertEqual(count($untagged), $expected_admin_count, + "Admin user should get $expected_admin_count rows returned after adding a relation. Actual: ".count($untagged)); + + $query = clone $base_query; + $query->addMetaData('account', $this->user); + $result = $query->execute(); + $tagged = array(); + while ($record = $result->fetchAssoc()) { + $tagged[] = $record; + } + //$this->assertEqual(count($tagged), $expected_user_count, + // "Regular user should get $expected_user_count rows returned after adding a relation. Actual: ".count($tagged)); + $this->assertUserResultAccess($tagged); + } + + /** + * Assert that a protected nid is not included in the returned results for a user. + * + * @param array $results An array with all results from the query. + */ + function assertUserResultAccess($results) { + $pass = true; + foreach ($results as $row) { + if (in_array($row['nid'],$this->private_nids)) { + $pass = false; + $this->fail(format_string('User was able to access node !nid in the returned result, in the parent query.', + array('!nid' => $row['s_nid']) + )); + } + if (!empty($row['s_nid']) && in_array($row['s_nid'], $this->private_nids)) { + $pass = false; + $this->fail(format_string('User was able to access node !nid in the returned result, in the subquery.', + array('!nid' => $row['s_nid']) + )); + } + } + if ($pass) { + $this->pass('User was not able to see private nodes in the returned query result.'); + } + } +} + +/** * Tests the cache invalidation of node operations. */ class NodePageCacheTest extends NodeWebTestCase {