Skip to content

Commit

Permalink
Fixed #20760 -- Reduced timing variation in ModelBackend.
Browse files Browse the repository at this point in the history
Thanks jpaglier and erikr.
  • Loading branch information
aaugustin committed Jul 23, 2013
1 parent e716518 commit 5dbca13
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
4 changes: 3 additions & 1 deletion django/contrib/auth/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ def authenticate(self, username=None, password=None, **kwargs):
if user.check_password(password):
return user
except UserModel.DoesNotExist:
return None
# Run the default password hasher once to reduce the timing
# difference between an existing and a non-existing user (#20760).
UserModel().set_password(password)

def get_group_permissions(self, user_obj, obj=None):
"""
Expand Down
25 changes: 24 additions & 1 deletion django/contrib/auth/tests/test_auth_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@
from django.http import HttpRequest
from django.test import TestCase
from django.test.utils import override_settings
from django.contrib.auth.hashers import MD5PasswordHasher


class CountingMD5PasswordHasher(MD5PasswordHasher):
"""Hasher that counts how many times it computes a hash."""

calls = 0

def encode(self, *args, **kwargs):
type(self).calls += 1
return super(CountingMD5PasswordHasher, self).encode(*args, **kwargs)


class BaseModelBackendTest(object):
Expand Down Expand Up @@ -107,10 +118,22 @@ def test_has_no_object_perm(self):
self.assertEqual(user.get_all_permissions(), set(['auth.test']))

def test_get_all_superuser_permissions(self):
"A superuser has all permissions. Refs #14795"
"""A superuser has all permissions. Refs #14795."""
user = self.UserModel._default_manager.get(pk=self.superuser.pk)
self.assertEqual(len(user.get_all_permissions()), len(Permission.objects.all()))

@override_settings(PASSWORD_HASHERS=('django.contrib.auth.tests.test_auth_backends.CountingMD5PasswordHasher',))
def test_authentication_timing(self):
"""Hasher is run once regardless of whether the user exists. Refs #20760."""
CountingMD5PasswordHasher.calls = 0
username = getattr(self.user, self.UserModel.USERNAME_FIELD)
authenticate(username=username, password='test')
self.assertEqual(CountingMD5PasswordHasher.calls, 1)

CountingMD5PasswordHasher.calls = 0
authenticate(username='no_such_user', password='test')
self.assertEqual(CountingMD5PasswordHasher.calls, 1)


@skipIfCustomUser
class ModelBackendTest(BaseModelBackendTest, TestCase):
Expand Down

1 comment on commit 5dbca13

@claudep
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issue with this new test, in the case the used test settings are not set to use a MD5-based password hasher. create_users called in setUp() will use the hasher from current settings, then authenticate will fail because of not matching hasher, and CountingMD5PasswordHasheris never called.
A workaround might be to reset self.user password at the start of the test

@@ -125,6 +126,8 @@ class BaseModelBackendTest(object):
     @override_settings(PASSWORD_HASHERS=('django.contrib.auth.tests.test_auth_backends.CountingMD5PasswordHasher',))
     def test_authentication_timing(self):
         """Hasher is run once regardless of whether the user exists. Refs #20760."""
+        self.user.set_password('test')
+        self.user.save()
         CountingMD5PasswordHasher.calls = 0
         username = getattr(self.user, self.UserModel.USERNAME_FIELD)
         authenticate(username=username, password='test')

Please sign in to comment.