diff --git a/includes/database/select.inc b/includes/database/select.inc
index 3abd205..89fb82b 100644
--- a/includes/database/select.inc
+++ b/includes/database/select.inc
@@ -883,7 +883,7 @@ class SelectQuery extends Query implements SelectQueryInterface {
* 'type' => $join_type (one of INNER, LEFT OUTER, RIGHT OUTER),
* 'table' => $table,
* 'alias' => $alias_of_the_table,
- * 'condition' => $condition_clause_on_which_to_join,
+ * 'condition' => $join_condition (string or DatabaseCondition object).
* 'arguments' => $array_of_arguments_for_placeholders_in_the condition.
* 'all_fields' => TRUE to SELECT $alias.*, FALSE or NULL otherwise.
* )
@@ -891,6 +891,10 @@ class SelectQuery extends Query implements SelectQueryInterface {
* If $table is a string, it is taken as the name of a table. If it is
* a SelectQuery object, it is taken as a subquery.
*
+ * If $join_condition is a DatabaseCondition object, any arguments
+ * should be incorporated into the object; a separate array of arguments
+ * does not need to be provided.
+ *
* @var array
*/
protected $tables = array();
@@ -1028,6 +1032,10 @@ class SelectQuery extends Query implements SelectQueryInterface {
if ($table['table'] instanceof SelectQueryInterface) {
$args += $table['table']->arguments();
}
+ // If the join condition is an object, grab its arguments recursively.
+ if (!empty($table['condition']) && $table['condition'] instanceof QueryConditionInterface) {
+ $args += $table['condition']->arguments();
+ }
}
foreach ($this->expressions as $expression) {
@@ -1079,6 +1087,10 @@ class SelectQuery extends Query implements SelectQueryInterface {
if ($table['table'] instanceof SelectQueryInterface) {
$table['table']->compile($connection, $queryPlaceholder);
}
+ // Make sure join conditions are also compiled.
+ if (!empty($table['condition']) && $table['condition'] instanceof QueryConditionInterface) {
+ $table['condition']->compile($connection, $queryPlaceholder);
+ }
}
// If there are any dependent queries to UNION, compile it recursively.
@@ -1099,6 +1111,11 @@ class SelectQuery extends Query implements SelectQueryInterface {
return FALSE;
}
}
+ if (!empty($table['condition']) && $table['condition'] instanceof QueryConditionInterface) {
+ if (!$table['condition']->compiled()) {
+ return FALSE;
+ }
+ }
}
foreach ($this->union as $union) {
@@ -1543,7 +1560,7 @@ class SelectQuery extends Query implements SelectQueryInterface {
$query .= $table_string . ' ' . $this->connection->escapeTable($table['alias']);
if (!empty($table['condition'])) {
- $query .= ' ON ' . $table['condition'];
+ $query .= ' ON ' . (string) $table['condition'];
}
}
diff --git a/modules/node/node.module b/modules/node/node.module
index 1d88834..6cd0e7f 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -3341,7 +3341,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) {
@@ -3448,14 +3448,27 @@ function _node_query_node_access_alter($query, $type) {
}
$subquery->where("$nalias.$field = na.nid");
- // For an entity query, attach the subquery to entity conditions.
+ // For an entity query, attach the subquery to the entity conditions.
if ($type == 'entity') {
$node_conditions->exists($subquery);
}
// Otherwise attach it to the node query itself.
- else {
+ elseif (empty($tableinfo['join type'])) {
$query->exists($subquery);
}
+ else {
+ // If it's a join, add the node access check to the join condition.
+ $join_cond = db_and()->exists($subquery);
+ // Add the existing join conditions into the Condition object.
+ if ($tables[$nalias]['condition'] instanceof ConditionInterface) {
+ $join_cond->condition($tables[$nalias]['condition']);
+ }
+ else {
+ $join_cond->where($tables[$nalias]['condition'], $tables[$nalias]['arguments']);
+ $tables[$nalias]['arguments'] = array();
+ }
+ $tables[$nalias]['condition'] = $join_cond;
+ }
}
}
diff --git a/modules/node/node.test b/modules/node/node.test
index 4ffc88e..7b1f844 100644
--- a/modules/node/node.test
+++ b/modules/node/node.test
@@ -2912,6 +2912,258 @@ class NodeEntityViewModeAlterTest extends NodeWebTestCase {
}
}
+ /**
+ * Tests the interaction of the node access system with joined Query objects.
+ */
+class NodeAccessJoinTest extends NodeWebTestCase {
+
+ public static function getInfo() {
+ return array(
+ 'name' => 'Node access joins',
+ 'description' => 'Tests that node access checks get applied across left-joined tables.',
+ 'group' => 'Node',
+ );
+ }
+
+ public function setUp() {
+ $modules = array('node_access_test');
+ parent::setUp($modules);
+
+ node_access_rebuild();
+ variable_set('node_access_test_private', TRUE);
+
+ // Add a custom field to the page content type.
+ $this->field_name = 'related_article';
+ $this->field = field_create_field(
+ array(
+ 'field_name' => $this->field_name,
+ 'type' => 'number_integer',
+ 'cardinality' => FIELD_CARDINALITY_UNLIMITED
+ )
+ );
+ $page_instance = array(
+ 'field_name' => $this->field_name,
+ 'entity_type' => 'node',
+ 'bundle' => 'page',
+ );
+ field_create_instance($page_instance);
+ $article_instance = array(
+ 'field_name' => $this->field_name,
+ 'entity_type' => 'node',
+ 'bundle' => 'article',
+ );
+ field_create_instance($article_instance);
+ }
+
+ /**
+ * Tests the accessibility of joined nodes.
+ *
+ * - Create two users with "access content" and "create article" permissions
+ * who can each access their own private articles but not others'.
+ * - Create article-type nodes with and without references to other articles.
+ * The articles and references represent all possible combinations of the
+ * tested access types.
+ * - Create page-type nodes referencing each of the articles, as well as a
+ * page with no reference.
+ * - Use a custom view that creates two joins between nodes and has a
+ * node_access tag. The view lists the page nodes, the article
+ * referenced by each page node, and the article referenced by each
+ * article.
+ *
+ * - Login with the author user and check that he does not have access to
+ * private nodes created by other users. Test access using total row
+ * count as well as checking for presence of individual page titles.
+ * - Repeat tests using a user with only the "access content" permission,
+ * confirming this user does not have access to any private nodes.
+ * - Repeat tests using a user with "access content" and "node test view"
+ * permissions, confirming this user sees the complete view.
+ */
+ public function testNodeAccessJoin() {
+
+ // User to add articles and test author access.
+ $this->authorUser = $this->drupalCreateUser(['access content', 'create article content']);
+ // Another user to add articles (whose private articles can not be accessed
+ // by authorUser).
+ $this->otherUser = $this->drupalCreateUser(array('access content', 'create article content'));
+
+ // Create the articles.
+ $langcode = LANGUAGE_NONE;
+
+ // The articles are stored in an array keyed by $article and $reference2, where
+ // $article is the access type of the article itself, and $reference2 is the
+ // access type of the reference linked to by the article.
+ // 'public' articles are created by otherUser with private=0.
+ // 'private' articles are created by otherUser with private=1.
+ // 'author_public' articles are created by authorUser with private=0.
+ // 'author_private' articles are created by authorUser with private=1.
+ // 'no_reference' is used for references when there is no related article.
+ foreach (array('no_reference', 'public', 'private', 'author_public', 'author_private') as $reference2) {
+ foreach (array('public', 'private', 'author_public', 'author_private') as $article) {
+ $is_author = (substr($article, 0, 6) == 'author');
+ $is_private = (substr($article, -7) == 'private');
+ $edit = array(
+ 'type' => 'article',
+ 'uid' => $is_author ? $this->authorUser->uid : $this->otherUser->uid,
+ 'private' => $is_private,
+ );
+ // The article names provide the access status of the article and the
+ // access status of the related article (if any).
+ // The naming system ensures that the text 'Article $article' will only appear
+ // in the view if an article with that access type is displayed in the view. (The text
+ // '$article' alone will appear in the titles of other nodes that reference
+ // an article.)
+ $edit['title'] = "Article $article - $reference2";
+ if ($reference2 != 'no_reference') {
+ $edit['related_article'][$langcode][0]['value'] = $this->articles[$reference2]['no_reference'];
+ }
+ $node = $this->drupalCreateNode($edit);
+ $this->articles[$article][$reference2] = $node->nid;
+
+ $this->assertEqual((int) $is_private, (int) $node->private, 'The private status of the article node was properly set in the node_access_test table.');
+ if ($reference2 != 'no_reference') {
+ $this->assertEqual((int) $this->articles[$reference2]['no_reference'], (int) $node->related_article[$langcode][0]['value'], 'Proper article attached to article.');
+ }
+ }
+ }
+
+ // Add a blank 'no_reference' entry to the article list, so that a page with
+ // no reference gets created.
+ $this->articles['no_reference']['no_reference'] = NULL;
+
+ $total = 0;
+ $count_s_total = $count_s2_total = 0;
+ $count_s_public = $count_s2_public = 0;
+ $count_s_author = $count_s2_author = 0;
+ $total_public = $total_author = 0;
+
+ // Create page nodes referencing each article, as well as a page with no reference.
+ foreach ($this->articles as $reference => $list) {
+ foreach ($list as $reference2 => $article_nid) {
+ $title = "Page - $reference";
+ if ($reference != 'no_reference') {
+ $title .= " - $reference2";
+ }
+ $edit = array(
+ 'type' => 'page',
+ 'title' => $title,
+ 'private' => FALSE,
+ );
+ if ($article_nid) {
+ $edit['related_article'][$langcode][0]['value'] = $article_nid;
+ }
+ $node = $this->drupalCreateNode($edit);
+ if ($article_nid) {
+ $this->assertEqual((int) $article_nid, (int) $node->related_article[$langcode][0]['value'], 'Proper article attached to page.');
+ }
+
+ // Calculate totals expected for each user type
+ // Total number of pages.
+ $total++;
+ // Total number of primary and secondary references.
+ if ($reference != 'no_reference') {
+ $count_s_total++;
+ if ($reference2 != 'no_reference') {
+ $count_s2_total++;
+ }
+ }
+ // Public users only see 'public' and 'author_public' articles.
+ if (substr($reference, -6) == 'public') {
+ $count_s_public++;
+ if (substr($reference2, -6) == 'public') {
+ $count_s2_public++;
+ }
+ }
+ // authorUser sees 'public', 'author_public', and 'author_private' articles.
+ if (substr($reference, -6) == 'public' || substr($reference, 0, 6) == 'author') {
+ $count_s_author++;
+ if (substr($reference2, -6) == 'public' || substr($reference2, 0, 6) == 'author') {
+ $count_s2_author++;
+ }
+ }
+
+ // $total_public and $total_author are not currently in use -- but
+ // represent the totals when joins are handled by adding an is-null
+ // check (i.e., if inaccessible references caused the entire row to be
+ // be hidden from view, instead of hiding just one cell of the table).
+ // Count of pages where all related articles are accessible by
+ // public users.
+ if (substr($reference, -7) != 'private' && substr($reference2, -7) != 'private') {
+ $total_public++;
+ }
+ // Count of pages where all related articles are accessible by
+ // authorUser.
+ if ($reference != 'private' && $reference2 != 'private') {
+ $total_author++;
+ }
+ }
+ }
+
+ // Open a webpage listing all the pages, and check the webpage's content for
+ // users with three different access levels. (The webpage mimics what the view used
+ // for these tests in drupal8, but without the need for non-core modules.)
+
+ // Check the author of the 'author' articles.
+ $this->drupalLogin($this->authorUser);
+ $this->drupalGet('node_access_join_test_page');
+ $chk_total = count($this->xpath("//td[@headers='view-title-table-column']"));
+ $this->assertEqual($chk_total, $total, 'Author should see ' . $total . ' rows. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-1-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s_author, 'Author should see ' . $count_s_author . ' primary references. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-2-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s2_author, 'Author should see ' . $count_s2_author . ' secondary references. Actual: ' . $chk_total);
+ $this->assertText('Page - no_reference', 'Author should see page with no reference.');
+ $this->assertText('Page - public - no_reference', 'Author should see page with one public reference.');
+ $this->assertText('Page - public - public', 'Author should see page with two public references.');
+ $this->assertText('Page - author_private - no_reference', 'Author should see page with own private reference.');
+ $this->assertText('Article public', 'Author should see articles with access type: public.');
+ $this->assertNoText('Article private', 'Author should not see articles with access type: private.');
+ $this->assertText('Article author_public', 'Author should see articles with access type: author_public.');
+ $this->assertText('Article author_private', 'Author should see articles with access type: author_private.');
+ // Following test is no longer relevant.
+ //$this->assertNoText('- private', 'Author should not see pages related to others\' private articles.');
+
+ // Check a regular user (who did not author any articles).
+ $this->regularUser = $this->drupalCreateUser(['access content']);
+ $this->drupalLogin($this->regularUser);
+ $this->drupalGet('node_access_join_test_page');
+ $chk_total = count($this->xpath("//td[@headers='view-title-table-column']"));
+ $this->assertEqual($chk_total, $total, 'Public user should see ' . $total . ' rows. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-1-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s_public, 'Public user should see ' . $count_s_public . ' primary references. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-2-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s2_public, 'Public user should see ' . $count_s2_public . ' secondary references. Actual: ' . $chk_total);
+ $this->assertText('Page - no_reference', 'Public user should see page with no reference.');
+ $this->assertText('Page - public - no_reference', 'Public user should see page with one public reference.');
+ $this->assertText('Page - public - public', 'Public user should see page with two public references.');
+ $this->assertText('Article public', 'Public user should see articles with access type: public.');
+ $this->assertNoText('Article private', 'Public user should not see articles with access type: private.');
+ $this->assertText('Article author_public', 'Public user should see articles with access type: author_public.');
+ $this->assertNoText('Article author_private', 'Public user should not see articles with access type: author_private.');
+ // Following test is no longer relevant.
+ //$this->assertNoText('private', 'Public user should not see pages related to any private articles.');
+
+ // Check a user with the special 'node test view' permission, who should
+ // be able to view all pages and articles.
+ $this->accessUser = $this->drupalCreateUser(['access content', 'node test view']);
+ $this->drupalLogin($this->accessUser);
+ $this->drupalGet('node_access_join_test_page');
+ $chk_total = count($this->xpath("//td[@headers='view-title-table-column']"));
+ $this->assertEqual($chk_total, $total, 'Full-access user should see ' . $total . ' rows. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-1-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s_total, 'Full-access user should see ' . $count_s_total . ' primary references. Actual: ' . $chk_total);
+ $chk_total = count($this->xpath("//td[@headers='view-title-2-table-column']/a"));
+ $this->assertEqual($chk_total, $count_s2_total, 'Full-access user should see ' . $count_s2_total . ' secondary references. Actual: ' . $chk_total);
+ $this->assertText('Page - no_reference', 'Full-access user should see page with no reference.');
+ $this->assertText('Page - public - no_reference', 'Full-access user should see page with one public reference.');
+ $this->assertText('Page - public - public', 'Full-access user should see page with two public references.');
+ $this->assertText('Page - author_private - no_reference', 'Full-access user should see page with author_private reference.');
+ $this->assertText('Article public', 'Full-access user should see articles with access type: public.');
+ $this->assertText('Article private', 'Full-access user should see articles with access type: private.');
+ $this->assertText('Article author_public', 'Full-access user should see articles with access type: author_public.');
+ $this->assertText('Article author_private', 'Full-access user should see articles with access type: author_private.');
+ }
+}
+
/**
* Tests the cache invalidation of node operations.
*/
diff --git a/modules/node/tests/node_access_test.module b/modules/node/tests/node_access_test.module
index 7932c55..f6008a5 100644
--- a/modules/node/tests/node_access_test.module
+++ b/modules/node/tests/node_access_test.module
@@ -91,6 +91,12 @@ function node_access_test_menu() {
'access arguments' => array('access content'),
'type' => MENU_SUGGESTED_ITEM,
);
+ $items['node_access_join_test_page'] = array(
+ 'title' => 'Node access join test',
+ 'page callback' => 'node_access_join_test_page',
+ 'access arguments' => array('access content'),
+ 'type' => MENU_SUGGESTED_ITEM,
+ );
return $items;
}
@@ -171,6 +177,95 @@ function node_access_entity_test_page() {
}
/**
+ * Page callback for node access join test page.
+ *
+ * This page simulates the views used for testing in the d8 version of the
+ * code, but without introducing a views dependency in d7.
+ *
+ * Page should say "No nodes" if there are no nodes, and "Yes, # nodes" (with
+ * the number filled in) if there were nodes the user could access. If there
+ * were nodes, a table lists the query results. Also, the database query is
+ * shown, for debugging purposes. And if there is a query exception, the page
+ * says "Exception" and gives the error.
+ *
+ * @see node_access_test_menu()
+ */
+function node_access_join_test_page() {
+ $output = '';
+ try {
+ // Get custom field info
+ $field_name = 'related_article';
+ $field = field_info_field($field_name);
+ $join_table = _field_sql_storage_tablename($field);
+ $join_column = $field_name . '_value';
+
+ // Set up template query.
+ $query = db_select('node', 'n');
+ // Add the tag that triggers node_access processing.
+ $query->addTag('node_access');
+ $query->condition('n.type', 'page');
+ $query->orderBy('n.title');
+
+ // Add primary related article field and its associated node.
+ $query->addJoin('LEFT OUTER', $join_table, 'jf1', 'n.vid = jf1.revision_id');
+ $query->addJoin('LEFT OUTER', 'node', 's1', 'jf1.' . $join_column .' = s1.nid');
+
+ // Add the article's related article field and its associated node.
+ $query->addJoin('LEFT OUTER', $join_table, 'jf2', 's1.vid = jf2.revision_id');
+ $query->addJoin('LEFT OUTER', 'node', 's2', 'jf2.' . $join_column .' = s2.nid');
+
+ $query
+ ->fields('n', array('nid', 'title'))
+ ->fields('s1', array('nid', 'title'))
+ ->fields('s2', array('nid', 'title'));
+
+ $total_count = $query->countQuery()->execute()->fetchField();
+
+ if (!empty($total_count)) {
+ $output .= 'Yes, ' . $total_count . ' nodes
';
+
+ // Generate table listing the results. The tests using this table rely on
+ // xpath counting to get the number of rows and number of non-empty cells.
+ // Those xpath counts are controlled using "headers" attributes that are
+ // automatically inserted by views in drupal8. This table adds the same
+ // headers attributes so the drupal8 xpath requests can be used unaltered.
+ $output .= '';
+ }
+ else {
+ $output .= 'No nodes
';
+ }
+
+ $output .= '' . ((string) $query ) . '
';
+ $output .= '' . serialize($query->getArguments()) . '
';
+ }
+ catch (Exception $e) {
+ $output = 'Exception
';
+ $output .= '' . $e->getMessage() . '
';
+ }
+
+ return $output;
+}
+
+/**
* Implements hook_form_BASE_FORM_ID_alter().
*/
function node_access_test_form_node_form_alter(&$form, $form_state) {
diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test
index 59d2e5d..b384843 100644
--- a/modules/simpletest/tests/database_test.test
+++ b/modules/simpletest/tests/database_test.test
@@ -2374,6 +2374,52 @@ class DatabaseSelectComplexTestCase2 extends DatabaseTestCase {
$pos2 = strpos($str, 'db_condition_placeholder_0', $pos + 1);
$this->assertFalse($pos2, 'Condition placeholder is not repeated.');
}
+
+ /**
+ * Test that join conditions can use Condition Objects.
+ */
+ function testJoinConditionObject() {
+ // Same test as testDefaultJoin, but with a Condition Object.
+ $query = db_select('test_task', 't');
+ $join_cond = db_and()->where('t.pid = p.id');
+ $people_alias = $query->join('test', 'p', $join_cond);
+ $name_field = $query->addField($people_alias, 'name', 'name');
+ $task_field = $query->addField('t', 'task', 'task');
+ $priority_field = $query->addField('t', 'priority', 'priority');
+
+ $query->orderBy($priority_field);
+ $result = $query->execute();
+
+ $num_records = 0;
+ $last_priority = 0;
+ foreach ($result as $record) {
+ $num_records++;
+ $this->assertTrue($record->$priority_field >= $last_priority, 'Results returned in correct order.');
+ $this->assertNotEqual($record->$name_field, 'Ringo', 'Taskless person not selected.');
+ $last_priority = $record->$priority_field;
+ }
+
+ $this->assertEqual($num_records, 7, 'Returned the correct number of rows.');
+
+ // Test a condition object that creates placeholders.
+ $t1_name = 'John';
+ $t2_name = 'George';
+ $join_cond = db_and()
+ ->condition('t1.name', $t1_name)
+ ->condition('t2.name', $t2_name);
+ $query = db_select('test', 't1');
+ $query->innerJoin('test', 't2', $join_cond);
+ $query->addField('t1', 'name', 't1_name');
+ $query->addField('t2', 'name', 't2_name');
+
+ $num_records = $query->countQuery()->execute()->fetchField();
+ $this->assertEqual($num_records, 1, 'Query expected to return 1 row. Actual: ' . $num_records);
+ if ($num_records==1) {
+ $record = $query->execute()->fetchObject();
+ $this->assertEqual($record->t1_name, $t1_name, 'Query expected to retrieve name ' . $t1_name . ' from table t1. Actual: ' . $record->t1_name);
+ $this->assertEqual($record->t2_name, $t2_name, 'Query expected to retrieve name ' . $t2_name . ' from table t2. Actual: ' . $record->t2_name);
+ }
+ }
}
class DatabaseSelectPagerDefaultTestCase extends DatabaseTestCase {