Hi,

We need to filter access of entities referenced (only node) in bulk and we are facing with two problems :

  • First, even if user cannot view the reference, if he could edit it, he could view it ! I supposed the logic behind : if user could edit it, of course he should view it ! But this is not our use case...
  • For performance reason, we cannot apply hook_node_access() (more globally entity_access()) per entity, as called by entityreference_field_formatter_prepare_view(). We must do it in bulk, to reduce the number of SQL queries executed (1 instead of X references)

For these reasons, it would be great if it could let's a chance to other modules to make their own ACL check and switch back to the default behavior if no hook is implemented.

Here is a patch.

Thanks

CommentFileSizeAuthor
#7 entityreference_prepare_view_access_query_alter-2333393-7.patch1.07 KBSolthun
#6 entityreference_prepare_view_access_query_alter-2333393-4.patch1.18 KBSolthun
#4 entityreference_hook_prepare_view_alter-2333393-3.patch1.12 KBchaby
#1 entityreference_hook_prepare_view_alter-2333393-1.patch2.03 KBchaby

Comments

chaby’s picture

chaby commented
Status: Active » Needs review
StatusFileSize
new entityreference_hook_prepare_view_alter-2333393-1.patch2.03 KB
SebCorbin’s picture

SebCorbin commented
Status: Needs review » Needs work 
+++ b/entityreference.module
@@ -1215,6 +1211,23 @@ function entityreference_field_formatter_prepare_view($entity_type, $entities, $
+  if ($target_entities && count(module_implements('entityreference_prepare_view_access_alter')) > 0) {
+    drupal_alter('entityreference_prepare_view_access', $items, $target_entities, $entities, $field['settings']['target_type']);
+  }

Hmm, this could hack the behavior of other modules, i.e. if any hook_entityreference_prepare_view_access_alter() is implemented, modules relying on entity_access aka the old way, hook_entity_access/hook_node_access will not be called.

Thus, I would let the default behavior as it is, and add the drupal_alter() after so that we can override $items

chaby’s picture

chaby commented

Hmm, this could hack the behavior of other modules

Indeed you are right but it was intented as explain in the issue summary :

For performance reason, we cannot apply hook_node_access() (more globally entity_access()) per entity, as called by entityreference_field_formatter_prepare_view(). We must do it in bulk, to reduce the number of SQL queries executed (1 instead of X references)

However, bypassing other modules behaviours is indeed not a good idea and furthermore, the hook name is not correct (by design, an alter means you change data previously set, not an alternative)..
If it would be a real hook alter (so called after default behaviour), we cannot implement hook_node_access() for full view node page in our use case (and did'nt find another way to achieve it).
Anyway, keep in mind that if other modules make some queries for view op in entity_access, return false and user has update access, it would execute X references queries for nothing...

chaby’s picture

chaby commented
StatusFileSize
new entityreference_hook_prepare_view_alter-2333393-3.patch1.12 KB

Take an another approach : use a pre-filtered query with tag to let's other modules remove referenced entities where access should be denied.
Allowed references would be still called with entity_access() but the implemented hooks such as node_access() could play with ugly static cache to check that access was previously allowed (i.e not excluded) and prevent redundant check for performance.

chaby’s picture

chaby commented
Status: Needs work » Needs review
Solthun’s picture

Solthun commented
StatusFileSize
new entityreference_prepare_view_access_query_alter-2333393-4.patch1.18 KB

I tried the latest approach, but ran into the problem of not making anydifference when the actual query tag alter was removing each referenced item.
Added a small change to solve this.

Solthun’s picture

Solthun commented
StatusFileSize
new entityreference_prepare_view_access_query_alter-2333393-7.patch1.07 KB

Re-rolled this lonely patch.