Every node that a user can access, can be loaded.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 354092_11_services_get_node_check_node_access.patch | 1.25 KB | amitaibu |
| #10 | 354092_10_services_get_node_check_node_access.patch | 909 bytes | amitaibu |
| services_1_node_access_service.patch | 1.26 KB | amitaibu |
Comments
Comment #1
marcingy commentednode_access alreay does this so the additional is not required
Comment #2
amitaibuI think This is not correct, for example:
1) Deny access content to anon users
2) Allow 'load any node data' to anon users
3) Try to get.Node - anon user will be able to view the node.
The current node_service_get_access() doesn't check node_access.
Comment #3
marcingy commentedWhich is the same as standard Drupal. To overcome this issue you need to install a content access module which allows more ganular control of node grants.
Comment #4
amitaibuI accept this statement. I'd just like to refine my use case. I'm using organic groups. Because of the nature of Drupal's access system you can only grant access - you can't really deny.
So installing OG and content access module for example, is problematic.
Having this service, might simplify how things are handled - and since service already has an access mechanism I think it's appropriate to use it to deny access, same as we are doing with (i.e. 'load own node data').
With that said, I of course respect any decision you take :)
Comment #5
marcingy commentedDruapl access system can be used to deny as well as grant access - if a grant isn't present in the node_access table then access is denied. Content_access combined with ACL allows per user and per role access. The way to think of this is how would you enforce it if the services module wasn't involved. Then once those controls are in place assuming it utilises node access services will then respect those access controls.
Comment #6
amitaibu1) That is correct, however if there are several rows in node_access - having a single row with grant is enough - another module can't 'deny', that's what make it problematic in using several node_access modules, the fact that node access is using OR and can't use AND.
2) Either way, if services doesn't wish to implement it's own access_check then 'Load own node' isn't following this concept.
btw, Are you in the IRC?
Comment #7
amitaibuI was thinking about #3, and I think the problem is that Services simple bypasses node access...
The user was not allowed to access content, but she still can using services.
The whole statement should be something like
Comment #8
marcingy commentedThanks for this input and now that it put this way it makes prefect sense, and I now understand what the issue is.
Can you roll a patch and I'll get this committed as quickly as possible.
Comment #9
marcingy commentedComment #10
amitaibuattached patch.
Comment #11
amitaibuAfter IRC with RobLoach, this patch makes it even more sane, and let's other content access module the chance to work - by keeping only a single permission 'load node data'.
Comment #12
snelson commentedWierd. If you look at Services for D5, this is exactly how its doing it. Don't know how that got changed.
Comment #13
snelson commentedCommited to D6.
Thanks Amitaibu
Comment #14
amitaibuThanks snelson :) 2 remakrs:
1) Maybe there should be a security announcement about this node access bypass.
2) I see you commited it with 'and' instead of '&&' - however core uses the later.
Comment #15
snelson commentedI agree about the security announcement. Let me finish up a few more things, get a release out, and I'll contact security team.
"and" and "or" is used in several places throughout the module. But, I agree, we should keep up with Drupal coding standards. I'll do a separate patch that takes care of all these.