diff --git a/modules/node/node.module b/modules/node/node.module index 20fc37e..66e93c7 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -3110,7 +3110,7 @@ function node_access_view_all_nodes($account = NULL) { * 'update' and 'delete'). */ function node_query_node_access_alter(QueryAlterableInterface $query) { - _node_query_node_access_alter($query, 'node', 'node'); + _node_query_node_access_alter($query, 'node'); } /** @@ -3121,7 +3121,7 @@ function node_query_node_access_alter(QueryAlterableInterface $query) { * conditions are added for field values belonging to nodes only. */ function node_query_entity_field_access_alter(QueryAlterableInterface $query) { - _node_query_node_access_alter($query, $query->getMetaData('base_table'), 'entity'); + _node_query_node_access_alter($query, 'entity'); } /** @@ -3129,14 +3129,12 @@ function node_query_entity_field_access_alter(QueryAlterableInterface $query) { * * @param $query * The query to add conditions to. - * @param $base_table - * The table holding node ids. * @param $type * Either 'node' or 'entity' depending on what sort of query it is. See * node_query_node_access_alter() and node_query_entity_field_access_alter() * for more. */ -function _node_query_node_access_alter($query, $base_table, $type) { +function _node_query_node_access_alter($query, $type) { global $user; // Read meta-data from query, if provided. @@ -3160,14 +3158,61 @@ function _node_query_node_access_alter($query, $base_table, $type) { return; } + $tables = $query->getTables(); + $base_table = $query->getMetaData('base_table'); + // If no base table is specified explicitly, search for one. + if (!$base_table) { + $fallback = ''; + foreach ($tables as $alias => $table_info) { + if (!($table_info instanceof SelectQueryInterface)) { + $table = $table_info['table']; + // If the node table is in the query, it wins immediately. + if ($table == 'node') { + $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. + 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.')); + } + } + } + // Prevent duplicate records. $query->distinct(); - // Find all instances of the {node} table being joined -- could appear + // Find all instances of the base table being joined -- could appear // more than once in the query, and could be aliased. Join each one to // the node_access table. - $tables = $query->getTables(); $grants = node_access_grants($op, $account); if ($type == 'entity') { // The original query looked something like: diff --git a/modules/node/node.test b/modules/node/node.test index 8a871c0..56a2d34 100644 --- a/modules/node/node.test +++ b/modules/node/node.test @@ -969,6 +969,156 @@ class NodeAccessRecordsUnitTest extends DrupalWebTestCase { } /** + * Tests for Node Access with a non-node base table. + */ +class NodeAccessBaseTableTestCase extends DrupalWebTestCase { + + public static function getInfo() { + return array( + 'name' => 'Node Access on any table', + 'description' => 'Checks behavior of the node access subsystem if the base table is not node.', + 'group' => 'Node', + ); + } + + /** + * Enable modules and create user with specific permissions. + */ + public function setUp() { + parent::setUp('node_access_test'); + node_access_rebuild(); + variable_set('node_access_test_private', TRUE); + } + + /** + * Test the "private" node access. + * + * - Create 2 users with "access content" and "create article" permissions. + * - Each user creates one private and one not private article. + + * - Test that each user can view the other user's non-private article. + * - Test that each user cannot view the other user's private article. + * - Test that each user finds only appropriate (non-private + own private) + * in taxonomy listing. + * - Create another user with 'view any private content'. + * - Test that user 4 can view all content created above. + * - Test that user 4 can view all content on taxonomy listing. + */ + function testNodeAccessBasic() { + $num_simple_users = 2; + $simple_users = array(); + + // nodes keyed by uid and nid: $nodes[$uid][$nid] = $is_private; + $this->nodesByUser = array(); + $titles = array(); // Titles keyed by nid + $private_nodes = array(); // Array of nids marked private. + for ($i = 0; $i < $num_simple_users; $i++) { + $simple_users[$i] = $this->drupalCreateUser(array('access content', 'create article content')); + } + foreach ($simple_users as $this->webUser) { + $this->drupalLogin($this->webUser); + foreach (array(0 => 'Public', 1 => 'Private') as $is_private => $type) { + $edit = array( + 'title' => t('@private_public Article created by @user', array('@private_public' => $type, '@user' => $this->webUser->name)), + ); + if ($is_private) { + $edit['private'] = TRUE; + $edit['body[und][0][value]'] = 'private node'; + $edit['field_tags[und]'] = 'private'; + } + else { + $edit['body[und][0][value]'] = 'public node'; + $edit['field_tags[und]'] = 'public'; + } + + $this->drupalPost('node/add/article', $edit, t('Save')); + $nid = db_query('SELECT nid FROM {node} WHERE title = :title', array(':title' => $edit['title']))->fetchField(); + $private_status = db_query('SELECT private FROM {node_access_test} where nid = :nid', array(':nid' => $nid))->fetchField(); + $this->assertTrue($is_private == $private_status, t('The private status of the node was properly set in the node_access_test table.')); + if ($is_private) { + $private_nodes[] = $nid; + } + $titles[$nid] = $edit['title']; + $this->nodesByUser[$this->webUser->uid][$nid] = $is_private; + } + } + $this->publicTid = db_query('SELECT tid FROM {taxonomy_term_data} WHERE name = :name', array(':name' => 'public'))->fetchField(); + $this->privateTid = db_query('SELECT tid FROM {taxonomy_term_data} WHERE name = :name', array(':name' => 'private'))->fetchField(); + $this->assertTrue($this->publicTid, t('Public tid was found')); + $this->assertTrue($this->privateTid, t('Private tid was found')); + foreach ($simple_users as $this->webUser) { + $this->drupalLogin($this->webUser); + // Check own nodes to see that all are readable. + foreach ($this->nodesByUser as $uid => $data) { + foreach ($data as $nid => $is_private) { + $this->drupalGet('node/' . $nid); + if ($is_private) { + $should_be_visible = $uid == $this->webUser->uid; + } + else { + $should_be_visible = TRUE; + } + $this->assertResponse($should_be_visible ? 200 : 403, strtr('A %private node by user %uid is %visible for user %current_uid.', array( + '%private' => $is_private ? 'private' : 'public', + '%uid' => $uid, + '%visible' => $should_be_visible ? 'visible' : 'not visible', + '%current_uid' => $this->webUser->uid, + ))); + } + } + + // Check to see that the correct nodes are shown on taxonomy/private + // and taxonomy/public. + $this->assertTaxonomyPage(FALSE); + } + + // Now test that a user with 'access any private content' can view content. + $access_user = $this->drupalCreateUser(array('access content', 'create article content', 'node test view', 'search content')); + $this->drupalLogin($access_user); + + foreach ($this->nodesByUser as $uid => $private_status) { + foreach ($private_status as $nid => $is_private) { + $this->drupalGet('node/' . $nid); + $this->assertResponse(200); + } + } + + // This user should be able to see all of the nodes on the relevant + // taxonomy pages. + $this->assertTaxonomyPage(TRUE); + } + + protected function assertTaxonomyPage($super) { + foreach (array($this->publicTid, $this->privateTid) as $tid_is_private => $tid) { + $this->drupalGet("taxonomy/term/$tid"); + $this->nids_visible = array(); + foreach ($this->xpath("//a[text()='Read more']") as $link) { + $this->assertTrue(preg_match('|node/(\d+)$|', (string) $link['href'], $matches), 'Read more points to a node'); + $this->nids_visible[$matches[1]] = TRUE; + } + foreach ($this->nodesByUser as $uid => $data) { + foreach ($data as $nid => $is_private) { + // Private nodes should be visible on the private term page, + // public nodes should be visible on the public term page. + $should_be_visible = $tid_is_private == $is_private; + // Non-superusers on the private page can only see their own nodes. + if (!$super && $tid_is_private) { + $should_be_visible = $should_be_visible && $uid == $this->webUser->uid; + } + $this->assertIdentical(isset($this->nids_visible[$nid]), $should_be_visible, strtr('A %private node by user %uid is %visible for user %current_uid on the %tid_is_private page.', array( + '%private' => $is_private ? 'private' : 'public', + '%uid' => $uid, + '%visible' => isset($this->nids_visible[$nid]) ? 'visible' : 'not visible', + '%current_uid' => $this->webUser->uid, + '%tid_is_private' => $tid_is_private ? 'private' : 'public', + ))); + } + } + } + } +} + +/** * Test case to check node save related functionality, including import-save */ class NodeSaveTestCase extends DrupalWebTestCase { diff --git a/modules/node/tests/node_access_test.install b/modules/node/tests/node_access_test.install index e69de29..6b3ef5d 100644 --- a/modules/node/tests/node_access_test.install +++ b/modules/node/tests/node_access_test.install @@ -0,0 +1,42 @@ + 'The base table for node_access_test.', + 'fields' => array( + 'nid' => array( + 'description' => 'The {node}.nid this record affects.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + 'default' => 0, + ), + 'private' => array( + 'description' => 'Boolean indicating whether the node is private (visible to administrator) or not (visible to non-administrators).', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + ), + 'indexes' => array( + 'nid' => array('nid'), + ), + 'primary key' => array('nid'), + 'foreign keys' => array( + 'versioned_node' => array( + 'table' => 'node', + 'columns' => array('nid' => 'nid'), + ), + ), + ); + + return $schema; +} \ No newline at end of file diff --git a/modules/node/tests/node_access_test.module b/modules/node/tests/node_access_test.module index 3004e0c..91c117a 100644 --- a/modules/node/tests/node_access_test.module +++ b/modules/node/tests/node_access_test.module @@ -12,8 +12,10 @@ */ function node_access_test_node_grants($account, $op) { $grants = array(); + // First grant a grant to the author for own content. + $grants['node_access_test_author'] = array($account->uid); if ($op == 'view' && user_access('node test view', $account)) { - $grants['node_access_test'] = array(888); + $grants['node_access_test'] = array(8888); } if ($op == 'view' && $account->uid == variable_get('node_test_node_access_all_uid', 0)) { $grants['node_access_all'] = array(0); @@ -26,14 +28,27 @@ function node_access_test_node_grants($account, $op) { */ function node_access_test_node_access_records($node) { $grants = array(); - $grants[] = array( - 'realm' => 'node_access_test', - 'gid' => 888, - 'grant_view' => 1, - 'grant_update' => 0, - 'grant_delete' => 0, - 'priority' => 999, + // For NodeAccessBaseTableTestCase, only set records for private nodes. + if (!variable_get('node_access_test_private') || $node->private) { + $grants[] = array( + 'realm' => 'node_access_test', + 'gid' => 8888, + 'grant_view' => 1, + 'grant_update' => 0, + 'grant_delete' => 0, + 'priority' => 0, ); + // For the author realm, the GID is equivalent to a UID, which + // means there are many many groups of just 1 user. + $grants[] = array( + 'realm' => 'node_access_test_author', + 'gid' => $node->uid, + 'grant_view' => 1, + 'grant_update' => 1, + 'grant_delete' => 1, + 'priority' => 0, + ); + } return $grants; } @@ -142,3 +157,62 @@ function node_access_entity_test_page() { return $output; } + +/** + * Implements hook_form_node_form_alter(). + */ +function node_access_test_form_node_form_alter(&$form, $form_state) { + // Only show this checkbox for NodeAccessBaseTableTestCase. + if (variable_get('node_access_test_private')) { + $form['private'] = array( + '#type' => 'checkbox', + '#title' => t('Private'), + '#description' => t('Check here if this content should be set private and only shown to privileged users.'), + '#default_value' => isset($form['#node']->private) ? $form['#node']->private : FALSE, + ); + } +} + +/** + * Implements hook_node_load(). + */ +function node_access_test_node_load($nodes, $types) { + $result = db_query('SELECT nid, private FROM {node_access_test} WHERE nid IN(:nids)', array(':nids' => array_keys($nodes))); + foreach ($result as $record) { + $nodes[$record->nid]->private = $record->private; + } +} + +/** + * Implements hook_node_delete(). + */ + +function node_access_test_node_delete($node) { + db_delete('node_access_test')->condition('nid', $node->nid)->execute(); +} + +/** + * Implements hook_node_insert(). + */ +function node_access_test_node_insert($node) { + _node_access_test_node_write($node); +} + +/** + * Implements hook_nodeapi_update(). + */ +function node_access_test_node_update($node) { + _node_access_test_node_write($node); +} + +/** + * Helper for node insert/update. + */ +function _node_access_test_node_write($node) { + if (isset($node->private)) { + db_merge('node_access_test') + ->key(array('nid' => $node->nid)) + ->fields(array('private' => (int) $node->private)) + ->execute(); + } +}