Posted by John Money on April 18, 2009 at 9:47pm
| Project: | Workflow |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Not sure if this is technically a bug, but any anonymous user can view/edit/delete nodes created by other anonymous authors when workflow access is set to author role.
I think the author role should only apply to nodes created by authenticated users, otherwise the author role access permission becomes relatively meaningless for node types that can be created by both authenticated and anonymous users.
Attached is patch which will return the workflow_access_owner grant id as $node->uid only for authenticated users, otherwise it returns 1 (e.g. superuser). Previously, it could also return 0 which, as I said, is every anonymous user.
| Attachment | Size |
|---|---|
| workflow_access-anon-author.patch | 620 bytes |
Comments
#1
Very much a security flaw imo; drupal general treats uid 0 isn't a real user so shouldn't be treated as a user, right?
ie from node_access
<?phpif ($op == 'view' && $account->uid == $node->uid && $account->uid != 0) {
return TRUE;
}
?>
ie most users would not think this the behaviour, I'd think XD
Anyhow I added the additional check
<?phpfunction workflow_access_node_grants($account, $op) {
$access['workflow_access'] = array_keys($account->roles);
if ($account->uid) $access['workflow_access_owner'] =array($account->uid);
return $access;
}
?>
as a double gotcha so uid 0 doesn't get workflow_access_owner permission.
#2
It's not bug: it was decision of module author to make it work like that. And you can't change default module behavior only because you don't like it, in this case. Some people may want to treat all anonymous users equally and allow edit between all anonymous content. So if you want to change default behavior in the module, you need to implement it as module setting which can be switched to what user likes.
#3
Hm, I see ya point,
There may be a bug then in workflow_field_choices in that anonymous users are not given the author role
<?phpif (($user->uid == $node->uid && $node->uid > 0) || (arg(0) == 'node' && arg(1) == 'add')) {
$roles += array('author' => 'author');
}
?>
Anyhow here's my new suggestion for a patch. It introduces one variable, worklfow_user0_is_author and one alter hook 'workflow_authors'. Twas done that way to other modules could alter who is considered an author as far as work flow is considered; useful for those using user reference to have several authors? Missing one $node->uid in workflow_access ie. in workflow_access_form_submit; not sure what that form is doing.
#4
Can anyone give me a realistic use case where anonymous user permissions > authenticated user permissions?
As the module now stands, an anonymous user could view a node created by another anonymous user whereas an authenticated user could not... that makes sense how? where? when?
Further, said authenticated used could just logout and then view the node. It's pretty clearly a flaw in logic... unless I'm missing something (likely).
#5
(Applies to version 6.x-1.4 too.)
John's primary solution sounds reasonable and works. Thanks a lot.
#6
I just setup the first patch in this que.
It was simple and I have no use for anon authors in our workflow.
However, the patch in #3 looks valid and more flexible.
Which I like.
Anybody want to check it out and try and get it committed?
I'm 'subscribing' so I can come back to it later.
Thanks for the patches!
#7
As core sets uid to 0 for all nodes created by a user when it's deleted, and that there's no configuration, this is a bug, and might be a security issue. All anon users gaining access to nodes created by a deleted user!
Updated patch 3 to remove the variable as it seems unnecessary overhead; the alter hook on the occasion someone would want to customize this, can. fixed some coding style issues, but haven't re-tested it.
#8
Removing "$" from beginning of call for workflow_get_authors from the patch on #7.
Adjusting coding style and tested hook_workflow_authors_alter.
Also changing out "SELECT *" to only the necessary fields.
All seems to work well.
This patch is against 6.x-1.4.
If I have time later I will make one against dev.
#9
+1 for this. I was wondering why when I logged out I was able to edit workflow settings on certain nodes. The reason was that when I migrated all of my old content to Drupal, my migration script saved all of the nodes as an Anonymous user. So if you weren't logged in, you had access to workflow settings because you were also an Anonymous user. This definitely doesn't make any sense. Thanks for the patch!
#10
hm, didn't apply to dev, so here's an re-done
#11
Failure of a manual apply
#12
And realized an update operation is needed to say to rebuild node access :/
#13
Forcing someone to rebuild all node access is a bit of a PIA for big sites; here's one that just drops the owner for uid = 0;
#14
This bug allows spammers to find and edit nodes, deleting and posting anything they want. It's a major problem that can disrupt a site and I dont know if it isn't even a security issue...