First, workbench is awesome.

Second, I understand that workbench is not designed to restrict the ability to view content. (I'm not sure I'm ready to create an extension module that handles this for Workbench, although I'd be willing to try.) In the meantime, I'm trying to use workbench to help an organization that wants to use Drupal for employee performance reviews. Using a taxonomy heirarchy of CEO > Manager > Employee, I have been able to hit most of the requirements. Doing something like this, I think I've restricted access to performance reviews between employees:

function review_node_access($node, $op, $account) {
  $type = is_string($node) ? $node : $node->type;
  $employee_role_id = 4;
  $ceo_role_id = 6;

  if (isset($node->field_manager['und'][0]['uid'])) {
    $manager_uid = $node->field_manager['und'][0]['uid'];
  }
  else {
    $manager_uid = NULL;
  }

  if ($type == 'annual_review') {
    // Only employees should create reviews because employees can only edit a review they created
    if ($op == 'create' && (isset($account->roles[$employee_role_id]))) {
      return NODE_ACCESS_ALLOW;
    }
    // Only node author (employee), manager indicated on node, or CEO can view or edit a review
    if (($op == 'view' || $op == 'update')
      && ($account->uid == $node->uid
      || $account->uid == $manager_uid
      || isset($account->roles[$ceo_role_id]))) {
      return NODE_ACCESS_ALLOW;
    }
    // Only manager listed on node and CEO can delete a review
    if (($op == 'delete') 
      && ($account->uid == $manager_uid
      || isset($account->roles[$ceo_role_id]))) {
      return NODE_ACCESS_ALLOW;
    }
    return NODE_ACCESS_DENY;
 }

However, I want to go a step further and restrict viewing access between departments. I'm using Devel Node Access to debug the permissions, and it tells me that this code (which comes before the code above) is working -- that the employees of one department can't view/update/delete the content of other departments:

  // Only allow department employees to view their department's content
  if (in_array($op, array('view', 'update', 'delete'))) {
    if (isset($node->field_department['und'][0]['nid']) && isset($account->field_department['und'][0]['nid'])) {
      if ($node->field_department['und'][0]['nid'] <> $account->field_department['und'][0]['nid']) {
        return NODE_ACCESS_DENY;
      }
    }
  }

But it does not actually work, so I was looking over your API and tried doing something like this:

function review_workbench_access_user_alter(&$access, $account) {
  $node = menu_get_object();
  $types = node_type_get_types();
  if (isset($node->field_department['und'][0]['nid']) && isset($account->field_department['und'][0]['nid'])) {
    if ($node->field_department['und'][0]['nid'] <> $account->field_department['und'][0]['nid']) {
      foreach ($access as $id => $data) {
        $access[$id]['view'] = array();
        foreach ($types as $type => $value) {
          $access[$id]['view'][$type] = 0;
        }
      }
    }
  }
}

And that doesn't work, either, and obviously I'm doing something wrong. So this is my question: assuming every relevant node and user has a "department" field, should I be trying to restrict using hook_node_access or hook_workbench_access_user_alter? Or am I completely on the wrong track here?

Thanks,
Matthew

Comments

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Are you trying to restrict the View of a single node (e.g. node/*) or a _list_ of nodes? hook_node_access() doesn't operate on lists.

That second chunk of code doesn't really depend on WA in any way. It looks like a straight-up hook_node_access() rule based on your own field data. I would think you want to return DENY in cases where the values are not set as well.

In essence, DENY should be your default state, and IGNORE would be the exception IFF both values match. This is the reverse of your current logic.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented

@agentrickard Thanks for the prompt reply. I tried reversing my logic, and still ran into problems. It looks like it might be an issue with DNA, because I'm getting the (hover) message: "Core seems to disagree on this item. This is a bug in either DNA or Core and should be fixed! Try to look at this node as this user and check whether there is still disagreement."

Nonetheless, since I will need to restrict the view of both a single node as well as a list of nodes, I'm reading up (in your book, possibly even your chapter) on hook_node_grants_alter and hook_node_access_records_alter, and I'll try it that way.

I was starting to worry that workbench access might have been somehow modifying my NODE_ACCESS_DENY or that I really should have been using the workbench access API. I'm feeling rather confident that I'll find success using grants rather than hook_node_access().

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

hook_node_grants_alter() and hook_node_access_records_alter() are not relevant unless you are using a Node Access module (this is covered in the book.)

Workbench Access will either return NODE_ACCESS_IGNORE or NODE_ACCESS_DENY, and always returns IGNORE on 'view' if the node is published. If the node is not published, it returns IGNORE if you can edit the node.

So unless you're dealing with unpublished nodes, you should be able to handle this in your hook_node_access() implimentatation by returning DENY if the node is not in the current user's group. I don't think you should need to _alter anything.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented

Thanks for keeping with me on this. I thought hook_node_access() does not restrict nodes from appearing in lists. I've been able to restrict viewing/editing/deleting nodes using code like this in my implementation of hook_node_access():

  $user_fields = &drupal_static(__FUNCTION__);
  if (!isset($user_fields)) {
    $user_fields = user_load($account->uid);
  }
  if ($op != 'create') {
    // Both user and content must have something in the department field
    if (empty($node->field_department['und'][0]['nid']) || empty($user_fields->field_department['und'][0]['nid'])) {
      return NODE_ACCESS_DENY;
    }
    // Only allow employees to view/edit/delete their department's content
    if ($node->field_department['und'][0]['nid'] != $user_fields->field_department['und'][0]['nid']) {
      drupal_set_message("Sorry, you cannot view this content because it was created by another department.", 'warning', FALSE);
      return NODE_ACCESS_DENY;
    }
    return NODE_ACCESS_IGNORE;
  }

But to keep this at least somewhat Workbench Access related, the above code doesn't stop the (published) content from appearing in the 'All Recent Content' section of 'My Workbench' in employees from another department. So maybe I'm missing something, but it seems like neither workbench access nor hook_node_access() will help me meet this requirement.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

You need hook_query_alter() for lists.

Specifically, mymodule_query_node_access_alter().

freddura’s picture

freddura commented

I have had great success in integrating the following module with Workbench Access to control both editorial access and viewership access through taxonomy structure!

Access By Term

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

ABT largely looks like a functional duplicate of this module.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Active » Closed (duplicate)