[drupal-devel] [bug] node revisions should only be viewable by admins

Sat Sep 3 19:50:29 UTC 2005

Issue status update for 
http://drupal.org/node/30098
Post a follow up: 
http://drupal.org/project/comments/add/30098

 Project:      Drupal
 Version:      cvs
 Component:    node system
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  killes at www.drop.org
 Updated by:   mathias
 Status:       patch (code needs review)

Having node module set revision permissions for all other node modules
contradicts the interface of the permissions screen.  The current
interface suggests that if I want to set revision permissions for
books, I would look underneath book heading, not node.

Interface issues aside, if node module is going to set revision
permissions for all other node mods why stop there?  Why not "create
$type", "edit own $type", etc?

mathias

Previous comments:
------------------------------------------------------------------------

Wed, 31 Aug 2005 13:36:19 +0000 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB)

With or without the revisions patch, every user who can access content
can access old revisions. I think this is a bug because why would you
need to see old revisions if you cannot change them or make them the
current revision?

The attached patch fixes this and lets only users with "administer
nodes" permission see old revs as you need this permission to change
revisiosn to be the current one.

One might make a case for introducing new "view revisions" and "set
revisions" perms. You woud need those if you wanted to mimic a wiki
with Drupal.

------------------------------------------------------------------------

Wed, 31 Aug 2005 13:55:14 +0000 : jakeg

+1

I would also like to see the extra permissions added if possible: 

'view node revisions' - just for viewing node revisions

'administer node revisions' - to delete node revisions or set them as
default revision (i.e. current revision) for a node

I think this patch is important because e.g. if a revision is made
because sensitive information was included in a past revision, such as
a phone number, plain text email address (someone stupid didn't know
about spammers or the contact form) then these should definitely NOT be
accessible, and it would often be desirable to create a new revision to
a node rather than to edit the current copy without creating a
revision.

------------------------------------------------------------------------

Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff

I disagree. I see revisions as a means of updating a document for the
users. If a user has read the document, then sees that a revision has
been made, I'd want to provide him with some sort of diff that shows
him exactly what, as opposed to making him read the whole blasted thing
over and over again. In essence, I want a a wiki diff between revisions
(for example) [1]. Putting this permission in place would restrict my
ability to do that.
[1]
http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=4785

------------------------------------------------------------------------

Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff

Note to self: actually read the whole report before commenting.

-1 to JUST this patch. +1 to view/set revisions.

------------------------------------------------------------------------

Wed, 31 Aug 2005 13:59:13 +0000 : jakeg

good point #2 morbis, but the revisions tab isn't accessible to normal
users anyway... they only get to see the revisions if they know the URL
to get there (?revision=x in 4.6; /revisions/x in head)

but i agree that its definitely better with the extra permissions

------------------------------------------------------------------------

Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass

Please don't lump it in with administer nodes, please make a new
permission.

------------------------------------------------------------------------

Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann

Yes, please make this a separate permission.

In actuality, the use cases for viewing revisions probably needs a
permission *per node type*.

E.g. a site uses "story" for front page content. Editors some times
revise stories, but don't want those revisions public. The same site
uses book pages like a wiki. It gives all users create/edit/maintain
book privileges, and additionally wants them to be able to see
revisions.

So, if we don't enable viewing revisions for node type, I would hope
for a way to override this at the node type/module level -- so a custom
wiki.module could automatically add a "view revisions" permission.

------------------------------------------------------------------------

Thu, 01 Sep 2005 23:55:46 +0000 : killes at www.drop.org

Boris, what you propose makes sense  to me. I am however waiting for
feedback from Dries before I re-roll this patch.

if we introduce this new permissions, maybe we should factor the
revisions out into their own module?

------------------------------------------------------------------------

Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann

Revisions as a separate module might very well make more sense. Sites
can choose to run with them enabled or not. Might make it easier to
eventually do file versioning if we can hook into a module.

------------------------------------------------------------------------

Fri, 02 Sep 2005 05:46:30 +0000 : Dries

+1 for the permission(s), -1 for moving the revisions to a new module at
this point.

------------------------------------------------------------------------

Fri, 02 Sep 2005 13:08:21 +0000 : killes at www.drop.org

Dries, thanks for the feedback. I will prepare such a patch. Thanks for
saving my time, too. ;)

Boris, we already do some sort of file revisions. A file that is
attached to one revision can be deleted from another one, but will
still be available with the original revision. Only if we delete a node
completely, the file will actually deleted (or you delete it from every
revision).

------------------------------------------------------------------------

Fri, 02 Sep 2005 15:18:03 +0000 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB)

Ok, here is a patch which implements edit and view permissions per node
type. "admin nodes" is still valid as well. Needs testing.

------------------------------------------------------------------------

Fri, 02 Sep 2005 15:23:42 +0000 : killes at www.drop.org

NB: I'd appreciate a review from wiki users. I've implemented "view" as
access to the revision overview page and the individual revs and
""edit" as "change current revision" + "delete revision". I guess the
latter is a bit dangerous.

------------------------------------------------------------------------

Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff

I haven't tested the patch, but "delete revision" concerns me - it's the
style of a wiki to NEVER delete a previous version of a document - to
always have a permanent record of its manifestations. Annoyingly
enough, I woudn't +1 these new permissions until "delete" became its
own perm as well, separate from edit.

------------------------------------------------------------------------

Sat, 03 Sep 2005 16:20:00 +0000 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB)

Expected this. ;)

I've renamed "edit revisions" to "revert revisions" because that is
what you can do with that permission. I did not introduce a "delete
revs" permission but moved that permission back to "administer nodes".

------------------------------------------------------------------------

Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff

Sounds good to me!