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

[David Owen]

Added checks for cookies to the major entry points, except the root. Root can't have a redirect because it's used as a service endpoint for other servers (OpenID Relying Parties).

[David Owen (dsowen)]

I found the Django test client used in some of our unit tests (I hadn't known about this test client before). It doesn't follow redirects automatically, you have to walk it through them. I think I'd be able to use this to test the cookie-test flows, but our doctests I think aren't setup for this client yet.

=== modified file 'identityprovider/decorators.py'
--- identityprovider/decorators.py	2010-05-05 08:32:42 +0000
+++ identityprovider/decorators.py	2010-06-01 20:49:21 +0000
@@ -9,6 +9,7 @@
 from datetime import datetime, timedelta
 import functools
 from hashlib import sha1
+import urllib
 
 
 def guest_required(func):
@@ -48,6 +49,19 @@
     return wrapper
 
 
+disable_cookie_check = False
+
+def requires_cookies(func):
+    @functools.wraps(func)
+    def wrapper(request, *args, **kwargs):
+        if disable_cookie_check or request.session.test_cookie_worked():
+            return func(request, *args, **kwargs)
+        request.session.set_test_cookie()
+        quoted = urllib.quote_plus(request.get_full_path())
+        return HttpResponseRedirect('/+cookie;' + quoted)
+    return wrapper
+
+
 class ratelimit(object):
     """ A rate-limiting decorator.
         Strongly based on Simon Willison's code,

=== added file 'identityprovider/templates/cookies.html'
--- identityprovider/templates/cookies.html	1970-01-01 00:00:00 +0000
+++ identityprovider/templates/cookies.html	2010-06-01 20:49:21 +0000
@@ -0,0 +1,18 @@
+<!-- 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 "Cookies required" %}{% endblock %}
+
+{% block "content" %}
+<h1 class="main">{% trans "Cookies required" %}</h1>
+
+<div>
+    <p>
+    {% blocktrans %}Cookies are required to use this service.  Please
+    enable cookies in your browser.{% endblocktrans %}
+    </p>
+</div>
+{% endblock %}

=== modified file 'identityprovider/tests/test_views_ui.py'
--- identityprovider/tests/test_views_ui.py	2010-05-26 16:59:02 +0000
+++ identityprovider/tests/test_views_ui.py	2010-06-01 20:49:21 +0000
@@ -3,16 +3,17 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import logging
+import re
 import urllib2
-import re
+from urlparse import urlsplit
 
 from django.conf import settings
-from django.core import mail
-from django.core import urlresolvers
+from django.core import mail, urlresolvers
+from django.http import QueryDict
 from django.test.client import Client
 from openid.message import IDENTIFIER_SELECT
 
-from identityprovider import signed
+from identityprovider import decorators, signed
 from identityprovider.models import authtoken as at
 from identityprovider.models import EmailAddress, Person, OpenIDRPConfig
 from identityprovider.models.const import EmailStatus
@@ -781,3 +782,39 @@
         self.assertContains(self.client.get('/'), 'es.png')
         self.client.login(username='[e-mail]', password='test')
         self.assertContains(self.client.get('/'), 'es.png')
+
+
+class CookiesTestCase(BasicAccountTestCase):
+
+    def setUp(self):
+        super(CookiesTestCase, self).setUp()
+        self._disable_cookie_check = decorators.disable_cookie_check
+        decorators.disable_cookie_check = False
+
+    def tearDown(self):
+        decorators.disable_cookie_check = self._disable_cookie_check
+        super(CookiesTestCase, self).tearDown()
+
+    def _get(self, url, *params):
+        r = self.client.get(url)
+        # The following line makes this cookieless:
+        self.client.cookies.clear()
+        if 300 <= r.status_code < 400:
+            location = r['Location']
+            scheme, netloc, path, query, fragment = urlsplit(location)
+            return self._get(path, QueryDict(query))
+        else:
+            return r
+
+    def _check_url(self, url):
+        r = self._get(url)
+        self.assertContains(r, '<title>Cookies required</title>')
+
+    def test_login(self):
+        self._check_url('/+login')
+
+    def test_forgot_password(self):
+        self._check_url('/+forgot_password')
+
+    def test_new_account(self):
+        self._check_url('/+new_account')

=== modified file 'identityprovider/tests/utils.py'
--- identityprovider/tests/utils.py	2010-05-28 15:11:06 +0000
+++ identityprovider/tests/utils.py	2010-06-01 20:49:21 +0000
@@ -23,6 +23,8 @@
     OpenIDServiceEndpoint, OPENID_1_0_TYPE, OPENID_1_1_TYPE,
     OPENID_2_0_TYPE, OPENID_IDP_2_0_TYPE)
 from openid.message import IDENTIFIER_SELECT
+
+from identityprovider import decorators
 from identityprovider.tests import mockdb
 
 
@@ -219,6 +221,7 @@
     sys.stderr = NullDevice()
 
     setup_test_environment()
+    decorators.disable_cookie_check = True
     settings.DEBUG = False
     suite = TestSuite()
 

=== modified file 'identityprovider/urls.py'
--- identityprovider/urls.py	2010-05-12 19:41:02 +0000
+++ identityprovider/urls.py	2010-06-01 20:49:21 +0000
@@ -45,6 +45,7 @@
 urlpatterns += patterns('identityprovider.views.account',
     (r'^%(optional_token)s\+edit$' % repls, 'index'),
     (r'^$', 'index'),
+    (r'^\+cookie;(?P<url>.+)$', 'cookie'),
     (r'^%(optional_token)s\+index$' % repls, 'index'),
     (r'^%(optional_token)s\+new-email$' % repls, 'new_email'),
     (r'^%(optional_token)s\+verify-email$' % repls, 'verify_email'),

=== modified file 'identityprovider/views/account.py'
--- identityprovider/views/account.py	2010-05-21 14:02:52 +0000
+++ identityprovider/views/account.py	2010-06-01 20:49:21 +0000
@@ -2,10 +2,11 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from openid.yadis.constants import YADIS_HEADER_NAME
+import urllib
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.http import HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import render_to_response, get_object_or_404
 from django.template import RequestContext
 from django.utils.translation import ugettext as _
@@ -24,6 +25,8 @@
     if request.user.is_authenticated():
         resp = account_index(request, token)
     else:
+        if not request.session.test_cookie_worked():
+            request.session.set_test_cookie()
         context = RequestContext(request, {
             'form': LoginForm(),
         })
@@ -32,6 +35,15 @@
     return resp
 
 
+def cookie(request, url=None):
+    if request.session.test_cookie_worked():
+        return HttpResponseRedirect(url or '/')
+    # Send the cookie again, so that the user can hit reload on the
+    # page we return to continue on.
+    request.session.set_test_cookie()
+    return render_to_response('cookies.html')
+
+
 @login_required
 def account_index(request, token=None):
     account = request.user

=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py	2010-06-01 15:39:30 +0000
+++ identityprovider/views/ui.py	2010-06-01 20:49:21 +0000
@@ -20,7 +20,7 @@
 from django.template.loader import render_to_string
 
 from identityprovider.branding import current_brand
-from identityprovider.decorators import guest_required, dont_cache, limitlogin
+from identityprovider.decorators import dont_cache, guest_required, limitlogin, requires_cookies
 from identityprovider.forms import (LoginForm, ForgotPasswordForm,
     ResetPasswordForm, NewAccountForm, ConfirmNewAccountForm)
 from identityprovider.models import (Account, AccountPassword, AuthToken,
@@ -42,6 +42,7 @@
 @guest_required
 @dont_cache
 @limitlogin()
+@requires_cookies
 def login(request, token=None, rpconfig=None):
     form = LoginForm()
     if request.method == 'POST':
@@ -123,6 +124,7 @@
 
 @guest_required
 @check_readonly
+@requires_cookies
 def new_account(request, token=None):
     if request.method == 'GET':
         if 'email' in request.GET:
@@ -334,6 +336,7 @@
 
 @guest_required
 @check_readonly
+@requires_cookies
 def forgot_password(request, token=None):
     if request.method == 'GET':
         if 'email' in request.GET:

=== modified file 'scripts/test'
--- scripts/test	2010-05-28 14:30:22 +0000
+++ scripts/test	2010-06-01 20:49:21 +0000
@@ -89,11 +89,13 @@
 
     sudo $ENV `which python` wsgi_test_server.py &
     sso_pid=$!
+    trap "sudo kill $sso_pid ; exit" SIGINT
 
     wait_until_up
 
     (cd doctests && python runner.py)
 
+    trap SIGINT
     sudo kill $sso_pid
 
     sleep 1