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.

CommentFileSizeAuthor
#2 services-1059132.patch785 byteskylebrowning

Comments

kylebrowning’s picture

Category: support » bug
Priority: Normal » Major

Yes, this is a bug,

Marking as MAjor since this is security related.

kylebrowning’s picture

Status: Active » Needs review
StatusFileSize
new785 bytes

This patch fixes that.
Please review

gdd’s picture

Status: Needs review » Needs work

When 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.

kylebrowning’s picture

Status: Needs work » Closed (works as designed)