Steps to reproduce:
* Grant anonymous users the permission "Access the resource node".
* Add node 1, publish it
* Add node 2, unpublish it
* Visit /node.xml as anonymous and observe that both nodes are listed. This should not be possible as the anonymous user is not allowed to see unpublished nodes.

Solution: the resource controller needs to check for access. Maybe EntityFieldQuery supports something, in the worst case we have to run entity_access() on every item.

And a test case would be good.

CommentFileSizeAuthor
#10 1717410-querying-access_nodes2.patch2.35 KBsepgil
#8 1717228-xml-create.patch2.62 KBsepgil
#4 1717410-querying-access_nodes.patch893 bytessepgil
#2 1717410-querying-access.patch2.29 KBsepgil

Comments

fago’s picture

fago
Primary language German
Location Vienna
commented

efq usually applies node access grants by default (what sucks), look for the respective tags.

sepgil’s picture

sepgil commented
Status: Active » Needs review
StatusFileSize
new 1717410-querying-access.patch2.29 KB

Couldn't find any other solution, than to call RestWSResourceControllerInterface::access() for each entity efq has retrieved. However I'm not sure, what to do if the user no access to a node. For now it simply filters those entities out.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

what about paging? Filtering out entities afterwards will clash with the count query for paging, right?

And as fago said EFQ should use the node access tag per default, why does that not work in our case?

sepgil’s picture

sepgil commented
Status: Needs work » Needs review
StatusFileSize
new 1717410-querying-access_nodes.patch893 bytes

You are right, the patch above will break the paging.
node_access works just for nodes, and we should respect the permissions for all entities, but I currently don't know how.
This new patch, add the node_access tag, which has to be set explicit for each query.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work 
+++ b/restws.entity.inc
@@ -234,6 +234,10 @@ class RestWSEntityResourceController implements RestWSQueryResourceControllerInt
 
+    // Add node_access, so that at least the node permission are respected.
+    // todo: find a solution for all entities.
+    $query->addTag('node_access');
+

The tag should only be added if the resource type is node.

And this needs a test case. An unpublished node must not show up in query results for unprivileged users.

sepgil’s picture

sepgil commented

-removed-

sepgil’s picture

sepgil commented
Status: Needs work » Needs review

After a lot of debugging klausi and I found out that node_access doesn't checks for published/unpublished node (details here). So this patches checks if the user lacks the right permission and adds a propertyCondition() with status=1. I ignored the permission view own unpublished content, since we would have to check it against each loaded node...

sepgil’s picture

sepgil commented
StatusFileSize
new 1717228-xml-create.patch2.62 KB

forgot to post the patch

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

wrong patch?

sepgil’s picture

sepgil commented
Status: Needs work » Needs review
StatusFileSize
new 1717410-querying-access_nodes2.patch2.35 KB

Got confused with all the similiar issue numbers, sorry.
here is the right patch.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Committed a slightly modified version of this patch.

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