Every anonymous user has author role access to nodes with 'anonymous' author (D6)

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

  if ($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

function 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.

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.

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

  if (($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.

(Applies to version 6.x-1.4 too.)

John's primary solution sounds reasonable and works. Thanks a lot.

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!

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.

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.

+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!

hm, didn't apply to dev, so here's an re-done

Failure of a manual apply

And realized an update operation is needed to say to rebuild node access :/

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;

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...

I realized I'd been bitten by this when I saw a bunch of node edit URLs in public search engine results! It could be argued that this constitutes an access bypass.

The patch in #13 works great, but it doesn't apply cleanly anymore. Here's one that does. I also fixed a couple of typos and coding style issues (see interdiff.txt below).

I brought it up to security team before, but since it's been in the open so long and since it could be considered a configuration issue, it doesn't need to be handled via them.

Quick glance over the patch looks good, though the coding style/sql optimizations should probably be a different issue.

No, I'm sorry: The typos and coding style issues were all in the patch. I didn't touch anything outside of it. See the interdiff.txt. And I didn't do anything to the SQL—I just changed it to save the result to the the correct variable. Before the result wasn't getting returned at all. See? ($res ≠ $rest)

+++ b/workflow_access/workflow_access.install
@@ -37,7 +37,7 @@ function workflow_access_schema() {
-  $res = array();
-  $rest[] = update_sql('DELETE from node_access where realm = "workflow_access_owner" and gid = 0');
-  return $res;
+  $ret = array();
+  $ret[] = update_sql('DELETE from node_access where realm = "workflow_access_owner" and gid = 0');
+  return $ret;

Oops.

Hah, bad of me then! XD.

Can we get this committed?

The default import user for the Feeds module is 'anonymous', meaning edit links appear on all nodes it creates if you have workflow enabled.

Did you test the patch @vinmassaro? If you have, you could probably put the issue into "reviewed and tested by the community", which would bring it to the maintainers' attention faster.

Sure, what version of workflow does your patch apply against in #15? I'm having issues getting it to work.

It's against 6.x-1.x-dev.

Sorry for the delay. I updated to 6.x-1.x-dev and was able to apply it with patch -p1. Works correctly. I no longer see any tabs on the node when viewing it as an anonymous user.

Unrelated but problematic: since the directory structure changed and submodules were added, workflow_ui is not enabled after running update.php against 6.x-1.x-dev. Is this intentional or just overlooked?

changes in directory should not effect whether a module is enabled.

I move modules around all the time

In Workflow 6.x-1.5, there was no submodule for the UI. You got the UI by default by just enabling workflow. This was split off in 6.x-1.x-dev as workflow_admin_ui submodule, so it needs to get enabled when updating, or you have no UI. Sounds trivial but we would be updating across 500 sites.

It's probably intended behavior, @vinmassaro, that the newly-separated UI module isn't enabled by default, but I'm only guessing. If you feel it should be otherwise, I would encourage you to open a separate issue for it.

Perhaps we can mark this RTBC now? I'm not supposed to, since I rolled the patch.

small clarification of the title.

I'm committing this patch to 7.x-2.x. D6 is not maintained.
I've split this patch in two: a first commit is for the basic Workflow module:
"Fixed hide workflow tab and 'author' transitions for anonymous users on nodes with anonymous author" is committed here and here.

The commit for workflow_access "Fixed access grants in workflow_access for anonymous users on nodes with anonymous author" is here.

As stated, fixed in 7.x-2.x. Leaving this open for D6. There will be no backport.

Tried an tested this patch from #15, works and sorted the issue out. Thanks TravisCarden!

I wish there was a nicer way to clear the issue queue from 'D6-issues that are fixed in D7' then a "won't fix for D6."

In case anyone runs into this and is stuck on version 7.x-1.2, I've attached a patch that adapts TravisCarden's patch #15 for that version.