launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-597324 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #597324 Login fails when logging in while accessing URL with query parameters 
  https://bugs.launchpad.net/bugs/597324


Fix a unicode sin: use the decoded query parameters instead of creating
new, undecoded copies.

This /should/ fix the OOPS introduced in on edge by this branch (see bug
609029).

I also added a test to ensure this particular failure doesn't reoccur.

-- 
https://code.launchpad.net/~benji/launchpad/bug-597324/+merge/30829
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-597324 into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py	2010-07-21 13:32:53 +0000
+++ lib/canonical/launchpad/webapp/login.py	2010-07-23 21:22:53 +0000
@@ -251,6 +251,14 @@
         query_string = request.get('QUERY_STRING')
         if query_string is not None:
             params.update(cgi.parse_qsl(query_string))
+        # The production OpenID provider has some Django middleware that
+        # generates a token used to prevent XSRF attacks and stuffs it into
+        # every form.  Unfortunately that includes forms that have off-site
+        # targets and since our OpenID client verifies that no form values have
+        # been injected as a security precaution, this breaks logging-in in
+        # certain circumstances (see bug 597324).  The best we can do at the
+        # moment is to remove the token before invoking the OpenID library.
+        params.pop('csrfmiddlewaretoken', None)
         return params
 
     def _get_requested_url(self, request):

=== modified file 'lib/canonical/launchpad/webapp/tests/test_login.py'
--- lib/canonical/launchpad/webapp/tests/test_login.py	2010-07-21 13:32:53 +0000
+++ lib/canonical/launchpad/webapp/tests/test_login.py	2010-07-23 21:22:53 +0000
@@ -187,6 +187,24 @@
         }
         self.assertEquals(params, expected_params)
 
+    def test_csrfmiddlewaretoken_is_ignored(self):
+        # Show that the _gather_params filters out the errant
+        # csrfmiddlewaretoken form field.  See comment in _gather_params for
+        # more info.
+        request = LaunchpadTestRequest(
+            SERVER_URL='http://example.com',
+            QUERY_STRING='foo=bar',
+            form={'starting_url': 'http://launchpad.dev/after-login',
+                'csrfmiddlewaretoken': '12345'},
+            environ={'PATH_INFO': '/'})
+        view = OpenIDCallbackView(context=None, request=None)
+        params = view._gather_params(request)
+        expected_params = {
+            'starting_url': 'http://launchpad.dev/after-login',
+            'foo': 'bar',
+        }
+        self.assertEquals(params, expected_params)
+
     def test_get_requested_url(self):
         # The OpenIDCallbackView needs to pass the currently-being-requested
         # URL to the OpenID library.  OpenIDCallbackView._get_requested_url