Like all other entities, we should ensure that the 'file_access' tag is used whenever we query for files and display them to the user. Just in case a module wants to alter files that are visible.

Files: 
CommentFileSizeAuthor
#46 1959260-fix.patch5.76 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#43 1959260-fix.patch3.41 KBDave Reid
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#39 1959260-fix.patch3 KBDave Reid
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#38 1959260-fix.patch0 bytesDave Reid
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#34 file_entity-file_access-1959260-33.patch558 bytesczigor
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#29 1959260-file-access-api-query-alters.patch8.23 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#27 1959260-file-access-api-query-alters.patch378 bytesDave Reid
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1959260-file-access-api-query-alters.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#24 add-file-access-tag-to-all-queries-1959260-24.patch7.75 KBDevin Carlson
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#22 add-file-access-tag-to-all-queries-1959260-22.patch7.79 KBDevin Carlson
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch add-file-access-tag-to-all-queries-1959260-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#12 1959260-temp.patch8.3 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 829 pass(es).
[ View ]
#6 file_entity-7.x-2.x-1959260-6.patch2.57 KBLaurent Ren
PASSED: [[SimpleTest]]: [MySQL] 821 pass(es).
[ View ]
#2 add-file-access-tag-to-user-facing-queries-1959260-2.patch6.15 KBDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 814 pass(es).
[ View ]

Comments

We also need to consider moving the following code into a hook_query_file_access_alter():

<?php
 
if (user_access('bypass file access')) {
   
// Administrators don't need to be restricted to only permanent files.
   
$query->condition('fm.status', FILE_STATUS_PERMANENT);
  }
  elseif (
user_access('view files')) {
   
// For non-private files, users can view if they have the 'view files'
    // permission.
   
$query->condition('fm.status', FILE_STATUS_PERMANENT);
  }
  elseif (
user_access('view private files') && user_is_logged_in()) {
   
// For private files, users can view private files if the
    // user is not anonymous, and has the 'view private files' permission.
   
$query->condition('fm.uri', db_like('private://') . '%', 'LIKE');
   
$query->condition('fm.status', FILE_STATUS_PERMANENT);
  }
  elseif (
user_access('view own files')) {
   
// For non-private files, allow to see if user owns the file.
   
$query->condition('fm.uid', $user->uid, '=');
   
$query->condition('fm.status', FILE_STATUS_PERMANENT);
  }
  elseif (
user_access('view own private files') && user_is_logged_in()) {
   
// For private files, users can view their own private files if the
    // user is not anonymous, and has the 'view own private files' permission.
   
$query->condition('fm.uri', db_like('private://') . '%', 'LIKE');
   
$query->condition('fm.uid', $user->uid, '=');
   
$query->condition('fm.status', FILE_STATUS_PERMANENT);
  }
  foreach (
array_keys(file_entity_get_hidden_stream_wrappers()) as $name) {
   
$query->condition('fm.uri', $name . '%', 'NOT LIKE');
  }
?>

Assigned:Dave Reid» Devin Carlson
Status:Active» Needs review
StatusFileSize
new6.15 KB
PASSED: [[SimpleTest]]: [MySQL] 814 pass(es).
[ View ]

This has been on my list for a while, as it will help simplify file access restrictions in CKEditor Link File.

Status:Needs review» Fixed

Committed to 7.x-2.x.

Status:Fixed» Needs work

You're going too fast for me to be able to make sure we have the relevant information. We need this added to views as well.

We need the following added to file_entity_views_data_alter():

<?php
 
// Add access tag for all queries against file_managed.
 
$data['file_managed']['table']['base']['access query tag'] = 'file_access';
?>

Priority:Normal» Major

This also fails if the main query doesn't alias the file managed table to 'fm'. Our query alter needs to do some introspection to the exiting tables and find the alias used for the file_managed table.

StatusFileSize
new2.57 KB
PASSED: [[SimpleTest]]: [MySQL] 821 pass(es).
[ View ]

From my (duplicate) issue #1974002: EntityReference autocomplete field with file entity :

One way to remove 'fm.' alias is to use this piece of code :

$tables = $query->getTables();
  $alias = isset($tables['file_managed']['alias']) ? $tables['file_managed']['alias'] : 'fm';

(I let 'fm' as default alias)

Hope it helps.

Status:Needs work» Needs review

I'm pleased to report that Laurent's patch in #6 corrected the error I was getting from an entity reference field.

Status:Needs review» Needs work

Hrm, I'm afraid we're going to have to also ensure that this works on EntityFieldQuery queries as well, which is going to take some more work. It looks like we might have to duplicate a lot of logic from _node_query_node_access_alter().

We're also missing the following change for Views:

diff --git a/file_entity.views.inc b/file_entity.views.inc
index 95e0b58..ea8b5a1 100644
--- a/file_entity.views.inc
+++ b/file_entity.views.inc
@@ -113,6 +113,9 @@ function file_entity_views_data() {
  * Implements hook_views_data_alter().
  */
 function file_entity_views_data_alter(&$data) {
+  // Add access tag for all queries against file_managed.
+  $data['file_managed']['table']['base']['access query tag'] = 'file_access';
+  // Override the filename field handler.
   $data['file_managed']['filename']['field']['handler'] = 'views_handler_field_file_filename';
 }
 

For documentation:
The patch #2 makes the linkit module with files unusable. The patch #6 solves the problem.

http://drupal.org/node/1975176#comment-7355854

The module adds a status condition to the query:

  function getQueryInstance() {
    // Call the parent getQueryInstance method.
    parent::getQueryInstance();
    // Only search for permanent files.
    $this->query->propertyCondition('status', FILE_STATUS_PERMANENT);
  }

Error:
PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'fm.status' in 'where clause': SELECT file_managed.fid AS entity_id, file_managed.type AS bundle, :entity_type AS entity_type, NULL AS revision_id FROM {file_managed} file_managed WHERE (file_managed.status = :db_condition_placeholder_0) AND (file_managed.filename LIKE :db_condition_placeholder_1 ESCAPE '\\') AND (file_managed.type IN (:db_condition_placeholder_2)) AND (fm.status = :db_condition_placeholder_3) AND (fm.uri NOT LIKE :db_condition_placeholder_4 ESCAPE '\\') ORDER BY file_managed.filename ASC; Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => %test% [:db_condition_placeholder_2] => document [:db_condition_placeholder_3] => 1 [:db_condition_placeholder_4] => temporary% [:entity_type] => file ) in EntityFieldQuery->execute() (Zeile 1145 von \includes\entity.inc).

Reason: Hardcoded table alias "fm"

I had the same issue and #6 works for me.

StatusFileSize
new8.3 KB
PASSED: [[SimpleTest]]: [MySQL] 829 pass(es).
[ View ]

Here is the code I was attempting to work on to make our file access tag work the same as node access, for both direct queries, and EntityFieldQuery queries. This also includes the necessary change to ensure that any Views query gets automatically tagged with 'file_access'.

Status:Needs work» Needs review

Change the status to needs review.

Status:Reviewed & tested by the community» Needs review

Status:Needs review» Reviewed & tested by the community

Tested and works! I'm using the latest patch with the linkit module.

Status:Needs review» Needs work

The patch in #12 is definitely not ready to be committed.

The latest patch cleared up the aliasing problem for me. Thanks!

I can also confirm that linkit works with patched file_entity (patch from #12 applied cleanly to current dev).

Why is it not ready to commit, Dave Reid?

Because the patch in #12 was not actually ready to go at all, it was a work in progress. Functions like file_entity_query_entity_field_access_alter() are empty and therefore access integration with EntityFieldQuery is not complete.

Will a solution to this be part of the dev version some day soon?
The patch in #6 fixes my problems with the linkit and also with filedepot_linkit module.

Status:Needs work» Needs review
StatusFileSize
new7.79 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch add-file-access-tag-to-all-queries-1959260-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

An updated version of #12 which:

Status:Needs review» Needs work

The last submitted patch, add-file-access-tag-to-all-queries-1959260-22.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.75 KB
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Oops, forgot to merge in the latest changes.

Status:Needs review» Needs work

The last submitted patch, add-file-access-tag-to-all-queries-1959260-24.patch, failed testing.

Wow this is really coming along Devin, nice work. Just a few failures to fix!

Status:Needs work» Needs review
StatusFileSize
new378 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1959260-file-access-api-query-alters.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I think this resolves the query altering from all queries.

Status:Needs review» Needs work

The last submitted patch, 1959260-file-access-api-query-alters.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new8.23 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Try again.

Title:Ensure a 'file_access' tag is used on all queries of outputted files[Needs tests] Ensure a 'file_access' tag is used on all queries of outputted files
Issue tags:-7.x-2.0 alpha blocker+Needs tests, +7.x-2.0 beta blocker

ROCK ON. Committed #29 to 7.x-2.x just to unblock the alpha release. I did not write any tests, but I heavily tested this with Views, EntityFieldQuery, etc. This is now an beta blocker for tests.

I just updated to the alpha release and an EntityFieldQuery that I had running successfully is now no longer returning results. I've dumped my queries in MySQL and can see that the query alter introduced here is the culprit.

My EFQ:

$query = new EntityFieldQuery();
$query->entityCondition('entity_type', 'node')
  ->entityCondition('bundle', 'csm_user_review')
  ->propertyCondition('status', 1)
  ->fieldCondition('field_reference_csm_review', 'target_id', $nid, '=')
  ->propertyOrderBy('nid', 'ASC');

So just a basic query for nodes of type csm_user_review where the entityreference field_reference_csm_review is the nid for a given node. This query should return 702 rows. It returns 0 rows now. The SQL before this patch would be something like:

SELECT
field_data_field_reference_csm_review0.entity_type AS entity_type,
field_data_field_reference_csm_review0.entity_id AS entity_id,
field_data_field_reference_csm_review0.revision_id AS revision_id,
field_data_field_reference_csm_review0.bundle AS bundle
FROM
{field_data_field_reference_csm_review} field_data_field_reference_csm_review0
INNER JOIN
{node} node ON node.nid = field_data_field_reference_csm_review0.entity_id
WHERE
(field_data_field_reference_csm_review0.field_reference_csm_review_target_id = :db_condition_placeholder_0) AND
(field_data_field_reference_csm_review0.deleted = :db_condition_placeholder_1) AND
(node.status = :db_condition_placeholder_2) AND
(field_data_field_reference_csm_review0.entity_type = :db_condition_placeholder_3) AND
(field_data_field_reference_csm_review0.bundle = :db_condition_placeholder_4)
ORDER BY
node.nid ASC
where the placeholders are target_id = 1261672, deleted = 0, status = 1, entity_type = node, bundle = csm_user_review

The rewritten query is:

SELECT
field_data_field_reference_csm_review0.entity_type AS entity_type,
field_data_field_reference_csm_review0.entity_id AS entity_id,
field_data_field_reference_csm_review0.revision_id AS revision_id,
field_data_field_reference_csm_review0.bundle AS bundle
FROM
field_data_field_reference_csm_review field_data_field_reference_csm_review0
INNER JOIN
node node ON node.nid = field_data_field_reference_csm_review0.entity_id
INNER JOIN
file_managed fm ON fm.fid = field_data_field_reference_csm_review0.entity_id
WHERE
(field_data_field_reference_csm_review0.field_reference_csm_review_target_id = '1261672') AND
(field_data_field_reference_csm_review0.deleted = '0') AND
(node.status = '1') AND
(field_data_field_reference_csm_review0.entity_type = 'node') AND
(field_data_field_reference_csm_review0.bundle = 'csm_user_review') AND
(
  (
  (fm.uri NOT LIKE 'temporary%' ESCAPE '\\') AND
  (fm.uri NOT LIKE 'module%' ESCAPE '\\') AND
  (fm.uri NOT LIKE 'theme%' ESCAPE '\\') AND
  (fm.uri NOT LIKE 'profile%' ESCAPE '\\') AND
  (fm.uri NOT LIKE 'library%' ESCAPE '\\') AND
  (field_data_field_reference_csm_review0.entity_type = 'file')
  )
  OR
  (field_data_field_reference_csm_review0.entity_type <> 'file')
)
ORDER BY
node.nid ASC;

This query returns 0 rows.

IF I change the JOIN to file_managed from INNER JOIN to LEFT JOIN, I get back the expected 702 rows.

At this point I am not sure if the query I had written should expect to be rewritten here as I had no interest whatsoever in access to files via my EFQ so it's a bit disconcerting that this query alter is happening at all but I can't claim to understand the recent changes.

Status:Needs review» Needs work

I have the same issue as in #31 with the following EFQ:

<?php
$result
= $query->entityCondition('entity_type', 'commerce_product')
    ->
entityCondition('bundle', 'ticket')
    ->
propertyCondition('status', 1)
    ->
fieldCondition('field_concert', 'target_id', $concert_nid)
    ->
fieldCondition('field_seat', 'target_id', $seat_nids)
    ->
execute();
?>

When running this query as anonymous user the result set is empty because the file_managed table gets inner joined in.

For uid1 it gives the correct result set without the file_managed INNER JOIN.

If I comment out the two fieldCondition lines the INNER JOIN is gone and I get the correct result set.

Status:Needs work» Needs review
StatusFileSize
new558 bytes
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Maybe it's this simple?

Title:[Needs tests] Ensure a 'file_access' tag is used on all queries of outputted filesEnsure a 'file_access' tag is used on all queries of outputted files => alpha1 breaks EntityFieldQuery not releated to files
Category:task» bug
Priority:Major» Critical
Status:Needs review» Needs work

As described in #33, the alpha1 release that includes this feature breaks every EntityFiledQuery tha is not related to File Entities.

Due to the popularity of the Media module, that's a critical issue that needs to be fixed ASAP!

Every contrib module that uses EntityFieldQuery is broken!

The patch in #34 is not the right solution.

It's strange because I haven't actually been able to duplicate this yet locally. I'm working on a set of extensive tests to confirm that this either is working or not, but likely not.

I run this script with drush scr query.php:

<?php
$query
= new EntityFieldQuery();
$query->entityCondition('entity_type', 'node')
  ->
entityCondition('bundle', 'page');
var_dump($query->execute());
?>

Results:

  • file_entity module disabled: all "pages"
  • file_entity module enabled, no file entities in database: no "pages"
  • file_entity module enabled, some file entities in database: some "pages"

The third test returns an incomplete list of pages. It's not a random list. Due to the join with the table file_managed on fid=entity_id, it returns all pages having the same entity_id than a fid of a file managed by file_entity but is not related to the page at all.

If you have many of files, than you won't face the error.

From my point of view the problem is caused by function file_entity_query_entity_field_access_alter(). It must not add the join of table file_managed if the entity is something else than a file_entity.

Status:Needs work» Needs review
StatusFileSize
new0 bytes
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Ok I think this should resolve the major issue with EntityFieldQuery. Not sure why I didn't do this part in the same style as the node query altering. Also, it made me question some of the conditions we had inside the query alter, if the user has the 'view own files' permission, we were restricting the listing to only files the user can view. That permission itself doesn't necessarily mean that the user doesn't have the 'view all files' permission, so for now I'm going to uncomment them out and leave in the 'hidden' stream wrapper condition which is valid.

StatusFileSize
new3 KB
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Wrong patch.

Status:Needs review» Needs work

The last submitted patch, 1959260-fix.patch, failed testing.

Status:Needs work» Needs review

Confirm this issue. Patch from #39 solves it, but as I see it always adds sub-query for each EntityFieldQuery execution, but seems that it is not necessary, and slow-down query, because it executes on entities that don't use any file links or fields.

For example, I have entity (created via ECK module) with only one custom text field, not any file_entity dependencies.
So I do the query:

<?php
    $equery
= new EntityFieldQuery;
      ->
entityCondition('entity_type', 'workflow_log')
      ->
fieldCondition('field_order_status', 'value', 'test')
      ->
execute();
?>

and all works quckly via simple query:
SELECT field_data_field_commerce_order0.entity_type AS entity_type, field_data_field_commerce_order0.entity_id AS entity_id, field_data_field_commerce_order0.revision_id AS revision_id, field_data_field_commerce_order0.bundle AS bundle FROM {field_data_field_commerce_order} field_data_field_commerce_order0 WHERE (field_data_field_commerce_order0.field_commerce_order_target_id = :db_condition_placeholder_0) AND (field_data_field_commerce_order0.deleted = :db_condition_placeholder_1) AND (field_data_field_commerce_order0.entity_type = :db_condition_placeholder_2)

But when I enable file_entity module, it adds sub-query for searching files:

SELECT field_data_field_order_status0.entity_type AS entity_type, field_data_field_order_status0.entity_id AS entity_id, field_data_field_order_status0.revision_id AS revision_id, field_data_field_order_status0.bundle AS bundle FROM field_data_field_order_status field_data_field_order_status0 WHERE (field_data_field_order_status0.field_order_status_value = :db_condition_placeholder_3) AND (field_data_field_order_status0.deleted = :db_condition_placeholder_4) AND (field_data_field_order_status0.entity_type = :db_condition_placeholder_5) AND(( ( EXISTS (SELECT fm_access.fid AS fid FROM file_managed fm_access WHERE (fm_access.uri NOT LIKE :db_condition_placeholder_6 ESCAPE '\\') AND (field_data_field_order_status0.entity_id = fm_access.fid) )) AND (field_data_field_order_status0.entity_type = :db_condition_placeholder_7) )OR (field_data_field_order_status0.entity_type <> :db_condition_placeholder_8) )

Why it try to search file if this entity isn't use files?

Are you sure that we need to execute additional query for each entity? Maybe exists some easier way to detect is this entity have file dependencies?

It should only add the query altering if there is a fieldCondition() on a field that is an image or file field. I'll investigate some more.

StatusFileSize
new3.41 KB
FAILED: [[SimpleTest]]: [MySQL] 873 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

I am getting super frustrated with EntityFieldQuery, and not understanding how this actually works or could ever work. This is my solution for now - disabling any alters for EntityFieldQuery() for now until we can figure out how to do it properly. This leaves in the query altering for anything tagged with 'file_access'.

Status:Needs review» Needs work

The last submitted patch, 1959260-fix.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.76 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Ok, this should work!

HOORAY

Status:Needs review» Fixed
Issue tags:-Needs tests

Ok committed to 7.x-2.x: http://drupalcode.org/project/file_entity.git/commit/5f4e8a9

Started follow-up for re-enabling the query alteration on EntityFieldQueries: #2067671: Ensure file query alteration is performed when using EntityFieldQuery

Automatically closed -- issue fixed for 2 weeks with no activity.