Description

There is a problem with recreation of a deleted page if there are acls on that page and the user who does this does not have admin rights. -- ReimarBauer 2005-10-31 20:29:39

duplication of MoinMoinBugs/AclErrorOnUndeletedPage

Steps to reproduce

1. Set acl rights on a page and delete this page.
2. A user with no admin rights is not able to recreate this page. When he edits the page saving is not possible because

You can't change ACLs on this page since you have no admin rights on it!

Example

• Create a page like ExamplePage and do set for an user the acls

  #acl TestUser:read,write,delete All:read

• Then delete this page

• login as user TestUser

• Create a page named ExamplePage and select save result is

  You can't change ACLs on this page since you have no admin rights on it!

Details

Workaround

If a page with acls is deleted it should be better renamed before (prefix DeleteThisPage). Otherwise the name of the page is protected for using by users without admin rights. If the acl rights were used to prevent reading for all users it does not make sense to remove the acl right before deleting.

Discussion

At the moment it's not possible to recreate this page without admin rights. This is right because if TestUser was not able before to read the page because of #acl SomeUser:read,write,delete before he would be able to do this using the revision history. While he could do this too if one with admin rights recreates this page we should have an other solution for this.

In this example SomeUser could try to recreate the page could input all text and with save he got that he can't save because of acl protection. It would be better he got before typing anything that this page is protected by acls like for all other non admin users.

If an admin user recreates such a page it would be fine if the last used acl are defined for the page automatically. Then the usage of the revision history is protected again.

-- ReimarBauer 2006-02-03 03:23:04

Here is a possible "brute force" fix against Moin1.5.4-1 for that - as far as I could do that with my limited knowledge of the Moin code, Python and so on..

   1 --- wikiaction_old.py	2006-11-20 18:56:58.000000000 +0100
   2 +++ wikiaction.py	2006-11-20 21:59:04.000000000 +0100
   3 @@ -522,10 +522,25 @@
   4  def do_edit(pagename, request):
   5      _ = request.getText
   6  
   7 -    if not request.user.may.write(pagename):
   8 -        Page(request, pagename).send_page(request,
   9 +      
  10 +    # Check if pagename is an existing page
  11 +    if Page(request, pagename).exists() == True:
  12 +        # User allowed to edit? If not, return with error msg
  13 +        if not request.user.may.write(pagename):
  14 +            Page(request, pagename).send_page(request,
  15              msg = _('You are not allowed to edit this page.'))
  16 -        return
  17 +            return
  18 +        
  19 +    # Page does not exist? Check if pagename does still exist on disc.
  20 +    # If it does, kill it so as to have no acl conflicts when recreating it.
  21 +    elif Page(request, pagename).exists(True) == True:
  22 +        checklist = [0, 1]
  23 +        for use_underlay in checklist:
  24 +            pagedir = Page(request, pagename).getPagePath(use_underlay=use_underlay, check_create=0)
  25 +            if os.path.exists(pagedir):
  26 +                import distutils.dir_util
  27 +                distutils.dir_util.remove_tree(pagedir)
  28 +           
  29  
  30      valideditors = ['text', 'gui',]
  31      editor = ''

moin1-5-4-1wikiaction.patch

• That patch leads to an unwanted privilege escalation: usually delete privilege is nothing special (as it only makes a new, empty revision, but doesn't destroy anything - so you can easily revert it), but with this patch, a destructive user could nuke pages completely by first deleting them (harmless as before) and then editing them (nukes deleted pages). So I suggest to remove this patch.

It needs further thinking in the next days whether to apply this patch or leave the things as they are and add some hints on the problem of deleted pages with ACLs in HelpOnAccessControlLists.

Since it does make sense like Moin behaves at the moment I would prefer the second solution and maybe enhance Moin with some macro DeletedPages, that works like TitleIndex and lists all the deleted pages still remaining on disc. If the user has admin rights the macro could also show besides the pagenames actions like "unerase" and "purge completly" (i.e. killing the directory on disc as in the patch above). When calling DeletedPages(ACL) also the acls of the pages are listed. Maybe this could help administrating the wiki better and avoid problems / confused users as above. There has been already some request on MoinMoinQuestions for some tool like that which lists all deleted pages. -- OliverSiemoneit 2006-11-20 23:05:31

I agree that the patch above is not the best solution - but rather to "remove it from here" I would say: "Use it at your own risk. We don't recommand that. We don't support that". My aim is the following: I don't want to leave this bug report open forever, since it is not a bug but rather a clash of different opinions on what "deleting a page" should mean, a conflict between people who have totally different, incommensurable ideas and views on the power, the extent and safety-fallbacks of "page deletion". Therefore I want to suggestion the following for further proceeding:

• Turn this bug report and MoinMoinBugs/AclErrorOnUndeletedPage in MoinMoinNoBug. Add some hint on HelpOnAccessControlLists.

• Add a special section on "The problem of recreating deleted pages with ACL" in HelpOnAccessControlLists.

• Develop a DeletedPages macro as explained above so that admins can better check on deleted pages (with acls) and prevent users from being puzzeld by deleting the page totally in advance or to have the ability to easily restore pages if they were deleted by destructive users that want to spoil the wiki.

-- OliverSiemoneit 2006-11-21 13:17:28

The DeletedPages macro is now available on MacroMarket/DeletedPages -- OliverSiemoneit 2006-11-22 20:42:59

I suggest to add following paragraph "Recreating deleted pages with ACL" on HelpOnAccessControlLists:

Pages that do have acl settings are specially protected pages. You have to bear that in mind. Even if these pages have been deleted by the delete action but still remain silently on the filesystem, these pages are proteced against changing and overwritting by certain users without special rights. This is neither a bug nor a severe usability issue but intended behaviour of Moin. Primarily this is done to protect public wikis against destructive users. To make it clear: creating a new page with the same name of a page that was deleted before and has acls but still does exist silently on disc needs read rights to see the page, write rights to change the content and also admin rights because putting the page with its old history and all the old acls back into existance and try to overwrite it then means: you try to delete acl settings and/or you try to impose new acl settings to the page. Most users will have read, write rights. Only few users will have admin rights to set/change acls. Page creation  will fail for normal users therefore (if the users have also revert rights, they are at least able to put the page back into existance. From now, the old acls decide what the users can do with the page). To avoid these problems for normal users without admin rights, wiki admins and superusers are asked to use the MacroMarket/DeletedPages tool to regularily check the deleted pages on the wiki, unerase, completely purge those pages with acl which are not needed anymore or remove the acls from there. But be aware when removing acls: the current acl settings are in effect for the whole page history now so that users which were not allowed to view the content can now view the content over the revisions history.

So the question remains: is this still a bug? I would say: no! Or am I making a mistake forgetting something important?? -- OliverSiemoneit 2006-11-23 15:50:37

Plan

• Priority:
• Assigned to:
• Status:

CategoryMoinMoinBug