This has been discussed in #342579: Maintaining permissions sanity when node access is updated and probably a couple other issues. I think it may be time to revisit this as it has been a while since the other issue has been closed out.

The problem is that this module is practically useless as the only access control module on a site. Attempting to use multiple access control modules is not for the feint of heart. The only thing that would be required to allow this module to act on its own is to allow node authors to view their own nodes. The "default Drupal behavior" is to allow node authors to view their own nodes.

The attached patch simply tacks on another 'view' grant for the node author if any grants are being doled out by Node access user reference.

I have read through #342579: Maintaining permissions sanity when node access is updated a few times, and it seems like there were a few different issues embedded in there. Tackling just this one small usability issue on its own (and continuing to defer more complicated setups to other node access modules) seems like a good idea. At least, it would allow this module to be the lone node access module on many more sites.

When used in conjunction with the standard "delete own NODE-TYPE content" and "edit own NODE-TYPE content" permissions, the only grant that is required for node authors to see what they expect is the 'view' grant.

CommentFileSizeAuthor
#11 nodeaccess-userreference-author-grants-101126-11.patch4.3 KBmarcp
nodeaccess-userreference-author-read-grants.patch1.12 KBmarcp

Comments

danielb’s picture

danielb commented

I understand where you're coming from, but I still have some issues with this :/ The grants this module provides are for users that are referenced. The design choice to use the user's uid makes it very easy to also add in the author's uid - but that is a coincidence. Their 'grant' will mean something else other than everyone else's in the realm. For this module that is just an internal anomaly - but for other node access modules that use a more complicated grants system - it isn't possible to simply add the author's uid to the grants. So this makes me question whether it is something we should be doing?

I guess really what needs to be made clear is, whether a node access module is expected to provide access to the author?

There's nothing stopping us doing this. But I'm not sure it's the right thing to do, and then I have to think about whether to add author's grants to my other Node access modules where it won't be so easy.

I'd be happier with a new realm just for the author, or something. But it seems so awkward that every node access module would supply a version of this grant.

I also don't think there is anything hard about using multiple node access modules. You always read about how hard it is, but it isn't. I guess there is a use-case for not needing any other na modules, but I tend to think of this module as supplementing an existing node access set-up.

I'll try to find out more about this.

danielb’s picture

danielb commented
Category: bug » task

So basically what I've found out is

- It isn't my responsibility to add author grants, but....
- It is desirable to add author grants

I think what I'll look at is creating an author realm and automatic author grants, as a configurable extra feature.

bonobo’s picture

bonobo commented

From the place of maintaining sanity and a reasonable experience for end users, allowing an author to view the nodes they created is a sensible default.

RE: "I guess really what needs to be made clear is, whether a node access module is expected to provide access to the author?"

It's a rare case where the author is denied access to their posts. The only times I have seen that use case is when their is a need for an submission/review/approval workflow, and even in these situations removing edit rights has been more important than removing view rights.

In short, I'd say yes, it is reasonable to expect a node access module to provide view access to the author.

I'm currently testing the patch in the original post. Will report back with any issues.

marcp’s picture

marcp commented

@danielb - Your idea of a separate author realm with automatic grants as a configurable option sounds great. If I get around to it first, I'll supply a patch.

danielb’s picture

danielb commented

I've committed code for this to the 6.x-2.x dev branch. I just wrote it in cvs, haven't tested it, so keep this issue open. I'll also need to put similar functionality in the 7.x-2.x branch and some other modules.

ManyNancy’s picture

ManyNancy commented

Will this cause problems if I am using another content access module to grant view access?

bonobo’s picture

bonobo commented

@ManyNancy - please don't hijack this thread. Short answer: possibly, if you are using multiple forms of access control to cover the same node type.

You will likely need to adjust the weight of your access control modules, and test the outcomes. To test the outcomes, you should write out the "rules" you want your access control scheme to enforce. Then, configure your access control, set the module weights, create a set of nodes that are representative of your use cases, and test the results. You will likely need to adjust your config settings, rebuild the access controls settings, and re-test several times before you nail down the ideal solution.

Please open a separate ticket to address any of these issues/questions, and include details of the other access control modules and your setup.

@danielb - Awesome! I'll test this in the 6.x branch over the next couple days and if any issues arise, I'll ticket them up.

danielb’s picture

danielb commented

I tested it out; seems to work!

danielb’s picture

danielb commented
Version: 6.x-2.6 » 7.x-2.x-dev

gotta do it for 7.x

marcp’s picture

marcp commented

I haven't tested this yet but look forward to doing so. After a quick look at the code and thinking about this some more, it seems like we are going to want three similar checkboxes on the node type form nodeaccess_userreference_form_content_field_edit_form_alter() for granting view, edit and delete access to the node author.

If we don't do this, then after turning off the "edit own _node-type_" and "delete own _node-type_" permissions then there's no way to give users those rights.

I know that I'm going down multiple paths at once with wanting to be able to use nodeaccess_userreference on its own AND along with other node access modules, but I think it makes sense to try to keep it as general in that regard as we can.

marcp’s picture

marcp commented
Version: 7.x-2.x-dev » 6.x-2.x-dev
StatusFileSize
new nodeaccess-userreference-author-grants-101126-11.patch4.3 KB

I think the right way to deal with this in terms of author access is to provide 2 additional global settings (edit and delete) to go along with the view setting that you committed.

Attached is a patch that does just that. If users want to have different settings on a node-type basis, then they need to install another node access module. The three global settings for the author will now allow this module to stand on its own as a lone node access module.

The patch is against DRUPAL-6--2 in CVS. I'll re-roll for 7.x if you like this solution.

marcp’s picture

marcp commented

Note also that I tested what was in CVS and it worked great for author's "view" rights. However, even when granted global "edit own node-type content" rights via admin/user/permissions, the author would not be able to edit their own nodes (because we had already granted some rights to the node).

So this is why we need the two additional grant options.

danielb’s picture

danielb commented

rawr :( ok...

Would it be to awkward to make it one set of 'checkboxes', and just one drupal variable?
edit: nah actually forget that, matches the other form better this way

danielb’s picture

danielb commented

I've committed your patch with some other alterations.

marcp’s picture

marcp commented

Thank you, Daniel. I won't get a chance to test this until tomorrow.

Looking at the code, I think we only want to give the author grants when we have also given the userreference grants. I'll test that out and see if I can come up with some good reasoning for why that's the case.

danielb’s picture

danielb commented

You're right, we shouldn't 'fix up' our own grants if there aren't any set.

danielb’s picture

danielb commented

I've updated 6.x, still need to put this in 7.x

danielb’s picture

danielb commented
Status: Active » Fixed

I've added it to 7.x, this should do the trick, let me know if there are any problems.

marcp’s picture

marcp commented

@danielb - I still haven't tested this, but the code looks great. I'll ping back here after testing and will then start lobbying for a new release! Thanks very much for your work on this.

marcp’s picture

marcp commented

@danielb - Just tested this out really well and it's working great!

I added #1022106: Implement hook_node_access_explain() for devel integration with a patch that supplies hook_node_access_explain() -- after that it'd be great to get another 6.x release. I'll put a separate issue in for that and offer to help with any testing you'd need.

Status: Fixed » Closed (fixed)

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