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

[Łukasz Czyżykowski]

[David Owen (dsowen)]

(ready for review)

[Łukasz Czyżykowski (lukasz-czyzykowski)]

Looks OK

review: Approve

=== modified file 'identityprovider/middleware/csrf.py'
--- identityprovider/middleware/csrf.py	2010-04-21 15:29:24 +0000
+++ identityprovider/middleware/csrf.py	2010-06-04 14:56:29 +0000
@@ -19,6 +19,7 @@
 
 from django.conf import settings
 from django.http import HttpResponseForbidden
+from django.template import loader
 from django.utils.hashcompat import md5_constructor
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext as _
@@ -38,6 +39,9 @@
 def _make_token(session_id):
     return md5_constructor(settings.SECRET_KEY + session_id).hexdigest()
 
+def _render_403():
+    return HttpResponseForbidden(loader.render_to_string('403-csrf.html'))
+
 
 class CsrfViewMiddleware(object):
     """
@@ -63,10 +67,10 @@
             try:
                 request_csrf_token = request.POST['csrfmiddlewaretoken']
             except KeyError:
-                return HttpResponseForbidden(_ERROR_MSG)
+                return _render_403()
 
             if request_csrf_token != csrf_token:
-                return HttpResponseForbidden(_ERROR_MSG)
+                return _render_403()
 
         return None
 

=== added file 'identityprovider/templates/403-csrf.html'
--- identityprovider/templates/403-csrf.html	1970-01-01 00:00:00 +0000
+++ identityprovider/templates/403-csrf.html	2010-06-04 14:56:29 +0000
@@ -0,0 +1,21 @@
+<!-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file  LICENSE). -->
+
+{% extends "base.html" %}
+{% load i18n %}
+
+{% block "title" %}
+    {% trans "Stale request" %}
+{% endblock %}
+
+{% block "content" %}
+    <div id="box">
+        <h1 class="main">{% trans "Your page was stale." %}</h1>
+        <div>
+          <p>
+            {% blocktrans %}Apologies, the page you came from was a little old.  Perhaps you navigated here from a browser window other than the one you used to login.  If so, try using the other browser window.  Or, try your action again, starting from our home page.{% endblocktrans %}
+          </p>
+          <a class="btn alt" href="/"><span><span>{% trans "Go to our home page" %}</span></span></a>
+        </div>
+    </div>
+{% endblock %}

=== modified file 'identityprovider/tests/test_middleware.py'
--- identityprovider/tests/test_middleware.py	2010-05-28 19:56:57 +0000
+++ identityprovider/tests/test_middleware.py	2010-06-04 14:56:29 +0000
@@ -5,9 +5,13 @@
 import logging
 import re
 import sys
+from urlparse import urlsplit
 
 from django.conf import settings
 from django.contrib.auth.models import User, AnonymousUser
+from django.http import QueryDict
+import django.test.client
+
 from identityprovider.tests.utils import (BasicAccountTestCase, MockRequest,
                                           TestCase)
 from identityprovider.models import EmailAddress, Account
@@ -16,6 +20,14 @@
     UserAccountConversionMiddleware)
 
 
+def _extract_csrf_token(response):
+    csrf_field = re.search('<input [^>]*name=[\'"]csrfmiddlewaretoken[\'"]'
+                           '[^>]*/>', response.content)
+    return csrf_field and re.search(' value=[\'"]([^\'"]+)[\'"]',
+                                    csrf_field.group()).group(1)
+
+
+
 class UserAccountConversionMiddlewareTestCase(BasicAccountTestCase):
 
     def setUp(self):
@@ -144,38 +156,102 @@
 # The tests perform a login in order to acquire this session cookie.
 class CSRFMiddlewareTestCase(BasicAccountTestCase):
 
-    def login(self):
-        r = self.client.post('/+login', {
-            'email': '[e-mail]',
-            'password': 'test'})
-        self.assertTrue('sessionid' in self.client.cookies)
-
-    def test_allow_authorized(self):
-        self.login()
+    def _land(self, client=None):
+        client = client or self.client
+        r = client.get('/')
+        return r, _extract_csrf_token(r)
+
+    def _login(self, client=None, csrf_token=None):
+        client = client or self.client
+        form = {'email': '[e-mail]', 'password': 'test'}
+        if csrf_token:
+            form['csrfmiddlewaretoken'] = csrf_token
+        r = client.post('/+login', form)
+        self.assertTrue('sessionid' in client.cookies)
+        return r
+
+    def _logout(self, client=None):
+        client = client or self.client
+        return client.get('/+logout')
+
+    def test_allow_with_token(self):
+        self._login()
         r = self.client.get('/')
-        csrf_field = re.search('<input [^>]*name=[\'"]csrfmiddlewaretoken[\'"][^>]*/>', r.content)
-        self.assertTrue(csrf_field)
-        csrf_token = re.search(' value=[\'"]([^\'"]+)[\'"]', csrf_field.group()).group(1)
+        csrf_token = _extract_csrf_token(r)
+        self.assertTrue(csrf_token)
         r = self.client.post('/+edit', {
             'displayname': 'Mark Shuttleworthy',
             'csrfmiddlewaretoken': csrf_token
             })
         self.assertNotEquals(r.status_code, 403)
 
-    def test_forbid_unauthorized(self):
-        self.login()
+    def test_forbid_without_token(self):
+        self._login()
         r = self.client.post('/+edit', {'displayname': 'Mark Shuttleworthy'})
         self.assertEquals(r.status_code, 403)
 
-    def test_forbid_forged(self):
-        self.login()
+    def test_forbid_with_forged_token(self):
+        self._login()
         r = self.client.post('/+edit', {
             'displayname': 'Mark Shuttleworthy',
             'csrfmiddlewaretoken': '0'})
         self.assertEquals(r.status_code, 403)
 
+    def test_forbid_with_stolen_token(self):
+        self._login()
+        token = 'a'
+        r, token = self._land()
+        self.assertTrue(token)
+        # Get a new session, but don't invalidate the old session
+        self.client.cookies.clear()
+        self._login()
+        r = self.client.post('/+edit', {
+            'displayname': 'Mark Shuttleworthy',
+            'csrfmiddlewaretoken': token})
+        self.assertEquals(r.status_code, 403)
+
     def test_ajax(self):
-        self.login()
+        self._login()
         r = self.client.post('/+edit', {'displayname': 'Mark Shuttleworthy'},
                              HTTP_X_REQUESTED_WITH='XMLHttpRequest')
         self.assertNotEquals(r.status_code, 403)
+
+    def _login_multiple_windows(self, start_with_old_cookie,
+                                logout_before_stale_login):
+
+        # Multiple windows actually share a single browser state.
+        window1 = self.client
+        window2 = self.client
+
+        if start_with_old_cookie:
+            r1, csrf_token1 = self._land(window1)
+            r1 = self._login(window1, csrf_token1)
+            r1 = self._logout(window1)
+
+        r1, csrf_token1 = self._land(window1)
+
+        r2, csrf_token2 = self._land(window2)
+        r2 = self._login(window2, csrf_token2)
+
+        if logout_before_stale_login:
+            r2 = self._logout(window2)
+
+        r1 = self._login(window1, r1)
+        self.assertTrue(r1.status_code != 403)
+
+    def test_multiple_windows_no_cookie(self):
+        self._login_multiple_windows(start_with_old_cookie=False,
+                                     logout_before_stale_login=False)
+
+    def test_multiple_windows_old_cookie(self):
+        self._login_multiple_windows(start_with_old_cookie=True,
+                                     logout_before_stale_login=False)
+
+
+    def test_multiple_windows_no_cookie_logged_out(self):
+        self._login_multiple_windows(start_with_old_cookie=False,
+                                     logout_before_stale_login=True)
+
+    def test_multiple_windows_old_cookie_logged_out(self):
+        self._login_multiple_windows(start_with_old_cookie=True,
+                                     logout_before_stale_login=True)

=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py	2010-06-01 15:39:30 +0000
+++ identityprovider/views/ui.py	2010-06-04 14:56:29 +0000
@@ -32,13 +32,15 @@
     format_address, get_person_and_account_by_email, polite_form_errors,
     CannotResetPasswordException, PersonAndAccountNotFoundException)
 from identityprovider.decorators import check_readonly
+from identityprovider.middleware.csrf import csrf_exempt
+from identityprovider.models.captcha import Captcha
 from identityprovider.views.utils import get_rpconfig
-from identityprovider.models.captcha import Captcha
 
 
 logger = logging.getLogger('sso')
 
 
+@csrf_exempt
 @guest_required
 @dont_cache
 @limitlogin()