From 03efa304bce5ef0924948a74ae01cdf817dd416a Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Thu, 5 May 2016 12:42:19 -0400 Subject: [PATCH] Refs #25847 -- Added system check for UserModel.is_anonymous/is_authenticated methods. --- django/contrib/auth/checks.py | 21 +++++++++++++++++++++ docs/ref/checks.txt | 6 ++++++ tests/auth_tests/test_checks.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/django/contrib/auth/checks.py b/django/contrib/auth/checks.py index d1b0a44fdd..e7b5815cbd 100644 --- a/django/contrib/auth/checks.py +++ b/django/contrib/auth/checks.py @@ -6,6 +6,7 @@ from itertools import chain from django.apps import apps from django.conf import settings from django.core import checks +from django.utils import six from .management import _get_builtin_permissions @@ -73,6 +74,26 @@ def check_user_model(app_configs=None, **kwargs): ) ) + if isinstance(cls().is_anonymous, six.types.MethodType): + errors.append( + checks.Critical( + '%s.is_anonymous must be an attribute or property rather than ' + 'a method. Ignoring this is a security issue as anonymous ' + 'users will be treated as authenticated!' % cls, + obj=cls, + id='auth.C009', + ) + ) + if isinstance(cls().is_authenticated, six.types.MethodType): + errors.append( + checks.Critical( + '%s.is_authenticated must be an attribute or property rather ' + 'than a method. Ignoring this is a security issue as anonymous ' + 'users will be treated as authenticated!' % cls, + obj=cls, + id='auth.C010', + ) + ) return errors diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 486b88cc2e..e4af02c1a2 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -450,6 +450,12 @@ Auth to be at most 255 characters. * **auth.E008**: The permission named ```` of model ```` is longer than 255 characters. +* **auth.C009**: ``.is_anonymous`` must be an attribute or property + rather than a method. Ignoring this is a security issue as anonymous users + will be treated as authenticated! +* **auth.C010**: ``.is_authenticated`` must be an attribute or + property rather than a method. Ignoring this is a security issue as anonymous + users will be treated as authenticated! Content Types diff --git a/tests/auth_tests/test_checks.py b/tests/auth_tests/test_checks.py index 3ff78d9851..962444fb60 100644 --- a/tests/auth_tests/test_checks.py +++ b/tests/auth_tests/test_checks.py @@ -83,6 +83,39 @@ class UserModelChecksTests(SimpleTestCase): ), ]) + @override_settings(AUTH_USER_MODEL='auth_tests.BadUser') + def test_is_anonymous_authenticated_methods(self): + """ + .is_anonymous/is_authenticated must not be methods. + """ + class BadUser(AbstractBaseUser): + username = models.CharField(max_length=30, unique=True) + USERNAME_FIELD = 'username' + + def is_anonymous(self): + return True + + def is_authenticated(self): + return True + + errors = checks.run_checks(app_configs=self.apps.get_app_configs()) + self.assertEqual(errors, [ + checks.Critical( + '%s.is_anonymous must be an attribute or property rather than ' + 'a method. Ignoring this is a security issue as anonymous ' + 'users will be treated as authenticated!' % BadUser, + obj=BadUser, + id='auth.C009', + ), + checks.Critical( + '%s.is_authenticated must be an attribute or property rather ' + 'than a method. Ignoring this is a security issue as anonymous ' + 'users will be treated as authenticated!' % BadUser, + obj=BadUser, + id='auth.C010', + ), + ]) + @isolate_apps('auth_tests', attr_name='apps') @override_system_checks([check_models_permissions])