From 960f6f6e41c452c08aeca01ba8bd5af9042b3f22 Mon Sep 17 00:00:00 2001 From: Pierre Buyle Date: Fri, 6 Jun 2014 16:15:24 -0500 Subject: [PATCH] Issue #2067671: Ensure file query alteration is performed when using EntityFieldQuery. --- file_entity.module | 89 +++++++++++++++++++++++-------------- file_entity.test | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 32 deletions(-) diff --git a/file_entity.module b/file_entity.module index 6b5da8f..ac4f9c3 100644 --- a/file_entity.module +++ b/file_entity.module @@ -1744,7 +1744,7 @@ function file_entity_query_file_access_alter(QueryAlterableInterface $query) { * access conditions are added for field values belonging to files only. */ function file_entity_query_entity_field_access_alter(QueryAlterableInterface $query) { - //_file_entity_query_file_entity_access_alter($query, 'entity'); + _file_entity_query_file_entity_access_alter($query, 'entity'); } /** @@ -1772,6 +1772,12 @@ function _file_entity_query_file_entity_access_alter($query, $type) { $tables = $query->getTables(); $base_table = $query->getMetaData('base_table'); + // Do not use the base table for general entity queries unless this is + // querying the file_managed table directly. + if ($base_table && $type == 'entity' && $base_table != 'file_managed') { + $base_table = ''; + } + // If no base table is specified explicitly, search for one. if (!$base_table) { $fallback = ''; @@ -1803,6 +1809,10 @@ function _file_entity_query_file_entity_access_alter($query, $type) { $fallback = $table; } } + elseif (isset($schema['fields']['entity_id']) && isset($schema['fields']['entity_type']) && isset($schema['fields']['deleted']) && isset($schema['fields']['delta']) && isset($schema['fields']['delta'])) { + // The table is a field data table, use it as fallback. + $base_table = $table; + } } } } @@ -1813,6 +1823,11 @@ function _file_entity_query_file_entity_access_alter($query, $type) { $base_table = $fallback; } else { + // Ignore this query as it was a general field query and no + // relationships were found to the file_managed table. + if ($type == 'entity') { + return; + } throw new Exception(t('Query tagged for file access but there is no fid. Add foreign keys to file_managed.fid in schema to fix.')); } } @@ -1877,31 +1892,36 @@ function _file_entity_query_file_entity_access_alter($query, $type) { if ($subquery_conditions->count()) { $subquery->condition($subquery_conditions); + } + else { + // If there is no conditions, then user cannot access any file. + // Add a bogus condition in order to exclude all files. + $subquery->where('fid = FALSE'); + } - $field = 'fid'; - // Now handle entities. - if ($type == 'entity') { - // Set a common alias for entities. - $base_alias = $falias; - $field = 'entity_id'; - } - $subquery->where("$falias.$field = fm_access.fid"); + $field = 'fid'; + // Now handle entities. + if ($type == 'entity') { + // Set a common alias for entities. + $base_alias = $falias; + $field = ($falias == 'file_managed' ? 'fid' : 'entity_id'); + } + $subquery->where("$falias.$field = fm_access.fid"); - // For an entity query, attach the subquery to entity conditions. - if ($type == 'entity') { - $file_conditions->exists($subquery); - } - // Otherwise attach it to the node query itself. - elseif ($table == 'file_managed') { - // Fix for https://drupal.org/node/2073085 - $db_or = db_or(); - $db_or->exists($subquery); - $db_or->isNull($falias . '.' . $field); - $query->condition($db_or); - } - else { - $query->exists($subquery); - } + // For an entity query, attach the subquery to entity conditions. + if ($type == 'entity') { + $file_conditions->exists($subquery); + } + // Otherwise attach it to the node query itself. + elseif ($table == 'file_managed') { + // Fix for https://drupal.org/node/2073085 + $db_or = db_or(); + $db_or->exists($subquery); + $db_or->isNull($falias . '.' . $field); + $query->condition($db_or); + } + else { + $query->exists($subquery); } } } @@ -1909,14 +1929,19 @@ function _file_entity_query_file_entity_access_alter($query, $type) { if ($type == 'entity' && $file_conditions->count()) { // All the file access conditions are only for field values belonging to // files. - $file_conditions->condition("$base_alias.entity_type", 'file'); - $or = db_or(); - $or->condition($file_conditions); - // If the field value belongs to a non-file entity type then this function - // does not do anything with it. - $or->condition("$base_alias.entity_type", 'file', '<>'); - // Add the compiled set of rules to the query. - $query->condition($or); + if ($base_alias !== 'file_managed') { + $file_conditions->condition("$base_alias.entity_type", 'file'); + $or = db_or(); + $or->condition($file_conditions); + // If the field value belongs to a non-file entity type then this function + // does not do anything with it. + $or->condition("$base_alias.entity_type", 'file', '<>'); + // Add the compiled set of rules to the query. + $query->condition($or); + } + else { + $query->condition($file_conditions); + } } } diff --git a/file_entity.test b/file_entity.test index 3a440d3..b1b60ea 100644 --- a/file_entity.test +++ b/file_entity.test @@ -1266,4 +1266,130 @@ class FileEntityAccessTestCase extends FileEntityTestHelper { } } } + + /** + * @param EntityFieldQuery $query + * @param $expected + * An associative array of expected result. Keys are file ids, values are + * booleans to indicate if the result should include the file. + */ + function assertEntityFieldQueryAccess(EntityFieldQuery $query, $expected, $account = NULL, $query_name = 'unnamed') { + if ($account) { + $query->addMetaData('account', $account); + } + $query->addTag('entity_field_access'); + $results = $query->execute(); + $fids = isset($results['file']) ? array_keys($results['file']) : array(); + foreach ($expected as $fid => $in_result) { + if ($in_result) { + $this->assertTrue(in_array($fid, $fids), format_string("For the %name query, the result should contain %fid", array('%name' => $query_name, '%fid' => $fid))); + } + else { + $this->assertFalse(in_array($fid, $fids), format_string("For the %name query, the result should not contain %fid", array('%name' => $query_name, '%fid' => $fid))); + } + } + } + + /** + * Test file entity access for entity field queries. + */ + function testEntityFieldQueryAccess() { + // Attach a text field to the default image file type. + $field = array( + 'field_name' => drupal_strtolower($this->randomName()), + 'type' => 'text', + 'settings' => array( + 'max_length' => 255, + ) + ); + field_create_field($field); + $instance = array( + 'field_name' => $field['field_name'], + 'entity_type' => 'file', + 'bundle' => 'document', + 'widget' => array( + 'type' => 'text_textfield', + ), + 'display' => array( + 'default' => array( + 'type' => 'text_default', + ), + ), + ); + field_create_instance($instance); + // Create test files. + $file_owner = $this->drupalCreateUser(array('view own files', 'view own private files')); + $public_file = $this->createFileEntity(array( + 'status' => 0, + )); + $private_file = $this->createFileEntity(array('scheme' => 'private')); + $owned_public_file = $this->createFileEntity(array( + 'uid' => $file_owner->uid, + 'scheme' => 'public', + )); + $owned_private_file = $this->createFileEntity(array( + 'uid' => $file_owner->uid, + 'scheme' => 'private', + )); + $fids = array( + $public_file->fid, + $private_file->fid, + $owned_public_file->fid, + $owned_private_file->fid, + ); + foreach (file_load_multiple($fids) as $file) { + $file->{$field['field_name']}[LANGUAGE_NONE][0] = array('value' => 'find me'); + file_save($file); + } + + $queries['entity type and id conditions'] = (new EntityFieldQuery()) + ->entityCondition('entity_type', 'file') + ->entityCondition('entity_id', $fids); + $queries['single field condition'] = (new EntityFieldQuery()) + ->fieldCondition($field['field_name'], 'value', 'find me'); + + foreach($queries as $name => $query) { + $message = format_string(''); + // User should not see any file. + $this->assertEntityFieldQueryAccess(clone $query, array( + $public_file->fid => FALSE, + $private_file->fid => FALSE, + $owned_public_file->fid => FALSE, + $owned_private_file->fid => FALSE, + ), $this->drupalCreateUser(array('create files')), $name); + + // A user with the 'view own files' and 'view own private files' permissions should only see owned files. + $this->drupalLogin($file_owner); + $this->assertEntityFieldQueryAccess(clone $query, array( + $public_file->fid => FALSE, + $private_file->fid => FALSE, + $owned_public_file->fid => TRUE, + $owned_private_file->fid => TRUE, + ), $file_owner, $name); + + // User with the 'view files' permission should only see public files. + $this->assertEntityFieldQueryAccess(clone $query, array( + $public_file->fid => TRUE, + $private_file->fid => FALSE, + $owned_public_file->fid => TRUE, + $owned_private_file->fid => FALSE, + ), $this->drupalCreateUser(array('view files')), $name); + + // User with the 'view files' and 'view private files' permissions should only see all files. + $this->assertEntityFieldQueryAccess(clone $query, array( + $public_file->fid => TRUE, + $private_file->fid => TRUE, + $owned_public_file->fid => TRUE, + $owned_private_file->fid => TRUE, + ), $this->drupalCreateUser(array('view files', 'view private files')), $name); + + // User with the 'bypass file access' permissions should only see all files. + $this->assertEntityFieldQueryAccess(clone $query, array( + $public_file->fid => TRUE, + $private_file->fid => TRUE, + $owned_public_file->fid => TRUE, + $owned_private_file->fid => TRUE, + ), $this->drupalCreateUser(array('bypass file access')), $name); + } + } } -- 1.9.1