diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index 5684731..8d08fae 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -524,6 +524,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) @@ -856,6 +857,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); @@ -865,6 +867,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 25a4369..78f2e1a 100644 --- a/core/modules/forum/forum.module +++ b/core/modules/forum/forum.module @@ -678,7 +678,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'); @@ -900,6 +901,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); @@ -908,6 +910,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/lib/Drupal/forum/Tests/ForumTest.php b/core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php index 49f136d..5a35c08 100644 --- a/core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php +++ b/core/modules/forum/lib/Drupal/forum/Tests/ForumTest.php @@ -82,7 +82,7 @@ class ForumTest extends WebTestBase { } /** - * Login users, create forum nodes, and test forum functionality through the admin and user interfaces. + * Login users, create forum nodes, and test forum functionality. */ function testForum() { //Check that the basic forum install creates a default forum topic @@ -444,7 +444,7 @@ class ForumTest extends WebTestBase { $this->assertNoRaw(t('The item %title is a forum container, not a forum.', array('%title' => $forum['name'])), t('No error message was shown')); } - // Retrieve node object, ensure that the topic was created and in the proper forum. + // Retrieve node object, ensure topic was created and in the proper forum. $node = $this->drupalGetNodeByTitle($title); $this->assertTrue($node != NULL, t('Node @title was loaded', array('@title' => $title))); $this->assertEqual($node->taxonomy_forums[LANGUAGE_NOT_SPECIFIED][0]['tid'], $tid, 'Saved forum topic was in the expected forum'); @@ -593,3 +593,155 @@ class ForumTest extends WebTestBase { } } } + +/** + * Tests the forum index listing. + */ +class ForumIndexTestCase extends WebTestBase { + + public static function getInfo() { + return array( + 'name' => 'Forum index', + 'description' => 'Tests the forum index listing.', + 'group' => 'Forum', + ); + } + + function setUp() { + parent::setUp('taxonomy', 'comment', 'forum'); + + // Create a test user. + $web_user = $this->drupalCreateUser(array('create forum content', 'edit own forum content', 'edit any forum content', 'administer nodes')); + $this->drupalLogin($web_user); + } + + /** + * Tests the forum index for published and unpublished nodes. + */ + function testForumIndexStatus() { + + $langcode = LANGUAGE_NOT_SPECIFIED; + + // The forum ID to use. + $tid = 1; + + // Create a test node. + $title = $this->randomName(20); + $edit = array( + "title" => $title, + "body[$langcode][0][value]" => $this->randomName(200), + ); + + // Create the forum topic, preselecting the forum ID via a URL parameter. + $this->drupalPost('node/add/forum/' . $tid, $edit, t('Save')); + + // Check that the node exists in the database. + $node = $this->drupalGetNodeByTitle($title); + $this->assertTrue(!empty($node), 'New forum node found in database.'); + + // Verify that the node appears on the index. + $this->drupalGet('forum/' . $tid); + $this->assertText($title, 'Published forum topic appears on index.'); + + // Unpublish the node. + $edit = array( + 'status' => FALSE, + ); + $this->drupalPost("node/{$node->nid}/edit", $edit, t('Save')); + $this->assertText(t('Access denied'), 'Unpublished node is no longer accessible.'); + + // Verify that the node no longer appears on the index. + $this->drupalGet('forum/' . $tid); + $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() { + parent::setUp(array('node', 'comment', 'forum', 'taxonomy', 'tracker', 'node_access_test', 'block')); + 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.'), '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.'), '[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 671d0f0..b947c7c 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -3145,10 +3145,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. @@ -3156,38 +3155,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 30eba40..861fae3 100644 --- a/core/modules/taxonomy/taxonomy.module +++ b/core/modules/taxonomy/taxonomy.module @@ -228,6 +228,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..cb74f02 --- /dev/null +++ b/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerNodeAccessTest.php @@ -0,0 +1,67 @@ + 'Tracker Node Access Tests', + 'description' => 'Tests for private node access on /tracker.', + 'group' => 'Tracker', + ); + } + + public function setUp() { + parent::setUp(array('node', 'comment', 'tracker', 'node_access_test')); + node_access_rebuild(); + variable_set('node_access_test_private', TRUE); + } + + + /** + * Ensure private node on /tracker is only visible to users with permission. + */ + 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 3f9728d..87a4159 100644 --- a/core/modules/tracker/tracker.pages.inc +++ b/core/modules/tracker/tracker.pages.inc @@ -39,6 +39,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')