- Patch #860180 by chx, dixon_, jhodgdon: entity listing and loading does not...

- Patch #860180 by chx, dixon_, jhodgdon: entity listing and loading does not allow for node access.

includes/entity.inc

@@ -405,6 +405,10 @@ class EntityFieldQueryException extends Exception {}
  * specified or if the query has field conditions or sorts that are stored in
  * different field storage engines. However, this logic can be overridden in
  * hook_entity_query().
+ *
+ * Also note that this query does not automatically respect entity access
+ * restrictions. Node access control is performed by the SQL storage engine but
+ * other storage engines might not do this.
  */
 class EntityFieldQuery {
   /**
@@ -515,6 +519,24 @@ class EntityFieldQuery {
    */
   public $age = FIELD_LOAD_CURRENT;
 
+  /**
+   * A list of the tags added to this query.
+   *
+   * @var array
+   *
+   * @see EntityFieldQuery::addTag()
+   */
+  public $tags = array();
+
+  /**
+   * A list of metadata added to this query.
+   *
+   * @var array
+   *
+   * @see EntityFieldQuery::addMetaData()
+   */
+  public $metaData = array();
+
   /**
    * The ordered results.
    *
@@ -816,6 +838,52 @@ public function age($age) {
     return $this;
   }
 
+  /**
+   * Adds a tag to the query.
+   *
+   * Tags are strings that mark a query so that hook_query_alter() and
+   * hook_query_TAG_alter() implementations may decide if they wish to alter
+   * the query. A query may have any number of tags, and they must be valid PHP
+   * identifiers (composed of letters, numbers, and underscores). For example,
+   * queries involving nodes that will be displayed for a user need to add the
+   * tag 'node_access', so that the node module can add access restrictions to
+   * the query.
+   *
+   * If an entity field query has tags, it must also have an entity type
+   * specified, because the alter hook will need the entity base table.
+   *
+   * @param string $tag
+   *   The tag to add.
+   *
+   * @return EntityFieldQuery
+   *   The called object.
+   */
+  public function addTag($tag) {
+    $this->tags[$tag] = $tag;
+    return $this;
+  }
+
+  /**
+   * Adds additional metadata to the query.
+   *
+   * Sometimes a query may need to provide additional contextual data for the
+   * alter hook. The alter hook implementations may then use that information
+   * to decide if and how to take action.
+   *
+   * @param $key
+   *   The unique identifier for this piece of metadata. Must be a string that
+   *   follows the same rules as any other PHP identifier.
+   * @param $object
+   *   The additional data to add to the query. May be any valid PHP variable.
+   *
+   * @return EntityFieldQuery
+   *   The called object.
+   */
+  public function addMetaData($key, $object) {
+    $this->metaData[$key] = $object;
+    return $this;
+  }
+
   /**
    * Executes the query.
    *
@@ -971,7 +1039,7 @@ protected function propertyQuery() {
   /**
    * Finishes the query.
    *
-   * Adds the range and returns the requested list.
+   * Adds tags, metaData, range and returns the requested list or count.
    *
    * @param SelectQuery $select_query
    *   A SelectQuery which has entity_type, entity_id, revision_id and bundle
@@ -983,6 +1051,13 @@ protected function propertyQuery() {
    *   See EntityFieldQuery::execute().
    */
   function finishQuery($select_query, $id_key = 'entity_id') {
+    foreach ($this->tags as $tag) {
+      $select_query->addTag($tag);
+    }
+    foreach ($this->metaData as $key => $object) {
+      $select_query->addMetaData($key, $object);
+    }
+    $select_query->addMetaData('entity_field_query', $this);
     if ($this->range) {
       $select_query->range($this->range['start'], $this->range['length']);
     }

modules/field/modules/field_sql_storage/field_sql_storage.module

@@ -502,6 +502,8 @@ function field_sql_storage_field_storage_query(EntityFieldQuery $query) {
     }
     else {
       $select_query = db_select($tablename, $table_alias);
+      $select_query->addTag('entity_field_access');
+      $select_query->addMetaData('base_table', $tablename);
       $select_query->fields($table_alias, array('entity_id', 'revision_id', 'bundle'));
       // As only a numeric ID is stored instead of the entity type add the
       // field_config_entity_type table to resolve the etid to a more readable
@@ -547,6 +549,7 @@ function field_sql_storage_field_storage_query(EntityFieldQuery $query) {
   if (isset($query->deleted)) {
     $select_query->condition("$field_base_table.deleted", (int) $query->deleted);
   }
+
   if ($query->propertyConditions || $query->propertyOrder) {
     if (empty($query->entityConditions['entity_type']['value'])) {
       throw new EntityFieldQueryException('Property conditions and orders must have an entity type defined.');

modules/node/node.module

@@ -2767,7 +2767,6 @@ function node_access($op, $node, $account = NULL) {
     $rights[$account->uid][$cid][$op] = TRUE;
     return TRUE;
   }
-
   if (!user_access('access content', $account)) {
     $rights[$account->uid][$cid][$op] = FALSE;
     return FALSE;
@@ -3009,6 +3008,33 @@ function node_access_view_all_nodes() {
  * 'update' and 'delete').
  */
 function node_query_node_access_alter(QueryAlterableInterface $query) {
+  _node_query_node_access_alter($query, 'node', 'node');
+}
+
+/**
+ * Implements hook_query_TAG_alter().
+ *
+ * This function implements the same functionality as
+ * node_query_node_access_alter() for the SQL field storage engine. Node access
+ * 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');
+}
+
+/**
+ * Helper for node access functions.
+ *
+ * @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) {
   global $user;
 
   // Read meta-data from query, if provided.
@@ -3037,31 +3063,82 @@ function node_query_node_access_alter(QueryAlterableInterface $query) {
 
   $tables = $query->getTables();
   $grants = node_access_grants($op, $account);
+  if ($type == 'entity') {
+    // The original query looked something like:
+    // @code
+    //  SELECT nid FROM sometable s
+    //  INNER JOIN node_access na ON na.nid = s.nid
+    //  WHERE ($node_access_conditions)
+    // @endcode
+    //
+    // Our query will look like:
+    // @code
+    //  SELECT entity_type, entity_id
+    //  FROM field_data_something s
+    //  LEFT JOIN node_access na ON s.entity_id = na.nid
+    //  WHERE (entity_type = 'node' AND $node_access_conditions) OR (entity_type <> 'node')
+    // @endcode
+    //
+    // So instead of directly adding to the query object, we need to collect
+    // in a separate db_and() object and then at the end add it to the query.
+    $entity_conditions = db_and();
+  }
   foreach ($tables as $nalias => $tableinfo) {
     $table = $tableinfo['table'];
-    if (!($table instanceof SelectQueryInterface) && $table == 'node') {
+    if (!($table instanceof SelectQueryInterface) && $table == $base_table) {
 
-      // The node_access table has the access grants for any given node.
-      $access_alias = $query->join('node_access', 'na', '%alias.nid = ' . $nalias . '.nid');
-      $or = db_or();
+      // The node_access table has the access grants for any given node so JOIN
+      // it to the table containing the nid which can be either the node
+      // table or a field value table.
+      if ($type == 'node') {
+        $access_alias = $query->join('node_access', 'na', '%alias.nid = ' . $nalias . '.nid');
+      }
+      else {
+        $access_alias = $query->leftJoin('node_access', 'na', '%alias.nid = ' . $nalias . '.entity_id');
+        $base_alias = $nalias;
+      }
+
+      $grant_conditions = db_or();
       // If any grant exists for the specified user, then user has access
       // to the node for the specified operation.
       foreach ($grants as $realm => $gids) {
         foreach ($gids as $gid) {
-          $or->condition(db_and()
+          $grant_conditions->condition(db_and()
             ->condition($access_alias . '.gid', $gid)
             ->condition($access_alias . '.realm', $realm)
           );
         }
       }
 
-      if (count($or->conditions())) {
-        $query->condition($or);
+      $count = count($grant_conditions->conditions());
+      if ($type == 'node') {
+        if ($count) {
+          $query->condition($grant_conditions);
+        }
+        $query->condition($access_alias . '.grant_' . $op, 1, '>=');
+      }
+      else {
+        if ($count) {
+          $entity_conditions->condition($grant_conditions);
+        }
+        $entity_conditions->condition($access_alias . '.grant_' . $op, 1, '>=');
       }
-
-      $query->condition($access_alias . '.grant_' . $op, 1, '>=');
     }
   }
+
+  if ($type == 'entity' && count($entity_conditions->conditions())) {
+    // All the node access conditions are only for field values belonging to
+    // nodes.
+    $etid = variable_get('field_sql_storage_node_etid');
+    $entity_conditions->condition("$base_alias.etid", $etid);
+    $or = db_or();
+    $or->condition($entity_conditions);
+    // If the field value belongs to a non-node entity type then this function
+    // does not do anything with it.
+    $or->condition("$base_alias.etid", $etid, '<>');
+    // Add the compiled set of rules to the query.
+    $query->condition($or);
+  }
 }
 
 /**

modules/node/node.test

@@ -1596,7 +1596,8 @@ class NodeQueryAlter extends DrupalWebTestCase {
     $this->drupalCreateNode();
     $this->drupalCreateNode();
 
-    // Create user with simple node access permission.
+    // Create user with simple node access permission. The 'node test view'
+    // permission is implemented and granted by the node_access_test module.
     $this->accessUser = $this->drupalCreateUser(array('access content', 'node test view'));
     $this->noAccessUser = $this->drupalCreateUser(array('access content'));
   }
@@ -1606,14 +1607,12 @@ class NodeQueryAlter extends DrupalWebTestCase {
    */
   function testNodeQueryAlterWithUI() {
     // Verify that a user with access permission can see at least one node.
-
     $this->drupalLogin($this->accessUser);
     $this->drupalGet('node_access_test_page');
     $this->assertText('Yes, 4 nodes', "4 nodes were found for access user");
     $this->assertNoText('Exception', "No database exception");
 
     // Verify that a user with no access permission cannot see nodes.
-
     $this->drupalLogin($this->noAccessUser);
     $this->drupalGet('node_access_test_page');
     $this->assertText('No nodes', "No nodes were found for no access user");
@@ -1692,6 +1691,71 @@ class NodeQueryAlter extends DrupalWebTestCase {
   }
 }
 
+
+/**
+ * Tests node_query_entity_field_access_alter().
+ */
+class NodeEntityFieldQueryAlter extends DrupalWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Node entity query alter',
+      'description' => 'Test that node access entity queries are properly altered by the node module.',
+      'group' => 'Node',
+    );
+  }
+
+  /**
+   * User with permission to view content.
+   */
+  protected $accessUser;
+
+  /**
+   * User without permission to view content.
+   */
+  protected $noAccessUser;
+
+  function setUp() {
+    parent::setUp('node_access_test');
+    node_access_rebuild();
+
+    // Creating 4 nodes with an entity field so we can test that sort of query
+    // alter. All field values starts with 'A' so we can identify and fetch them
+    // in the node_access_test module.
+    $settings = array('language' => LANGUAGE_NONE);
+    for ($i = 0; $i < 4; $i++) {
+      $body = array(
+        'value' => 'A' . $this->randomName(32),
+        'format' => filter_default_format(),
+      );
+      $settings['body'][LANGUAGE_NONE][0] = $body;
+      $this->drupalCreateNode($settings);
+    }
+
+    // Create user with simple node access permission. The 'node test view'
+    // permission is implemented and granted by the node_access_test module.
+    $this->accessUser = $this->drupalCreateUser(array('access content', 'node test view'));
+    $this->noAccessUser = $this->drupalCreateUser(array('access content'));
+  }
+
+  /**
+   * Tests that node access permissions are followed.
+   */
+  function testNodeQueryAlterWithUI() {
+    // Verify that a user with access permission can see at least one node.
+    $this->drupalLogin($this->accessUser);
+    $this->drupalGet('node_access_entity_test_page');
+    $this->assertText('Yes, 4 nodes', "4 nodes were found for access user");
+    $this->assertNoText('Exception', "No database exception");
+
+    // Verify that a user with no access permission cannot see nodes.
+    $this->drupalLogin($this->noAccessUser);
+    $this->drupalGet('node_access_entity_test_page');
+    $this->assertText('No nodes', "No nodes were found for no access user");
+    $this->assertNoText('Exception', "No database exception");
+  }
+}
+
 /**
  * Test node token replacement in strings.
  */

modules/node/tests/node_access_test.module

@@ -58,6 +58,12 @@ function node_access_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_SUGGESTED_ITEM,
   );
+  $items['node_access_entity_test_page'] = array(
+    'title' => 'Node access test',
+    'page callback' => 'node_access_entity_test_page',
+    'access arguments' => array('access content'),
+    'type' => MENU_SUGGESTED_ITEM,
+  );
   return $items;
 }
 
@@ -100,3 +106,37 @@ function node_access_test_page() {
 
   return $output;
 }
+
+/**
+ * Page callback for node access entity test page.
+ *
+ * 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. Also, the
+ * database query is shown, and a list of the node IDs, for debugging purposes.
+ * And if there is a query exception, the page says "Exception" and gives the
+ * error.
+ */
+function node_access_entity_test_page() {
+  $output = '';
+  try {
+    $query = new EntityFieldQuery;
+    $result = $query->fieldCondition('body', 'value', 'A', 'STARTS_WITH')->execute();
+    if (!empty($result['node'])) {
+      $output .= '<p>Yes, ' . count($result['node']) . ' nodes</p>';
+      $output .= '<ul>';
+      foreach ($result['node'] as $nid => $v) {
+        $output .= '<li>' . $nid . '</li>';
+      }
+      $output .= '</ul>';
+    }
+    else {
+      $output .= '<p>No nodes</p>';
+    }
+  }
+  catch (Exception $e) {
+    $output = '<p>Exception</p>';
+    $output .= '<p>' . $e->getMessage() . '</p>';
+  }
+
+  return $output;
+}