While using the framework of merlin
http://www.angrydonuts.com/publishing_articles_a_tutorial
for implementing a good workflow,
I noticed the following bug in node.module that disables checking
of node_access for unpublished nodes...
it says:
// If the module did not override the access rights, use those set in the
// node_access table.
#
#CCC: this disallows checking of node_access on unpublished nodes... what sense does this make?
# if ($op != 'create' && $node->nid && $node->status) {
if ($op != 'create' && $node->nid ) {
it is however desirable to work on unpublished nodes with other editors that DO NOT
have the "administer node" privilege, so IMO removing the check for status should be just fine...
what are your architects views? which implications does this have?
Thanks C.Cemper
Comments
Comment #1
drummComment #2
moshe weitzman commentedyes, that check should probably go. but we do need to hide unpublished nodes from regular users so i think all node modules should implement 'view own unpublished' functionality in their hook_access(). this need not be a new permission. hope thatt makes sense.
Comment #3
Christoph C. Cemper commentedMoshe,
the privileges in node_access are managed perfectly by
# na_arbitrator
# workflow_access
see http://www.angrydonuts.com/publishing_articles_a_tutorial
I wonder why a separate, hard-wired (hard coded!) implementation of privilege hooks would make sense?
the next guy that needs it round-other-way will issue it as a bug again
what's Drupal-Strategy? either it's configurable and flexible or hard-wired? I chose it for the first ...
christoph
Comment #4
moshe weitzman commentednaturally we prefer configurable over hard wired. but we also must provide sensible defaults for people who don't use those contrib modules. if you have a solution in mind to this need, please share.
Comment #5
Christoph C. Cemper commentedMoshe,
btw - is extra code really needed?
- unpublished nodes are not visisible to normal users anyway
- viewing it's own nodes was only disabled by the above IF statement
- if this 1-line fix would be left in there, the behaviour that people could read their own unpublished posts would stick.... and that's the most sensible default i can think of... I mean posting to a black-hole would mean a rather obscure function, eh? and those who need it could implement specific node_access rules (as I did :-)
what do you think?
christoph
Comment #6
webchickI also found this behaviour kind of bizarre... I wanted to implement a "view unpublished nodes" permission without having to hack node.module, and having that && $node->status there kind of severely limits my options. ;)
However, merely removing && $node->status is not the right solution; this makes it so unpublished nodes are always visible to everyone (even anonymous users) by going to node/# and that's no good.
Comment #7
magico commentedBumping this to 6.x-dev because I think this is 'by design'.
Anyway, it is a limitation that should be addressed.
Comment #8
pasqualleComment #9
BenK commentedSubscribing...
Comment #10
slosa commentedsub
Comment #11
minff commentedsubscribing
Comment #12
yang_yi_cn commentedmark #542788: node_access() prevent modules from applying grants on $node->status == 0 a duplicate of this.
Comment #13
yang_yi_cn commentedComment #14
yang_yi_cn commentedI think the reason why it's taking so long and nobody working on it is that the best way to fix (introduce a new permission for view unpublished node) is too complicated.
So I created another patch, which adds a little bit more logic on the grant table returned values. Instead of just returning 1 or false, I check the realm of the match record.
If the realm is "all", it means it's using the default / all implementation of node_access, which is published / unpublished ($node->status).
else (the realm is not "all", could be "workfow_access"), then use the granted value, which should overwrite the $node->status value.
So it's a minor change that work without introduction a new permission.
It seems to be working, please help me test it!
Comment #16
yang_yi_cn commentedComment #17
mdupontIt seems to be fixed in D7 ("view own unpublished content" and no requiring $node->status before checking for grants in node_access()) and at this state it's too late for D6. Closing.