From c922a046755db20d08c714b469503d3dcb5e0dfc Mon Sep 17 00:00:00 2001 From: Luke Plant Date: Wed, 30 Mar 2011 17:35:01 +0000 Subject: [PATCH] Updated FormPreview to use form_hmac rather than the old security_hash function This ought to have been done in [14218], but although the FormPreview class was modified, and some tests were added, the crucial lines of code were not changed (the 'FormPreview.security_hash' method), and tests for the new behaviour were not added. So it is being changed now. Unlike some of the other code in that changeset, this does not need to have a compatibility fallback to cope with existing hashes, because the consequence of a failed hash is minimal - the user is re-presented with the preview stage of the form, which will then have a correct hash. git-svn-id: http://code.djangoproject.com/svn/django/trunk@15952 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/formtools/preview.py | 23 +++--------------- django/contrib/formtools/tests/__init__.py | 28 ++++++++-------------- django/contrib/formtools/tests/urls.py | 1 - 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/django/contrib/formtools/preview.py b/django/contrib/formtools/preview.py index 7d6398c9a6..b4cdeba4de 100644 --- a/django/contrib/formtools/preview.py +++ b/django/contrib/formtools/preview.py @@ -12,7 +12,7 @@ from django.http import Http404 from django.shortcuts import render_to_response from django.template.context import RequestContext from django.utils.crypto import constant_time_compare -from django.contrib.formtools.utils import security_hash +from django.contrib.formtools.utils import form_hmac AUTO_ID = 'formtools_%s' # Each form here uses this as its auto_id parameter. @@ -72,24 +72,7 @@ class FormPreview(object): def _check_security_hash(self, token, request, form): expected = self.security_hash(request, form) - if constant_time_compare(token, expected): - return True - else: - # Fall back to Django 1.2 method, for compatibility with forms that - # are in the middle of being used when the upgrade occurs. However, - # we don't want to do this fallback if a subclass has provided their - # own security_hash method - because they might have implemented a - # more secure method, and this would punch a hole in that. - - # PendingDeprecationWarning <- left here to remind us that this - # compatibility fallback should be removed in Django 1.5 - FormPreview_expected = FormPreview.security_hash(self, request, form) - if expected == FormPreview_expected: - # They didn't override security_hash, do the fallback: - old_expected = security_hash(request, form) - return constant_time_compare(token, old_expected) - else: - return False + return constant_time_compare(token, expected) def post_post(self, request): "Validates the POST data. If valid, calls done(). Else, redisplays form." @@ -155,7 +138,7 @@ class FormPreview(object): Subclasses may want to take into account request-specific information, such as the IP address. """ - return security_hash(request, form) + return form_hmac(form) def failed_hash(self, request): "Returns an HttpResponse in the case of an invalid security hash." diff --git a/django/contrib/formtools/tests/__init__.py b/django/contrib/formtools/tests/__init__.py index a4324903bc..f093ba79f9 100644 --- a/django/contrib/formtools/tests/__init__.py +++ b/django/contrib/formtools/tests/__init__.py @@ -28,14 +28,6 @@ class TestForm(forms.Form): bool1 = forms.BooleanField(required=False) -class UserSecuredFormPreview(TestFormPreview): - """ - FormPreview with a custum security_hash method - """ - def security_hash(self, request, form): - return "123" - - class PreviewTests(TestCase): urls = 'django.contrib.formtools.tests.urls' @@ -124,36 +116,36 @@ class PreviewTests(TestCase): response = self.client.post('/test1/', self.test_data) self.assertEqual(response.content, success_string) - def test_form_submit_django12_hash(self): + def test_form_submit_good_hash(self): """ - Test contrib.formtools.preview form submittal, using the hash function - used in Django 1.2 + Test contrib.formtools.preview form submittal, using a correct + hash """ # Pass strings for form submittal and add stage variable to # show we previously saw first stage of the form. self.test_data.update({'stage':2}) response = self.client.post('/test1/', self.test_data) self.assertNotEqual(response.content, success_string) - hash = utils.security_hash(None, TestForm(self.test_data)) + hash = utils.form_hmac(TestForm(self.test_data)) self.test_data.update({'hash': hash}) response = self.client.post('/test1/', self.test_data) self.assertEqual(response.content, success_string) - def test_form_submit_django12_hash_custom_hash(self): + def test_form_submit_bad_hash(self): """ - Test contrib.formtools.preview form submittal, using the hash function - used in Django 1.2 and a custom security_hash method. + Test contrib.formtools.preview form submittal does not proceed + if the hash is incorrect. """ # Pass strings for form submittal and add stage variable to # show we previously saw first stage of the form. self.test_data.update({'stage':2}) - response = self.client.post('/test2/', self.test_data) + response = self.client.post('/test1/', self.test_data) self.assertEqual(response.status_code, 200) self.assertNotEqual(response.content, success_string) - hash = utils.security_hash(None, TestForm(self.test_data)) + hash = utils.form_hmac(TestForm(self.test_data)) + "bad" self.test_data.update({'hash': hash}) - response = self.client.post('/test2/', self.test_data) + response = self.client.post('/test1/', self.test_data) self.assertNotEqual(response.content, success_string) diff --git a/django/contrib/formtools/tests/urls.py b/django/contrib/formtools/tests/urls.py index 45888f99b6..6fc1e4ee22 100644 --- a/django/contrib/formtools/tests/urls.py +++ b/django/contrib/formtools/tests/urls.py @@ -7,7 +7,6 @@ from django.contrib.formtools.tests import * urlpatterns = patterns('', (r'^test1/', TestFormPreview(TestForm)), - (r'^test2/', UserSecuredFormPreview(TestForm)), (r'^wizard/$', WizardClass([WizardPageOneForm, WizardPageTwoForm, WizardPageThreeForm])),