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

drumm’s picture

Status: Needs review » Active
moshe weitzman’s picture

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

Christoph C. Cemper’s picture

Moshe,

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

moshe weitzman’s picture

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

Christoph C. Cemper’s picture

Moshe,

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

webchick’s picture

Version: 4.7.2 » 4.7.3

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

magico’s picture

Title: node_access not checked on unpublished nodes » node_access() not checked on unpublished nodes
Version: 4.7.3 » 6.x-dev
Category: bug » feature

Bumping this to 6.x-dev because I think this is 'by design'.
Anyway, it is a limitation that should be addressed.

pasqualle’s picture

Version: 6.x-dev » 7.x-dev
BenK’s picture

Subscribing...

slosa’s picture

sub

minff’s picture

subscribing

yang_yi_cn’s picture

yang_yi_cn’s picture

Version: 7.x-dev » 6.x-dev
yang_yi_cn’s picture

Status: Active » Needs review
StatusFileSize
new1.12 KB

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

Status: Needs review » Needs work

The last submitted patch, node_access_unpublished-72200-14.patch, failed testing.

yang_yi_cn’s picture

Status: Needs work » Needs review
Issue tags: +workflow, +Node access
mdupont’s picture

Status: Needs review » Closed (won't fix)

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