This module is to user_relationships as private_nodes is to buddylist.

It allows permitted viewers of a node to be restricted to those with at least one defined relationship to the author on a per node basis.

This module was sponsored by Jay Julian at kakool.com

CommentFileSizeAuthor
#12 user_relationships_node_access_0.diff3.18 KBsprsquish
#9 user_relationship_node_access.diff7.58 KBsprsquish
#8 user_relationships_private_nodes.diff4.92 KBjbrown
#7 user_relationships_private_nodes.diff4.98 KBjbrown
#5 user_relationships_private_nodes.diff4.99 KBjbrown
user_relationships_private_nodes.diff5.01 KBjbrown

Comments

sprsquish’s picture

sprsquish commented

Hey, thanks for this.

There are some issues though. They're the same problems I ran in to when I was trying to write this very same plugin type.

When node_access is called the op and node are passed in. After some checks node_access_grants is called. This is basically a wrapper around hook_node_grants that ensures a uid is set. If a user isn't passed in node_access_grants sets it as the current user then passes that on to all node_grants hooks. The user object and the operation are sent to the hooks. In this case it's the current user and to operation they'd like to perform. So the patch you sent looks up all the relationships the current user has, adds them to the list then adds the current user to the list. In essence you're giving access to the current user and all their friends.

Like I said, I've hit these same problems before. I don't really see them being solved well without the node being passed into hook_node_grants

sprsquish’s picture

sprsquish commented
Status: Needs review » Needs work
jbrown’s picture

jbrown commented

I don't think you understand the node access system correctly. Its pretty much the hardest thing to get your head around in Drupal.
http://api.drupal.org/api/group/node_access/5

Here is how my module works:

user_relationship_private_nodes_node_access_records() is called when the node is saved to determine the author of the node (gid - grant id) and what relationships (realms) the viewer can have to be permitted to view the node. This information is stored in {node_access}.

When the nodes are to be viewed, user_relationship_private_nodes_node_grants() is called to determine what relationships (realms and gids) the user who is attempting to view the node has.

node_access() or (db_rewrite_sql() for listings) then looks up {node_access} to determine if the viewer has at least one realm with the gid (uid) of the node author.

sprsquish’s picture

sprsquish commented

oooohhhh.. okay, I think I see it now. It does work, it's just really inefficient, which is really no fault of yours.

Then the only issue I have with the module is that it queries the user_relationships and user_relationship_types tables directly. It should use the API for those operations.

jbrown’s picture

jbrown commented
StatusFileSize
new user_relationships_private_nodes.diff4.99 KB

I think it is really efficient. The Drupal node access system is really clever.

user_relationship_private_nodes_node_access_records() is only called when saving the node or rebuilding {node_access} (when the modules page gets submitted).

user_relationship_private_nodes_node_grants() is only called once per page view. When viewing a node listing db_rewrite_sql() modifies the query to join with {node_access}. Obviously there is overhead, but it is considerably quicker than doing a query per node.

I have attached an updated version of the module. It only does one query in user_relationship_private_nodes_node_grants().

What API functions do you recommend I use?

jbrown’s picture

jbrown commented

by the way, the Devel Node Access module in the devel package is really useful when trying to understand the drupal node access system

jbrown’s picture

jbrown commented
StatusFileSize
new user_relationships_private_nodes.diff4.98 KB

The was a minor bug where sometimes the node author could not view the node. This is now fixed.

jbrown’s picture

jbrown commented
StatusFileSize
new user_relationships_private_nodes.diff4.92 KB

Okay - latest version.

It now uses user_relationships_types_load() and user_relationships_load().

I think it would be great if:

  1. user_relationships_types_load() had a sort parameter
  2. user_relationships_load() defaulted to approved relationships
  3. user_relationships_load() did not join with {user_relationship_types} unnecessarily

The module keeps getting smaller!

sprsquish’s picture

sprsquish commented
Status: Needs work » Needs review
StatusFileSize
new user_relationship_node_access.diff7.58 KB

Thank you so much for your work on this. I'm gonna dig deeper in to how the node access system works cause I clearly don't understand it well enough.

After I got done with it, the module actually grew and got a name change. I added support for update and delete and made the name more generic. I also added support for one-way relationships.

Here's the patch, let me know if it works for you and what you think. I'll commit it after I get your okay.

BTW, if you're using git I have some invites to GitHub. I'll send it over if you want it, then we wont need this patch process.

Regarding your other requests:

  1. I hadn't considered that only because I never envisioned large numbers of relationship types. The types_load function caches the types with a static var so when you call either types_load or type_load the database is only hit once and all types are cached
  2. I've heard this request once before and I might consider implementing it if I hear more. To me a function called user_relationships_load has no bias and shouldn't have a bias. It should just load the relationships. The caller needs be specific.
  3. I think the default of joining to the table is the right way to go. In the vast majority of cases, the relationship type data is useful. Maybe adding an option to only pull from the UR table would work.
jbrown’s picture

jbrown commented

I like it.

Why do you only allow the node author to change the node access permissions? This even prevents uid 1 from changing it. Surely if a user has the right to edit the node, then they should be able to edit the node access permissions.

There should be a user permission called something like 'control user relationship node access'.

Also, I think the view/edit/delete permissions should be in table format with each relationship type having a row. A bit like on admin/user/access .

There could also be a node type configuration where the admin can configure whether each of view/edit/delete node access permissions are allowed for that node type.

The other points:

  1. Maybe it should always sort them in alphabetical order and have no option.
  2. I Disagree. user_relationships_load() should return real relationships, potential relationships should only be returned if specifically asked.
  3. in _user_relationships_generate_query() you could check if the types table is used and only perform the join if necessary. Or maybe mysql performs this optimization automatically?
jbrown’s picture

jbrown commented

Also, see this: http://groups.drupal.org/node/4261

sprsquish’s picture

sprsquish commented
StatusFileSize
new user_relationships_node_access_0.diff3.18 KB

Thinking back on it, I'm not exactly sure why I set it up so only the author could set permissions. That's been redacted.

I added the permission. I think it's a good way to go.

Don't know why I hadn't thought of using a table for the view. I was stuck on the Content Access way of doing it (floated divs) and didn't like that so much.

I'll work out the more fine grained permissions later

  1. Great idea. Done
  2. I'm gonna mull this over a bit more. I can see both sides of the argument and I'm not yet convinced either way. I think it's fine for now, but if I hear more requests I'll probably change it.
  3. I joined the types table to keep down the DB hits and automatically include the type information. Maybe hitting it twice (once for ur_load and again to load all types) then merging the data might be the way to go. I'll look in to this more. I only hesitate because I use the query builder directly so I can do pagination.

Attached is a diff of only the changes listed above.

sprsquish’s picture

sprsquish commented
Status: Needs review » Closed (fixed)

Added to both the 2.x and 3.x branches

sprsquish’s picture

sprsquish commented

and now 6.x-1.x

jbrown’s picture

jbrown commented

It was great working with you.

mariusooms’s picture

mariusooms commented
Version: 5.x-2.x-dev » 6.x-1.0-beta7

Hi...I realize it it's 4 months later now, but quoting jbrown:

"There could also be a node type configuration where the admin can configure whether each of view/edit/delete node access permissions are allowed for that node type."

Will this still be implemented? It's a bit silly when I have all the permissions for the node module set up so that users can only create, edit and delete their own content. So in the node view I'd only like to see the option 'view'. The other two choices don't apply, so why even list them. Does that make any sense?

Kind regards,

Marius