In spite of the independent 'load raw node data' permission in the node service module, you must still enable 'access content' for anonymous users if you want anonymous external sites to be able to load nodes. This is a problem if you want the site to be private, which I guess the 'load raw node data' permission was intended to overcome, but it doesn't appear to function correctly. The offending code is here:

function node_service_load_access($nid) {
  $node = node_load($nid);
  return node_access('view', $node) && user_access('load raw node data');
}

To my mind that should be an *or*, not an *and* ... 

function node_service_load_access($nid) {
  $node = node_load($nid);
  return node_access('view', $node) || user_access('load raw node data');
}

E.g. if the requesting user has permission to view the node *or* has permission to load raw node data. This way I can set site access off but still allow service to work. I can't think of any use case for this to be && as it is now?

Comments

brmassa’s picture

brmassa commented
Assigned: Unassigned » brmassa
Status: Active » Needs review

Greg,

the permission "load raw node data" is important because the API returns all node data, including possible sensitive data. Its not the same as viewing the node, since the admin might control the information to be displayed.

in fact. i believe that "view" permission is irrelevant here. The only possible benefit from calling it is that if node_access() has any special custom validation. If so, its important to maintain it. If not, i might delete the node_access check altogether.

regards,

massa

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

I understand.

Hmmm, you make a good point about people perhaps wanting to set more granular access using other access modules like taxonomy_access, or even groups, e.g. with the node_access check in place you could be sure your service would not expose nodes in a private group from the OG module.

In that case, I propose is this is an option in admin. Just set a boolean variable, on or off, for this behaviour. It'll take me about 15 minutes to write if you think it's a good idea? I can use hook_form_alter to make it an additional security option when node_service.module is enabled on admin/build/services/settings ?

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

How's this?

/**
 * Implementation of hook_form_alter().
 */
function node_service_form_alter(&$form, $form_state, $form_id) {
  if ($form_id == 'services_admin_settings') {
    $form['security']['node_access']['#title'] = t('Check node access settings when loading node data');
    $form['security']['node_access']['#type'] = 'checkbox';
    $form['security']['node_access']['#description'] = t('When enabled, the node.load method will perform an additional node access view check before providing the node data.');
    if ((bool)variable_get('node_service_node_access', true)){
      $form['security']['node_access']['#default_value'] = 1;
    }
    $form['#submit'][] = 'node_service_save_access_setting';
  }
}

function node_service_save_access_setting($form_id, $form_values) {
  if ($form_values['values']['node_access'] == 0) {
    variable_set('node_service_node_access', false);
    drupal_set_message(t('Node access check disabled for node.load method.'));
  } else {
    variable_set('node_service_node_access', true);
  }
}

function node_service_load_access($nid) {
  if ((bool)variable_get('node_service_node_access', true)){
    $node = node_load($nid);
    return node_access('view', $node) && user_access('load raw node data');
  } else {
    return user_access('load raw node data');
  }  
}
brmassa’s picture

brmassa commented
Status: Needs review » Fixed

Greg,

nice. i commited it on CVS. Its not the definitive solution, once it forbid admins to use both situations. Its probably better to create another service for more flexible checks, but i might do this later.

regards,

massa

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Cool, thanks. I can't use CVS here because the port is blocked. =(

I've applied to IT to have it opened, then I'll be able to commit myself.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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