Merge lp:~canonical-isd-hackers/canonical-identity-provider/bug_608920_csrf_token into lp:canonical-identity-provider/release

[David Owen]

= Overview =

CSRF protection inserts CSRF tokens into all outgoing forms that
aren't marked as exempt (or whose generating views aren't marked as
exempt).

SSO has been inserting CSRF tokens into forms that submit to LP and
other places. Specifically, when the OpenID return_to URL is to long
to allow a GET response, we generate a form for the user to POST.
This form, as it submits to LP, shouldn't have any CSRF tokens in it.

= Details =

The view (+decide) that generates the OpenID POST reponse also
generates several other, user-visible responses. Some of these
redirect, and one other generates a form that submits back to +decide.

The OpenID POST *response* is marked as exempt (not the entire view).

The new test verifies that other aspects of +decide work as expected,
especially that the other form (submitting back to SSO) is still
CSRF-protected.

[Ricardo Kirkner (ricardokirkner)]

Very clean and elegant test. Reads almost like a doctest. Keep up the good work!

review: Approve

=== modified file 'identityprovider/tests/test_middleware.py'
--- identityprovider/tests/test_middleware.py	2010-06-24 13:36:36 +0000
+++ identityprovider/tests/test_middleware.py	2010-08-23 16:46:41 +0000
@@ -12,9 +12,11 @@
 from django.http import QueryDict
 import django.test.client
 
+from openid.message import (IDENTIFIER_SELECT, OPENID1_URL_LIMIT, OPENID2_NS)
+
 from identityprovider.tests.utils import (BasicAccountTestCase, MockRequest,
-                                          TestCase)
-from identityprovider.models import EmailAddress, Account
+    OpenIDProviderTestCase, TestCase)
+from identityprovider.models import Account, EmailAddress, OpenIDRPConfig
 from identityprovider.middleware.exception import LogExceptionMiddleware
 from identityprovider.middleware.useraccount import (
     UserAccountConversionMiddleware)
@@ -255,3 +257,59 @@
     def test_multiple_windows_old_cookie_logged_out(self):
         self._login_multiple_windows(start_with_old_cookie=True,
                                      logout_before_stale_login=True)
+
+    def test_csrf_in_openid_forms(self):
+        trust_root = 'http://localhost/'
+        return_to = trust_root + (OPENID1_URL_LIMIT * 'a')
+
+        # Setup the trust_root to be trusted for OpenID operations.
+        self.rpconfig = OpenIDRPConfig(trust_root=trust_root)
+        self.rpconfig.save()
+
+        # 1. Establish a shared secret with the Provider.
+        r = self.client.get('/+openid', {
+            'openid.mode': 'associate',
+            'openid.assoc_type': 'HMAC-SHA1'})
+        [assoc_handle] = re.findall('assoc_handle:(.*)', r.content)
+
+        # Reset cookies, just in case, because after this we pretend
+        # to be the user.
+        self.client.cookies.clear()
+
+        # 2. RP directs user to Provider for checkid_setup and login.
+        r = self.client.get('/+openid', {
+            'openid.mode': 'checkid_setup',
+            'openid.realm': trust_root,
+            'openid.return_to': return_to,
+            'openid.ns': OPENID2_NS,
+            'openid.identity': IDENTIFIER_SELECT,
+            'openid.claimed_id': 'http://openid.launchpad.dev/+id/mark_oid',
+            'openid.assoc_handle': assoc_handle,
+            }, follow=True)
+        self.assertTemplateUsed(r, 'registration/login.html')
+
+        [oid_token] = re.findall(r'http://[^/]+/([^/]+)/\+decide',
+                                 r.redirect_chain[-1][0])
+
+        # 3. User logs in to SSO, but is allowed to decide whether to
+        # continue back to RP.
+        r = self.client.post('/%s/+login' % oid_token, {
+            'email': '[e-mail]',
+            'password': 'test'
+            }, follow=True)
+        self.assertTemplateUsed(r, 'decide.html')
+
+        csrf_token = _extract_csrf_token(r)
+        # We *must* have a CSRF token on the form that submits to SSO.
+        self.assertNotEquals(csrf_token, None)
+
+        # 4. User decides to continue on to RP.
+        r = self.client.post('/%s/+decide' % oid_token, {
+            'csrfmiddlewaretoken': csrf_token,
+            'yes': 'yes'
+            }, follow=True)
+        self.assertTemplateUsed(r, 'post-assertion.html')
+
+        # We *must not* have a CSRF token on the form that submits
+        # to the RP.
+        self.assertEquals(_extract_csrf_token(r), None)

=== modified file 'identityprovider/views/server.py'
--- identityprovider/views/server.py	2010-08-04 23:02:20 +0000
+++ identityprovider/views/server.py	2010-08-23 16:46:41 +0000
@@ -508,6 +508,9 @@
         r.content = loader.render_to_string("post-assertion.html", {
             'form': r.content})
         r['Content-Type'] = 'text/html'
+        # Also, as this is POSTing to another server, disabled CSRF
+        # protection.
+        r.csrf_exempt = True
     return r