Description

In Moin 1.8, the POST request parameters override the URL parameters. This can cause the action parameter specified in the URL to be overridden with something that is inappropriate for the handling of the request. For example, Moin as an OpenID relying party likes to put action in the requests to the OpenID provider, and when this is received by Moin as an OpenID server, it wants to handle the request with the action specified in the request body (typically login), not the action specified in the URL (serveopenid).

 1 # HG changeset patch
 2 # User Paul Boddie <paul@boddie.org.uk>
 3 # Date 1359679127 -3600
 4 # Node ID f548add9c9d5a0c445c752c82fb6465c2b442d21
 5 # Parent f5c49cf80dad51e0eb3f6e0801f58edd123b5a7d
 6 Prevent any request body parameter from overriding the action in the URL.
 7 This prevents the OpenID relying party authentication module in Moin (and
 8 potentially other relying parties) from confusing Moin as an OpenID server by
 9 including an "action" parameter in a POST request to an identity endpoint and
 10 causing Moin as an OpenID server to try and handle the request using the named
 11 action (typically "login") instead of using the "serveopenid" action.
 12 
 13 diff -r f5c49cf80dad -r f548add9c9d5 MoinMoin/request/__init__.py
 14 --- a/MoinMoin/request/__init__.py	Sun Oct 09 19:51:05 2011 +0200
 15 +++ b/MoinMoin/request/__init__.py	Fri Feb 01 01:38:47 2013 +0100
 16 @@ -181,12 +181,13 @@
 17  self.action = 'xmlrpc'
 18  self.rev = None
 19  else:
 20 + self.action = None
 21  try:
 22  self.args = self.form = self.setup_args()
 23  except UnicodeError:
 24  self.makeForbidden(403, "The input you sent could not be understood.")
 25  return
 26 - self.action = self.form.get('action', ['show'])[0]
 27 + self.action = self.action or self.form.get('action', ['show'])[0]
 28  try:
 29  self.rev = int(self.form['rev'][0])
 30  except:
 31 @@ -1068,6 +1069,7 @@
 32  """
 33  args = cgi.parse_qs(self.query_string, keep_blank_values=1)
 34  args = self.decodeArgs(args)
 35 + self.action = args.get('action', [None])[0]
 36  # if we have form data (in a POST), those override the stuff we already have:
 37  if self.request_method == 'POST':
 38  postargs = self._setup_args_from_cgi_form()

A patch to fix this bug, preserving the URL-specified action, is attached: patch-prevent-action-overriding-from-POST-requests-but-preserve-parameters-1.8.diff

Steps to reproduce

1. Set up one instance of Moin as an OpenID server and another as an OpenID relying party.
2. Log into the server instance normally.
3. Choose an OpenID login in the relying party instance and specify the URL of the server instance.

4. Upon approving the login, the relying party should indicate failure: OpenID error: check_authentication failed

Example

OpenID exposes such an obvious conflict, but there may well be other cases where request body parameters inappropriately override the action or other URL-specified parameters.

Component selection

• general (the cause of all this is in the MoinMoin.request.RequestBase.setup_args method)

Details

Workaround

Unfortunately, this is so severe for Moin 1.8 as an OpenID server that the obvious workaround is to upgrade to 1.9, which most people would have done anyway.

Discussion

This bug has been filed just to tidy up a loose end. It's a shame that I just didn't get round to testing this properly the last time I looked into OpenID with Moin. Of course, changing the logic for the action parameter is quite a serious matter because its usage in Moin is obviously extensive, but then again the action is always indicated using a URL-originating parameter, in my experience. -- PaulBoddie 2013年01月31日 23:31:28

Plan

• Priority:
• Assigned to:
• Status:

CategoryMoinMoinBug CategoryMoinMoinPatch