Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#46 | 1959260-fix.patch | 5.76 KB | Dave Reid |
#43 | 1959260-fix.patch | 3.41 KB | Dave Reid |
#39 | 1959260-fix.patch | 3 KB | Dave Reid |
#38 | 1959260-fix.patch | 0 bytes | Dave Reid |
#34 | file_entity-file_access-1959260-33.patch | 558 bytes | czigor |
Comments
Comment #1
Dave ReidWe also need to consider moving the following code into a hook_query_file_access_alter():
Comment #2
Devin Carlson CreditAttribution: Devin Carlson commentedThis has been on my list for a while, as it will help simplify file access restrictions in CKEditor Link File.
Comment #3
Devin Carlson CreditAttribution: Devin Carlson commentedCommitted to 7.x-2.x.
Comment #4
Dave ReidYou'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():
Comment #5
Dave ReidThis 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.
Comment #6
Laurent Ren CreditAttribution: Laurent Ren commentedFrom my (duplicate) issue #1974002: EntityReference autocomplete field with file entity :
One way to remove 'fm.' alias is to use this piece of code :
(I let 'fm' as default alias)
Hope it helps.
Comment #7
Laurent Ren CreditAttribution: Laurent Ren commentedComment #8
Scott Ellis CreditAttribution: Scott Ellis commentedI'm pleased to report that Laurent's patch in #6 corrected the error I was getting from an entity reference field.
Comment #9
Dave ReidHrm, 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:
Comment #10
ayalon CreditAttribution: ayalon commentedFor 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:
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"
Comment #11
nonom CreditAttribution: nonom commentedI had the same issue and #6 works for me.
Comment #12
Dave ReidHere 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'.
Comment #13
aaron CreditAttribution: aaron commentedChange the status to needs review.
Comment #14
ayalon CreditAttribution: ayalon commentedComment #15
ayalon CreditAttribution: ayalon commentedTested and works! I'm using the latest patch with the linkit module.
Comment #16
Dave ReidThe patch in #12 is definitely not ready to be committed.
Comment #17
markisatacomputer CreditAttribution: markisatacomputer commentedThe latest patch cleared up the aliasing problem for me. Thanks!
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedI can also confirm that linkit works with patched file_entity (patch from #12 applied cleanly to current dev).
Comment #19
aaron CreditAttribution: aaron commentedWhy is it not ready to commit, Dave Reid?
Comment #20
Dave ReidBecause 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.
Comment #21
Niko_K CreditAttribution: Niko_K commentedWill 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.
Comment #22
Devin Carlson CreditAttribution: Devin Carlson commentedAn updated version of #12 which:
hook_query_TAG_alter()
for the file_access and entity_field_access tags.Comment #24
Devin Carlson CreditAttribution: Devin Carlson commentedOops, forgot to merge in the latest changes.
Comment #26
Dave ReidWow this is really coming along Devin, nice work. Just a few failures to fix!
Comment #27
Dave ReidI think this resolves the query altering from all queries.
Comment #29
Dave ReidTry again.
Comment #30
Dave ReidROCK 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.
Comment #31
jaydub CreditAttribution: jaydub commentedI 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:
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:
The rewritten query is:
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.
Comment #32
aaron CreditAttribution: aaron commentedComment #33
czigor CreditAttribution: czigor commentedI have the same issue as in #31 with the following EFQ:
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.
Comment #34
czigor CreditAttribution: czigor commentedMaybe it's this simple?
Comment #35
mkalkbrennerAs 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.
Comment #36
Dave ReidIt'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.
Comment #37
mkalkbrennerI run this script with
drush scr query.php
:Results:
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.
Comment #38
Dave ReidOk 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.
Comment #39
Dave ReidWrong patch.
Comment #41
MurzConfirm 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:
and all works quckly via simple query:
But when I enable file_entity module, it adds sub-query for searching files:
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?
Comment #42
Dave ReidIt 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.
Comment #43
Dave ReidI 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'.
Comment #46
Dave ReidOk, this should work!
Comment #47
Dave ReidHOORAY
Comment #48
Dave ReidOk 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