Steps to reproduce:
- Enable content access (any module implementing hook_node_grants should work)
- Enable views and entity-reference modules
- Create two content types, let the first one reference the other through a entity-reference field
- Create some content of the first content type, but don't bother adding any relations
- Create a view with a relation through an entity-reference, don't mark this relation as required (LEFT JOIN)
Without the 'bybass node access' privilege, users will not get any results in this view.
It's easier to illustrate with an example.
Something like could for example be appended to the node query:
... AND ( EXISTS (SELECT na.nid AS nid
FROM
node_access na
WHERE (( (na.gid = '0') AND (na.realm = 'all') )OR( (na.gid = '0') AND (na.realm = 'content_access_author') )OR( (na.gid = '1') AND (na.realm = 'content_access_rid') ))AND (na.grant_view >= '1') AND( node_field_data_field_person.nid = na.nid))
where node_field_data_field_person is the referenced base table of a "person" content type referenced through an entity-refrence field. node_field_data_field_person.nid will in the case where no such relation exists be NULL (because of the LEFT JOIN from the node table). This will cause the EXISTS condition to evaluate to FALSE, and the row to be excluded.
To remedy this the query should be written like this to protect against the case where node_field_data_field_person.nid IS NULL.
... AND ( EXISTS (SELECT na.nid AS nid
FROM
node_access na
WHERE (( (na.gid = '0') AND (na.realm = 'all') )OR( (na.gid = '0') AND (na.realm = 'content_access_author') )OR( (na.gid = '1') AND (na.realm = 'content_access_rid') ))AND (na.grant_view >= '1') AND( (node_field_data_field_person.nid IS NULL ) OR (node_field_data_field_person.nid = na.nid) )))
I will attach a patch with the needed changes.
Comment | File | Size | Author |
---|---|---|---|
#1 | node-access-left-join-relations-2160469-1.patch | 1.13 KB | gnucifer |
Comments
Comment #1
gnucifer CreditAttribution: gnucifer commentedComment #2
gnucifer CreditAttribution: gnucifer commentedComment #3
gnucifer CreditAttribution: gnucifer commentedHave been running this in production on about 100 sites for a couple of weeks now, no issues. Any chance of it being reviewed and hopefully committed?
Comment #4
gnucifer CreditAttribution: gnucifer commentedAny chance this patch can get some love? I'm sure there are many people affected by this bug, in our production environment there are about 4-5 sites that will bail out without the fix.
Comment #5
fengtanWe are also experiencing this problem.
Tested patch #1, confirmed it fixes the problem. Thanks.
The code makes sense to me. Would be great if someone from the core team could have a look.
Comment #6
spheresh CreditAttribution: spheresh commentedPatch was applied to my site. It works great.
Thanks.
Comment #8
spheresh CreditAttribution: spheresh commentedHi, guys. The required patch have been waiting to be approved already year
Does No one not use this functionality? Have you not using this functionality?
Comment #9
mgifford@spheresh want to mark it RTBC?
Does this also fail in D8? If so it needs to be fixed in D8 first and then backported.
Comment #10
David_Rothstein CreditAttribution: David_Rothstein commentedI am not positive, but I think this might be a duplicate of #1969208: Query node access alter filtering out accesible nodes?
Comment #11
mgiffordWe should test if this also fails in D8.
Comment #12
mgiffordComment #13
benjifisherI agree with the comment in #10: this is a duplicate. The patches on the two issues are similar, and we should decide which is the better fix.
There has already been some discussion on #1969208: Query node access alter filtering out accesible nodes of whether the problem still exists in D8. I think the answer is that it does, but there is another bug in Views that will have to be fixed before we can address this issue in D8.
Like others posting on this issue, I have patched Drupal core on a live site (just one) because of this problem. But I also know the policy: since we do not want to introduce regressions when moving to D8, we have to fix it there before the change will be accepted into Drupal core.
Comment #14
valthebaldIt seems that everyone has agreed this is a duplicate of #1969208: Query node access alter filtering out accesible nodes (and the latter is more actively discussed)