I have a term in my vocabulary called 'cartoons' in this term there are only nodes of the type 'cartoon' (generated by a cartoon harvester I've written). To view this node type you need to have access to view it.
implemented like this:


/**
 * Implementation of hook_perm().
 */
function zbz5_cartoons_perm() {
  return array('view cartoons', 'add cartoons', 'edit own cartoons', 'administer cartoons');
}

/**
 * Implementation of hook_access().
 */
function zbz5_cartoon_access($op, $node) {
  global $user;
  switch ($op) {
    case 'view':
      return user_access('view cartoons');
      break;

But when accessing the listing of nodes in the term 'cartoons' without permission, all I get is a messy screen scattered with access denied messages, with the forbidden cartoons at the bottom of the screen for everyone to see.
Is this a bug in Drupal, or have I done something wrong in my module?

links:
the bad listing: http://www.zbz5.net/cartoons
the module: http://www.zbz5.net/zbz5_cartoon_aggregator

Comments

rjl’s picture

I am having a similar issue where I've created a node type with the same sort of 'view' permission, but nodes of that type show up for all users in various node listings. In drupaldocs there is this post about node access. There is something about a db_rewrite_sql function that should be used. I haven't had time to really look into this function yet, but maybe it is helpful for you.

As for your problem of the access denied, in your implementation of hook_view(), you have...

  if (!user_access('view cartoons')) {
    drupal_access_denied();
  }

As far as I understand, hook_view() seems mostly to be used for altering the node body or teaser. This hook function is also called by a number of other modules via implementing hook_nodeapi(), thus allowing a module like event to show event dates in the node body. I have seen a few modules run an sql statement also with hook_view. The drupal_access_denied() doesn't seem appropriate here, as you well know. I wish I had solution for you.

Hoping someone else may have some better insights.

Vidarls’s picture

Thank you, I didn't realize the problem in hook_view until you pointed me there.

But I still can't get my head around access control in node listings. I just don't understand what db_rewrite_sql() has to do with access control.

From http://drupaldocs.org/api/head/group/node_access:

In node listings, the process above is followed except that hook_access() is not called on each node for performance reasons and for proper functioning of the pager system. When adding a node listing to your module, be sure to use db_rewrite_sql () to add the appropriate clauses to your query for access checks.

-Olegu
http://www.zbz5.net/

rjl’s picture

More information that I have been looking at and trying to understand. At drupaldocs - hook_db_rewrite_sql. This function allows you to add JOIN and WHERE clauses to an SQL string.

So what I think happens is that a module that does a node listing, such as tracker or taxonomy, creates a basic sql string for the nodes it wants to display. Then it runs this sql string thru the db_rewrite_sql() function which calls any modules that implement the hook. The hook, if implemented in a module, can add JOIN and WHERE clauses to the sql.

As your quote states hook_access is not called on each node for performance reasons. So I think you need to do an implimentation of hook_db_rewrite_sql(). My best guess from the docs is that it would be something like this...

function zbz5_cartoon_db_rewrite_sql($query, $primary_table, $primary_field, $args) {
  switch ($primary_field) { 
    case 'nid': 
      // this query deals with node objects       
      $return = array(); 
      if (!user_access('view cartoons')) {
        if ($primary_table != 'n') { 
          $return['join'] = "LEFT JOIN {node} n ON $primary_table.nid = n.nid"; 
        } 
        $return['where'] = "n.type <> 'zbz5_cartoon'"; 
      }
      return $return; 
      break; 
    case 'tid': 
      // this query deals with taxonomy objects 
      break; 
    case 'vid': 
      // this query deals with vocabulary objects 
      break; 
  } 
}

And if I am guessing correctly... this function would check for 'view cartoons' access, if not, then add the node table to the JOIN clause (if it wasn't already there), and add node type not equal to 'zbz5_cartoon' to the WHERE clause. I wish I had time to test this out on my own module, because maybe this is starting to make a little sense to me, I will definately try it this weekend.

Hope that I am being helpful. It is helpful to me to think this thru.

somebodysysop’s picture

I tried your db_rewrite_sql code above and it works. However, users with the "access my_node_type" permission can list all nodes of that type, where I actually only want them to be able to list only the nodes they own, e.g. "$user->uid = $node->uid".

I've been playing around, but can't seem to find a way to do it, if it's at all possible.

Is it?

somebodysysop’s picture

Here's the code I came up with to NOT list a particular node type unless the user owns it:

/**
 * Implementation of hook_db_rewrite_sql()
 */
function zimbra_import_db_rewrite_sql($query, $primary_table, $primary_field, $args) {
  global $user;
  
  // returning an empty array() means "do not rewrite any SQL"
  // else, return node ONLY if it belongs to this user

  switch ($primary_field) {
    case 'nid':
      // this query deals with node objects      
      $return = array();
      if ($primary_table != 'n') {
        $return['join'] = "LEFT JOIN {node} n ON $primary_table.nid = n.nid";
      }
  // This code will NOT list zimbra_import type by default
  // But, will list node if this user is it's author
  // The only problem here is if there is a node this user creates that should
  // NOT be listed.
      $return['where'] = "n.type <> 'zimbra_import' OR n.uid = " . $user->uid;
      return $return;
      break;
  } 
}

This is working, but I'm not sure about the $user->uid part. Is there a circumstance where we do NOT want a user to see a node he has created?

rjl’s picture

Here's another way of going about it.

$return['where'] = "NOT (n.type = 'zimbra_import' AND n.uid <> ". $user->uid .")";

In a truth table using these statements:
n.type='zimbra_import' (T below)
n.uid=$user->uid (U below)
----------------------
T : U : desired result
----------------------
1 : 1 : 1 (both are true - these nodes OK to view)
1 : 0 : 0 (type is true, but not user, these nodes need to be blocked)
0 : 1 : 1 (type is false, this module should let them pass)
0 : 0 : 1 (same note as above)
-----------------------

One issue though is that you don't have an over-ride, so this will limit the results for the admin user (uid=1) as well

somebodysysop’s picture

My code actually turned out not to work. It worked for the list presented in "recent posts" fine. But, it whacked out the "Groups" (I have OG installed) listing.

So, I decided to try using node grants. I actually used the hook_node_access_records and hook_node_grants examples from user_access_example.module http://api.drupal.org/api/5/file/developer/examples/node_access_example..... By making the grant realm access for this node type automatically false, but creating a grant id for the author giving him access, I am able to get the functionality I desire: That is, only node authors can view OR list the nodes they create.

Thanks for the response! I will try to use your code the next time.

rjl’s picture

I don't know if you've had a chance to try out that code I posted above, but I got curious and decided to blow off this meeting I had tonight so that I could try it out on my own module. I've only done some preliminary tests with tracker, but it seems to be working, my node or my type don't seem to appear on tracker lists for users without the special 'view' permission, but for users who do, the nodes do display.

This is very excting. Your post really made me start thinking harder about this.

Vidarls’s picture

Thank you very much!!
I got it to work now, *hurray*
The downside is that I just can't get my head around why/ how it works.

if ($primary_table != 'n') {
          $return['join'] = "LEFT JOIN {node} n ON $primary_table.nid = n.nid";
        }
        $return['where'] = "n.type <> 'zbz5_cartoon'";

I understand that if you've not got access to cartoons a "WHERE type != cartoons" is returned. In my head that where clause should be enough.
Why the join, what's it good for??

And again, thank you very much for helping me out.

-Olegu
http://www.zbz5.net/

rjl’s picture

As far as I understand it, when another module implements a node listing it doesn't necessarily have to include the node table in it's SQL statement (off the top of my head, it would seem strange not to, but I guess the possibility exists).

The if statement checks to see if the node table is included in the SQL, if not, we need to include it in the FROM clause otherwise our WHERE clause would return an SQL table not found error.

Vidarls’s picture

Ok, I get it.
Hopefully noone uses a non standard alias in their sql statement then :P

Thank you very much for enlightening me.

-Olegu
http://www.zbz5.net/

Ashraf Amayreh’s picture

But what if we want to change taxonomy's query, but not some other module's query?

Is there a way to specify that when implementing hook_db_rewrite_sql?

--
CEO | O-Minds
Skype. aamayreh
o-minds.com

Connect with us on Twitter or LinkedIn.

rjl’s picture

not sure what you mean, can you give an example?