Add Project Permissions Functionality

wonder95 - October 5, 2008 - 23:37
Project:Project
Version:5.x-1.2
Component:Miscellaneous
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review
Description

I went to the trouble to write a cool module called Project Permissions that gives the administrator the ability to control access to projects and issues at a user level. It uses hook_node_grants() to control individual access. However, since Project uses hook_access(), hook_node_grants() is never even called (I'm still not sure how I got it to work on the original site I did it for).

My suggestion is to do one of two things to add this functionality:

  1. Incorporate this functionality into Project module by adding the code in and adding capability for admin to decide if he wants to control access by role or individual
  2. Add code to look for Project Permissions module if it exists and use that, otherwise use hook_access

Or you could decide the Permissions module isn't worth it at all and just ignore it. :-) It just seems that this would be some good functionality to add to enhance Project module.

#1

aclight - October 6, 2008 - 00:00

This sounds a lot like the project_access module to me (http://drupal.org/project/project_access).

#2

dww - October 6, 2008 - 00:02

See also #234463: Remove 'access own projects' permission for more discussion about all of this, and why, if anything, we want to rip more of this functionality _out_ of project*, not add more to it.

#3

wonder95 - October 6, 2008 - 21:37

aclight - yes it does, but I wrote mine first (although it's probably not the best). :-)

#4

agentrickard - October 7, 2008 - 17:57

I would merge the two projects and then remove hook_access('view') from Project*.

@wonder95 -- you need to store copies of your node access data in a separate table so you can rebuild node access permissions on demand. Otherwise, installing a module like OG will wipe out all your node_access records.

One other thought, though. Project Access right now doesn't use {node_access} because of the hook_access() issue -- it expects a patched version of that function instead. If we go down the 'project issues as node access records' path, you have to write potentially multiple records for each issue, which could cause the table to get huge.

For instance, if Project module has 1,000 issues (not unreasonable) and 6 developers, that's 6,000 entries in the {node_access} table. On a site like d.o., that may become unusable.

#5

wonder95 - October 18, 2008 - 17:59

Hmmm, good point. When I wrote this, I thought I was saving code by not using my own table and writing directly to node_access. That wouldn't be too hard to do. I plagiarized a good bit of code from book_access, and that module does that, I so I could plagiarize some more.

What you are suggesting was exactly what I was thinking (i.e. pulling hook_access out of Project and using Project Permissions). The only issue I could think of would be in a case where a user wants to assign access by role instead of by user. Would that be possible? From what I understand of hook_access(), once it is used, it's either returning true or false, which means hook_node_grants() would never be called. Plus, I don't think there is a way to only call a hook when you want to, is there? If there was, I could add a config option to specify whether or not to assign access by role or by user, and use hook_access() if by role, otherwise skip it so hook_node_grants() would get called.

#6

agentrickard - October 18, 2008 - 19:42

When I wrote this, I thought I was saving code by not using my own table and writing directly to node_access.

I though the exact same thing when I first wrote Domain Access. It's a common mistake.

If you look at Project Access, it implements both per-user and per-role access controls, which is certainly possible. In a node_access module you just assign grants based on both items.

dww and I have been discussing ways to implement such a thing. One option is to use hook_access under normal conditions but allow it to be turned off via a setting if people want to use a node access module.

#7

wonder95 - October 18, 2008 - 23:11

One option is to use hook_access under normal conditions but allow it to be turned off via a setting if people want to use a node access module.

That's what I'm trying to understand. Since hook_access is automatically fired at the prescribed point in the process, how do you only call it when necessary? Even if you create a setting for it, how do you use it only when you want to?

#8

agentrickard - October 20, 2008 - 13:49

You set this at the top of the function:

<?php
function project_access($op, $node) {
  if (!
variable_get('project_default_access_system', TRUE) {
    return
NULL;
  }
 
// rest of function normally
}
?>

If your implementation of hook_access returns NULL, then node_access invokes the Node Access system.

See http://api.drupal.org/api/function/node_access/5

#9

wonder95 - December 31, 2008 - 04:12

@agentrickard - I went back and looked at this again, and I wasn't sure by your comment above if you wanted to merge my project and yours together, or just merge one of them with Project. I have some time to work on this now, so I can tackle it if you and dww are interested.

#10

agentrickard - December 31, 2008 - 15:17

I think we should merge our two projects independent of Project module, with the required changes to project_access() being submitted as patches.

I would prefer (for reasons of time) if you took the working parts of Project Access and merged them into your code. Then I can help you test and debug.

#11

wonder95 - January 3, 2009 - 06:09

@agentrickard - So since access can be given to both roles and users, which do you think should take precedence? I think it should be the user level. Say for instance, I belong to a role that doesn't have access to a certain module, but I am granted access to it individually. I think the more granular level should take precedence.

#12

agentrickard - January 3, 2009 - 21:09

Yes. That should be the way. If either your role or your account grants permission, you get permission. The role-based piece lets us do entire classes of users more efficiently. The user-based piece adds granularity.

#13

wonder95 - January 12, 2009 - 05:24

OK, here's my attempt, after a number of hours. I have the access records being written to node_access, but I must not be understanding how reading node_access works. I thought that by passing it the realm, it would only use the gids for that realm, but it doesn't look that way. The tricky part is allowing access based on users or roles, depending on how it's set up. What I started to try to do was look for records for the users role (using the aid of 'uid' in the project_access table), and if none were there, then look for any of the user's roles (with aid of 'rid' in project_access). But it looks like the realm is ignored when node_access is read, so if the user ID is 3 and role ID is 4, for instance, and there is a record in project_user_role realm with gid of 3 and access of 1/1/1, it will give access.

Anyway, I need a second pair of eyes. As I said, it's not complete, but I could use some direction from here.

Thanks.

AttachmentSize
project_access-add-user-access-control-317404.patch 2.02 KB
project_inc-add-user-access-control-317404.patch 408 bytes

#14

wonder95 - January 12, 2009 - 21:42
Status:active» needs review

Oops, forgot to indicate patch attached.

#15

agentrickard - January 19, 2009 - 23:19

My preference in the first case, is to use a hook instead of a direct module call.

  //return NULL if project_access module exists
  if (module_exists('project_access')) {
...

Instead becomes...

  //return NULL if a node access module exists

  $modules = module_implements('node_grants');
  if (count($modules) > 0) {
    return NULL;
  }

We have a number of ways to do this -- it could be a project-specific hook, and it could depend on a setting, but this way we could use any access control module.

#16

wonder95 - January 20, 2009 - 05:10

New patch.

AttachmentSize
project_inc-add-user-access-control-317404_v2.patch 443 bytes

#17

wonder95 - January 20, 2009 - 19:28

After thinking about this some more, this doesn't seem quite right. Just because there are modules installed that use node_grants, that doesn't mean that one of them is the one we want. For instance, with the way you suggested, if book_access is installed, hook_access will be disabled in project.inc (since book_access uses node_grants), and there will be no access control to project nodes since hook_access will return NULL.

#18

stodge - May 8, 2009 - 17:37

Any progress on this that supports Drupal 6?

#19

agentrickard - May 8, 2009 - 18:51

Take a look at how node_access() really works.

#20

wonder95 - May 15, 2009 - 14:50

I have spent a lot of time looking at it, and I obviously must be missing something, so could you please give me a little more than that? It would help.

 
 

Drupal is a registered trademark of Dries Buytaert.