# HG changeset patch # User Paul Boddie # Date 1318288392 -7200 # Node ID 445b18b4c7f8217a375b87c40fcf51af71aefbbc # Parent 272ec424a9874c1b67abf242d061f9d3e4665871 Introduced page-hiding by inserting ACLs into queued changes, requiring a special user to be created with write and admin privileges to perform the queuing of such hidden changes. Introduced signing of pages to prevent modification of queued changes, although the ACLs should prevent this sufficiently for unprivileged users. Made various role-testing convenience functions. diff -r 272ec424a987 -r 445b18b4c7f8 ApproveChangesSupport.py --- a/ApproveChangesSupport.py Mon Oct 10 22:28:58 2011 +0200 +++ b/ApproveChangesSupport.py Tue Oct 11 01:13:12 2011 +0200 @@ -2,11 +2,31 @@ """ MoinMoin - ApproveChanges library + This library relies on the existence of a user (by default + "ApprovalQueueUser") who has sufficient privileges to write pages with ACLs + to an approval queue (ACL permissions "write,admin"). + + If users other than the superuser are to be able to edit pages freely, they + must be present in a group (by default "ApprovedGroup"), and if they are to + be allowed to review changes, they must be present in a different group (by + default "PageReviewersGroup"). + @copyright: 2011 by Paul Boddie @license: GNU GPL (v2 or later), see COPYING.txt for details. """ +from MoinMoin import user import re +import base64 +import md5 +import hmac + +try: + from hashlib import sha1 +except ImportError: + from sha import new as sha1 + +acl_pattern = re.compile(ur"^#acl .*$", re.UNICODE | re.MULTILINE) __version__ = "0.1" @@ -16,6 +36,25 @@ def get_approved_editors_group(request): return getattr(request.cfg, "approved_editors_group", "ApprovedGroup") +def get_page_reviewers_group(request): + return getattr(request.cfg, "reviewers_group", "PageReviewersGroup") + +def get_queued_changes_user(request): + return getattr(request.cfg, "queued_changes_user", "ApprovalQueueUser") + +def get_secret_key(request): + return request.cfg.secrets["wikiutil/tickets"] + +def is_reviewer(request): + return request.user.valid and ( + request.dicts.has_member(get_approved_editors_group(request), request.user.name) or \ + request.user.isSuperUser()) + +def is_approved(request): + return request.user.valid and ( + request.dicts.has_member(get_approved_editors_group(request), request.user.name) or \ + request.user.isSuperUser()) + def is_queued_changes_page(request, pagename): "Return whether 'pagename' is a queued changes page by testing its name." @@ -29,6 +68,104 @@ return "/".join(pagename.split("/")[:-1]) +def get_user_for_saving(request): + + "Return a user that can save pages with ACLs." + + username = get_queued_changes_user(request) + uid = user.getUserId(request, username) + + # If the user does not exist, just return the existing user. + + if not uid: + return request.user + + # Otherwise, return the privileged user. + + return user.User(request, uid) + +def add_access_control(request, body): + + """ + Using the 'request', add an ACL to the page 'body' in order to prevent + anyone other than reviewers from seeing it in the queue. + """ + + # Find existing ACLs. + + match = acl_pattern.search(body) + if match: + start, end = match.span() + + # Comment out existing ACLs. + + parts = [] + parts.append(body[:start]) + parts.append("#") + parts.append(body[start:]) + else: + parts = [body] + + # Add the ACL. + + parts.insert(0, "#acl %s:read,write,delete,revert,admin %s:write All:\n" % ( + get_approved_editors_group(request), get_queued_changes_user(request))) + return "".join(parts) + +def remove_access_control(request, body): + + "Using the 'request', remove any added ACL to the page 'body'." + + lines = body.split("\n") + + try: + directive = lines[0].split()[0] + if directive == "#acl": + return "\n".join(lines[1:]) + except ValueError: + pass + + return body + +def get_page_signature(request, body): + + """ + Using the 'request', return a signature/digest for the given page 'body' + using a secret known only by the server. + """ + + secret_key = get_secret_key(request) + hash = hmac.new(secret_key, body.encode("utf-8"), sha1) + return base64.standard_b64encode(hash.digest()) + +def sign_page(request, body): + + """ + Using the 'request', sign the page 'body' using a secret known only by the + server. + """ + + return "#signature %s\n%s" % (get_page_signature(request, body), body) + +def check_page(request, body): + + """ + Using the 'request', find and check the signature in the page 'body', + returning the original page or None (if no valid signature is found). + """ + + lines = body.split("\n") + body = "\n".join(lines[1:]) + + try: + directive, signature = lines[0].split() + if directive == "#signature" and signature == get_page_signature(request, body): + return body + except ValueError: + pass + + return None + # Utility classes and associated functions. # NOTE: These are a subset of EventAggregatorSupport. diff -r 272ec424a987 -r 445b18b4c7f8 actions/ApproveChanges.py --- a/actions/ApproveChanges.py Mon Oct 10 22:28:58 2011 +0200 +++ b/actions/ApproveChanges.py Tue Oct 11 01:13:12 2011 +0200 @@ -38,7 +38,6 @@ d = { "buttons_html" : buttons_html, "prompt" : escape(_("Approve the displayed page version?")), - "purge_label" : escape(_("Purge all other queued versions")) } # Prepare the output HTML. @@ -46,16 +45,9 @@ html = ''' - + - - - - - @@ -74,12 +66,8 @@ # Make sure that only suitably privileged users can perform this action. queued_changes_page = get_queued_changes_page(request) - reviewers_group = getattr(request.cfg, "reviewers_group", "PageReviewersGroup") - if not request.user.valid or ( - not request.dicts.has_member(reviewers_group, request.user.name) and \ - not request.user.isSuperUser()): - + if not is_reviewer(request): return 0, _("Only page reviewers can perform this action.") # Edit the target page, using this page's content. @@ -91,10 +79,19 @@ target_page_name = get_target_page_name(self.pagename) target_page = PageEditor(request, target_page_name) - # Save the target page. + # Save the target page, first removing the signature and then removing + # any protective ACL. + + body = self.page.get_raw_body() + + body = check_page(request, body) + if not body: + return 0, _("The queued changes have been modified somehow. Not saving!") + + body = remove_access_control(request, body) try: - target_page.saveText(self.page.get_raw_body(), 0) + target_page.saveText(body, 0, comment=_("Changes to page approved.")) except PageEditor.Unchanged: pass diff -r 272ec424a987 -r 445b18b4c7f8 events/queue_for_review.py --- a/events/queue_for_review.py Mon Oct 10 22:28:58 2011 +0200 +++ b/events/queue_for_review.py Tue Oct 11 01:13:12 2011 +0200 @@ -17,27 +17,28 @@ request = event.request _ = request.getText - approved_editors_group = get_approved_editors_group(request) queued_changes_page = get_queued_changes_page(request) pagename = event.page_editor.page_name + body = event.new_text # Saving into queues has to be permitted or the mechanism will keep trying # to save into a queue of the specified page. if is_queued_changes_page(request, pagename): - # NOTE: Add ACL to prevent normal users from seeing the page anywhere. - # NOTE: (to-do/hide-queued-pages.txt) + # Test the integrity of the page in order to prevent direct replacement + # of the page. Reviewers can change the page as they please. - return None + if check_page(request, body) or is_reviewer(request): + return None + else: + return Abort(_("Queued changes may not be edited.")) # For normal pages, the user has to be approved. Otherwise, the page will be # saved into a queue. - elif not request.user.valid or ( - not request.dicts.has_member(approved_editors_group, request.user.name) and \ - not request.user.isSuperUser()): + elif not is_approved(request): # Save the page in the queue. # NOTE: Record the parent revision. @@ -45,8 +46,30 @@ new_page = PageEditor(request, "%s/%s" % (pagename, queued_changes_page)) + # Add an ACL to prevent normal users from seeing the page anywhere. + + body = add_access_control(request, body) + + # Sign the page to prevent modification in the queue. + + body = sign_page(request, body) + username = request.user.name + comment = (username or _("anonymous")) + " : " + _("Queued page edit") + try: - new_page.saveText(event.new_text, 0) + try: + new_page.saveText(body, 0, comment=comment) + + # Switch user in order to save a page with an ACL. + + except PageEditor.AccessDenied: + user = request.user + request.user = get_user_for_saving(request) + try: + new_page.saveText(body, 0, comment=comment) + finally: + request.user = user + except PageEditor.Unchanged: pass @@ -54,6 +77,8 @@ return Abort(_("Your changes have been queued for approval.")) + return None + def handle(event): if isinstance(event, PagePreSaveEvent): return handle_presave(event) diff -r 272ec424a987 -r 445b18b4c7f8 to-do/approve-specific-version.txt --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/to-do/approve-specific-version.txt Tue Oct 11 01:13:12 2011 +0200 @@ -0,0 +1,2 @@ +The approval action should act like the revert action and operate on a specific +revision of each queued changes page. diff -r 272ec424a987 -r 445b18b4c7f8 to-do/hide-queued-pages.txt --- a/to-do/hide-queued-pages.txt Mon Oct 10 22:28:58 2011 +0200 +++ b/to-do/hide-queued-pages.txt Tue Oct 11 01:13:12 2011 +0200 @@ -1,4 +1,5 @@ -Introduce ACLs in order to hide queued pages, thus defeating spammers by hiding +Done! +(Introduce ACLs in order to hide queued pages, thus defeating spammers by hiding their spam. This should be done when handling the pre-save event on the queued page so that direct editing of a queued page is unable to get around this -measure. +measure.) diff -r 272ec424a987 -r 445b18b4c7f8 to-do/improve-workflow.txt --- a/to-do/improve-workflow.txt Mon Oct 10 22:28:58 2011 +0200 +++ b/to-do/improve-workflow.txt Tue Oct 11 01:13:12 2011 +0200 @@ -1,7 +1,5 @@ Consider the approval workflow. Reverting the subpage to the desired version and then invoking an approval action could propagate the queued version, but other queued versions may need to be inspected later. Perhaps permit control over -whether earlier entries than the selected version can be purged, whether the -subpage should be deleted upon approval, and whether something like the diff -action's "revert to this version" mechanism can be used for a nicer approval -action. +whether earlier entries than the selected version can be purged, and whether the +subpage should be deleted upon approval.
%(prompt)s%(prompt)s
- -
%(buttons_html)s