Commit c922a046 authored by Luke Plant's avatar Luke Plant
Browse files

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
parent fa4bbfcb

django/contrib/formtools/preview.py

+3 −20
Original line number Diff line number Diff line
@@ -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."

django/contrib/formtools/tests/__init__.py

+10 −18
Original line number Diff line number Diff line
@@ -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)

django/contrib/formtools/tests/urls.py

+0 −1
Original line number Diff line number Diff line
@@ -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])),