Closed (outdated)
Project:
Project
Version:
6.x-1.x-dev
Component:
Projects
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
15 Mar 2008 at 04:40 UTC
Updated:
10 Oct 2019 at 23:49 UTC
Jump to comment: Most recent
Comments
Comment #1
agentrickardThere really is no need to give a user explicit view permissions to their own nodes. The default node_access behavior is to return TRUE if $user->uid == $node->uid. See http://api.drupal.org/api/function/node_access/5
I have been researching this, and in core, 'view' permissions are never handled by hook_access(). Doing so interrupts the flow of the Node Access system. The only special case I think we need to deal with is users who can 'administer projects' -- which probably needs to be refactored to an 'edit projects' permission, in line with core. That permission only allows for Update and Delete. View is handled separately.
The current logic inside project_project_access() and project_db_rewrite_sql() may need to be refactored.
See http://drupal.org/node/291449 for some additional notes and the 'permission matrix' for project*.
Comment #2
aclight commentedSo you're saying that this patch isn't necessary, and that in fact the behavior this patch would explicitly provide is already present in core? If so, that's quite possible. I don't recall if I submitted this issue because of a functional bug or just because it didn't look like the code in the two functions matched up to me.
Comment #3
dwwI think you're both right and wrong. ;) Adam is right that this is a real bug, but his patch is wrong. Ken is right that the attached patch would be a step backwards, but he's wrong about this not being a bug, even given the default behavior of node_access. ;)
The problem is the line right after what Adam's patch touches:
That means that if a user has 'access own project issues', but not 'access projects', then they're denied, even if they're trying to view their own issue. That's definitely a bug. However, the solution isn't what Adam proposes. Instead of a check on 'access own project issues' that returns TRUE (which would then break other node_access mechanisms, as Ken rightfully points out), we need something like this:
Is that clear? If they (can't access their own projects, or it's anonymous, or it's someone viewing another project) AND they can't access projects generally, we need to deny them. We could also write it with the ! outside the || clause and convert all of those to &&, like so:
That's equivalent boolean logic, but I'm not sure if that's any more readable. ;) This might be best for readability:
All of this is untested. But, can we now all agree that this is a real bug, that the original patch is wrong, but that we need something like the logic I've written here to solve it?
Thanks,
-Derek
Comment #4
agentrickardI think we should just rely on node_access (the function) to handle user projects. See the end loop on the function.
http://api.drupal.org/api/function/node_access/5
This logic is invoked IFF the node module (project) did not assert node access rights, and if a node access module is not being used.
With Node Access (the system), generally node modules are expected to handle Update and Delete, and the core system handles View, with the theory being that you cannot update something you cannot see.
What I am saying is that 'access own *' is a permission that should be removed entirely. Core node modules do not use it, since the mogic already exists in node_access().
See the permission matrix document attached here: http://drupal.org/node/291449#comment-951826
Comment #5
dwwIf we remove 'access own projects':
A) How do we implement the desired use-case on sec.d.o where anyone would be able to create issues against a certain project, but only view their own issues? Or, are you saying that the existing project* code itself should do away with this and force people to use project_access if they want anything like this?
B) What do we do about sites in the wild that are already using and relying on this permission?
Re: your permission matrix doc, see #291449-3: Project-based access control.
Thanks,
-Derek
Comment #6
agentrickardA) Either Project Access or another Node Access system.
B) Document the changes to allow smooth transitions.
Comment #7
dwwSounds good to me. It'll require a totally different (bigger) patch to fix hook_db_rewrite_sql() and rip out other traces of the permission. But, that will mostly big a large patch that removes lines from project*, which is always good with me. ;) We'll also need an official release of project_access to document a migration to.
Comment #8
agentrickardThat's the plan ;-)
Comment #9
dwwBump: @agentrickard -- how's life with Project access? I can't really rip out this code until there's a migration path and an official release. You probably can't make an official release until it's going to work with the new project* code. What do we do? I'd like to try to clean out some of the D5 bugs and patches languishing in this queue in the near future and cut another official release so that we can proceed unhindered with D6 porting. I'd love to rip all this access-related code, if possible. Let's make a plan. ;) Thanks.
Comment #10
agentrickardProject access is fine, though Moshe pointed out during DrupalCON that OG can handle that, if needed, by making a Project node a group node, so PA might get deprecated.
I have been waiting for some of the other D5 patches to land -- like the $node namespace patch. We should clear out as many of those as possible and then make the permission change.
It is worth noting that I was able to clone a very large portion of Project* in D6 with other modules: CCK, Views, Comment CCK, Comment Upload, Notifications. The only pieces of Project that we might need in D6 are Project Release, CVS, and some form_alter() goodness for multi-stage issue forms and the AJAX elements on comment changes.
Comment #11
dwwa) I thought the whole point of project_access was for people who wanted finer-grained ACLs without all of OG.
b) Yes, it'd be nice to get all the other patches in before we roll one for here -- I was just trying to plan ahead.
c) Do you have an upgrade path for your project* clone? ;) If so, let's talk. Meet me over at #85049: convert project* to CCK.
Comment #12
agentrickardTrue.
a) PA is good to go. We can still release it, then we refactor Project* so that people can use whatever access control they want.
b) Agreed.
c) Coming over...
Comment #13
nedjoA related project is http://drupal.org/project/cck_groups. I wrote it for use with casetracker but with extensibility in mind so it could be used with project or others.
Comment #14
fuzzy_texan commentedTagging for #439958: Document issues left before 6.x official release
Comment #15
dwwActually, I think this is a blocker for 6.x-1.0. We really need to rip out all of the "access" permissions provided by the Project* suite:
access projects
access own projects
access project issues
access own project issues
we'll probably want to remove 'edit own project issues' while we're at it...
The good news is that wonder95 is furiously working on making http://drupal.org/project/project_access a totally working solution to this problem, so we can document that folks that want fine-grained access controls can have something customized for working with the Project* suite (or, they can use OG, private, etc, and it should all really work properly).
Comment #16
raintonr commentedJust trying to get project module working and found this. It's a big deal in the way I'd like to work.
Notice this issue and http://drupal.org/project/project_access haven't been worked on for a while. Is this dead or do people need help?
Comment #17
avpadernoI am closing this issue, as Drupal 6 is no longer supported.