diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index 79f2ecb..2397630 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -523,6 +523,7 @@ function comment_get_recent($number = 10) { $query = db_select('comment', 'c'); $query->innerJoin('node', 'n', 'n.nid = c.nid'); $query->addTag('node_access'); + $query->addMetaData('base_table', 'comment'); $comments = $query ->fields('c') ->condition('c.status', COMMENT_PUBLISHED) @@ -854,6 +855,7 @@ function comment_get_thread(Node $node, $mode, $comments_per_page) { ->condition('c.nid', $node->nid) ->addTag('node_access') ->addTag('comment_filter') + ->addMetaData('base_table', 'comment') ->addMetaData('node', $node) ->limit($comments_per_page); @@ -863,6 +865,7 @@ function comment_get_thread(Node $node, $mode, $comments_per_page) { ->condition('c.nid', $node->nid) ->addTag('node_access') ->addTag('comment_filter') + ->addMetaData('base_table', 'comment') ->addMetaData('node', $node); if (!user_access('administer comments')) { diff --git a/core/modules/forum/forum.module b/core/modules/forum/forum.module index 176cf71..3b96016 100644 --- a/core/modules/forum/forum.module +++ b/core/modules/forum/forum.module @@ -677,7 +677,8 @@ function forum_block_save($delta = '', $edit = array()) { function forum_block_view($delta = '') { $query = db_select('forum_index', 'f') ->fields('f') - ->addTag('node_access'); + ->addTag('node_access') + ->addMetaData('base_table', 'forum_index'); switch ($delta) { case 'active': $title = t('Active forum topics'); @@ -897,6 +898,7 @@ function forum_get_topics($tid, $sortby, $forum_per_page) { $query ->condition('f.tid', $tid) ->addTag('node_access') + ->addMetaData('base_table', 'forum_index') ->orderBy('f.sticky', 'DESC') ->orderByHeader($forum_topic_list_header) ->limit($forum_per_page); @@ -905,6 +907,7 @@ function forum_get_topics($tid, $sortby, $forum_per_page) { $count_query->condition('f.tid', $tid); $count_query->addExpression('COUNT(*)'); $count_query->addTag('node_access'); + $count_query->addMetaData('base_table', 'forum_index'); $query->setCountQuery($count_query); $result = $query->execute(); diff --git a/core/modules/forum/forum.test b/core/modules/forum/forum.test index 4561092..d916c14 100644 --- a/core/modules/forum/forum.test +++ b/core/modules/forum/forum.test @@ -653,3 +653,95 @@ class ForumIndexTestCase extends WebTestBase { $this->assertNoText($title, 'Unpublished forum topic no longer appears on index.'); } } + +/** + * Tests forum block view for private node access. + */ + +class ForumNodeAccessTestCase extends WebTestBase { + protected $access_user; + protected $admin_user; + protected $no_access_user; + + public static function getInfo() { + return array( + 'name' => 'Forum private node access test', + 'description' => 'Tests forum block view for private node access', + 'group' => 'Forum', + ); + } + + function setUp() { + $modules = array('node','comment','forum','taxonomy','tracker','node_access_test','block'); + parent::setUp($modules); + node_access_rebuild(); + variable_set('node_access_test_private', TRUE); + } + + /** + * Creates some users and creates a public node and a private node. + * Adds both Active forum topics and new forum topics blocks to the sidebar. + * Tests to ensure private node/public node access is respected on blocks. + */ + function testForumNodeAccess() { + // Create some users. + $access_user = $this->drupalCreateUser(array('node test view')); + $no_access_user = $this->drupalCreateUser(); + $admin_user = $this->drupalCreateUser(array('access administration pages', 'administer modules', 'administer blocks', 'create forum content')); + + $this->drupalLogin($admin_user); + + // Create a private node. + $langcode = LANGUAGE_NOT_SPECIFIED; + $private_node_title = $this->randomName(20); + $edit = array( + 'title' => $private_node_title, + "body[$langcode][0][value]" => $this->randomName(200), + 'private' => TRUE, + ); + $this->drupalPost('node/add/forum/1',$edit, t('Save')); + $private_node = $this->drupalGetNodeByTitle($private_node_title); + $this->assertTrue(!empty($private_node). 'New private forum node found in database.'); + + // Create a public node. + $public_node_title = $this->randomName(20); + $edit = array( + 'title' => $public_node_title, + "body[$langcode][0][value]" => $this->randomName(200), + ); + $this->drupalPost('node/add/forum/1',$edit, t('Save')); + $public_node = $this->drupalGetNodeByTitle($public_node_title); + $this->assertTrue(!empty($public_node). 'New public forum node found in database.'); + + // Enable the active forum block. + $edit = array(); + $edit['blocks[forum_active][region]'] = 'sidebar_second'; + $this->drupalPost('admin/structure/block', $edit, t('Save blocks')); + $this->assertResponse(200); + $this->assertText(t('The block settings have been updated.'), t('Active forum topics forum block was enabled')); + + // Enable the new forum block. + $edit = array(); + $edit['blocks[forum_new][region]'] = 'sidebar_second'; + $this->drupalPost('admin/structure/block', $edit, t('Save blocks')); + $this->assertResponse(200); + $this->assertText(t('The block settings have been updated.'), t('[New forum topics] Forum block was enabled')); + + // Test for $access_user. + $this->drupalLogin($access_user); + $this->drupalGet('/'); + + // Ensure private node and public node are found. + $this->assertText($private_node->title, 'Private node found in block by $access_user'); + $this->assertText($public_node->title, 'Public node found in block by $access_user'); + + // Test for $no_access_user. + $this->drupalLogin($no_access_user); + $this->drupalGet('/'); + + // Ensure private node is not found but public is found. + $this->assertNoText($private_node->title, 'Private node not found in block by $no_access_user'); + $this->assertText($public_node->title, 'Public node found in block by $no_access_user'); + } +} + diff --git a/core/modules/node/node.module b/core/modules/node/node.module index 8dbc060..a5312aa 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -3142,10 +3142,9 @@ function _node_query_node_access_alter($query, $type) { $tables = $query->getTables(); $base_table = $query->getMetaData('base_table'); - // If no base table is specified explicitly, search for one. + // If the base table is not given, default to node if present. if (!$base_table) { - $fallback = ''; - foreach ($tables as $alias => $table_info) { + foreach ($tables as $table_info) { if (!($table_info instanceof SelectInterface)) { $table = $table_info['table']; // If the node table is in the query, it wins immediately. @@ -3153,38 +3152,11 @@ function _node_query_node_access_alter($query, $type) { $base_table = $table; break; } - // Check whether the table has a foreign key to node.nid. If it does, - // do not run this check again as we found a base table and only node - // can triumph that. - if (!$base_table) { - // The schema is cached. - $schema = drupal_get_schema($table); - if (isset($schema['fields']['nid'])) { - if (isset($schema['foreign keys'])) { - foreach ($schema['foreign keys'] as $relation) { - if ($relation['table'] === 'node' && $relation['columns'] === array('nid' => 'nid')) { - $base_table = $table; - } - } - } - else { - // At least it's a nid. A table with a field called nid is very - // very likely to be a node.nid in a node access query. - $fallback = $table; - } - } - } } } - // If there is nothing else, use the fallback. + // Bail out if the base table is missing. if (!$base_table) { - if ($fallback) { - watchdog('security', 'Your node listing query is using @fallback as a base table in a query tagged for node access. This might not be secure and might not even work. Specify foreign keys in your schema to node.nid ', array('@fallback' => $fallback), WATCHDOG_WARNING); - $base_table = $fallback; - } - else { - throw new Exception(t('Query tagged for node access but there is no nid. Add foreign keys to node.nid in schema to fix.')); - } + throw new Exception(t('Query tagged for node access but there is no node table, specify the base_table using meta data.')); } } diff --git a/core/modules/taxonomy/taxonomy.module b/core/modules/taxonomy/taxonomy.module index 024f3ed..b6b1cf3 100644 --- a/core/modules/taxonomy/taxonomy.module +++ b/core/modules/taxonomy/taxonomy.module @@ -226,6 +226,7 @@ function taxonomy_select_nodes($tid, $pager = TRUE, $limit = FALSE, $order = arr } $query = db_select('taxonomy_index', 't'); $query->addTag('node_access'); + $query->addMetaData('base_table', 'taxonomy_index'); $query->condition('tid', $tid); if ($pager) { $count_query = clone $query; diff --git a/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerNodeAccessTest.php b/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerNodeAccessTest.php new file mode 100644 index 0000000..bf8b5b9 --- /dev/null +++ b/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerNodeAccessTest.php @@ -0,0 +1,70 @@ + 'Tracker Node Access Tests', + 'description' => 'Tests for private node access on /tracker.', + 'group' => 'Tracker', + ); + } + + public function setUp() { + $modules = array('node','comment','tracker','node_access_test'); + parent::setUp($modules); + node_access_rebuild(); + variable_set('node_access_test_private', TRUE); + } + + + /** + * Ensures private nodes are only visible to users + * with permission on /tracker. + */ + function testTrackerNodeAccess() { + // Create user with node test view permission. + $access_user = $this->drupalCreateUser(array('node test view')); + + // Create user without node test view permission. + $no_access_user = $this->drupalCreateuser(); + + $this->drupalLogin($access_user); + + // Create some nodes. + $private_node = $this->drupalCreateNode(array( + 'title' => t('Private node test'), + 'private'=> TRUE, + )); + $public_node = $this->drupalCreateNode(array( + 'title' => t('Public node test'), + 'private'=>FALSE, + )); + + // User with access should see both nodes created. + $this->drupalGet('tracker'); + $this->assertText($private_node->title, 'Private node is visible to user with private access.'); + $this->assertText($public_node->title, 'Public node is visible to user with private access.'); + + // User without access should not see private node. + $this->drupalLogin($no_access_user); + $this->drupalGet('tracker'); + $this->assertNoText($private_node->title, 'Private node is not visible to user without private access.'); + $this->assertText($public_node->title, 'Public node is visible to user without private access.'); + } +} + diff --git a/core/modules/tracker/tracker.pages.inc b/core/modules/tracker/tracker.pages.inc index 30583be..c917cc8 100644 --- a/core/modules/tracker/tracker.pages.inc +++ b/core/modules/tracker/tracker.pages.inc @@ -37,6 +37,7 @@ function tracker_page($account = NULL, $set_title = FALSE) { // while keeping the correct order. $nodes = $query ->addTag('node_access') + ->addMetaData('base_table', 'tracker_node') ->fields('t', array('nid', 'changed')) ->condition('t.published', 1) ->orderBy('t.changed', 'DESC')