I'm modeling my REST resources on the logic given in the various 'standard' resources that come with Services 3.0. Specifically, I have a series of custom content types that an API client can do CRUD operations with, where I pass to the clients "managed versions" of these content types - meaning I'm not exposing the raw Drupal node to the clients, I give them a version with only the fields they need to care about for the service my company publishes.
Anyway, for my logic, I based my CRUD callbacks on the logic inside node_resource.inc, which has this code for handling field level permissions in the Create callback:
if (module_exists('content') && variable_get('services_use_content_permissions', TRUE)) {
$fields = content_fields(NULL, $node->type);
foreach ($fields as $field_name => $field_info) {
if (isset($node->$field_name)) {
$access = module_invoke_all('field_access', 'view', $field_info, $user, $node);
if (in_array(FALSE, $access)) {
unset($node->$field_name);
}
}
}
}
I just noticed that this same logic does not exist for the Update operation in node_resource.inc. Is that an oversight? It seems to me, unless I'm mistaken, that without the field level permissions logic in the Update operation, field level permissions could be violated with an Update.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | services-1059132.patch | 785 bytes | kylebrowning |
Comments
Comment #1
kylebrowning commentedYes, this is a bug,
Marking as MAjor since this is security related.
Comment #2
kylebrowning commentedThis patch fixes that.
Please review
Comment #3
gddWhen you submit a node through Services, it is passed through drupal_execute() which replicates a form submission. Since the form submission process happens, all the same permission checks will happen as well. However when you view a node through Services, we just call node_load() which does not pass through the same process as when you view a node on the page. Therefore we make these modifications by hand.
If you experience behavior different than this, please let us know. However I don't believe any patch is necessary.
Comment #4
kylebrowning commented